remkop / picocli

Picocli is a modern framework for building powerful, user-friendly, GraalVM-enabled command line apps with ease. It supports colors, autocompletion, subcommands, and more. In 1 source file so apps can include as source & avoid adding a dependency. Written in Java, usable from Groovy, Kotlin, Scala, etc.
https://picocli.info
Apache License 2.0
4.79k stars 414 forks source link

Custom boolean type converters behave strangely #2270

Closed remkop closed 2 months ago

remkop commented 2 months ago

As raised on the picocli mailing list by Sven K. (Many thanks!)

Problems

  1. Custom type converters for primitive boolean.class (Boolean.TYPE), registered with CommandLine.registerConverter, appear not to be invoked, resulting in errors that the custom value cannot be converted to boolean.

  2. Custom type converters for java.lang.Boolean, are invoked twice, once with the original value provided on the command line, and a 2nd time with the String value of the conversion result. (E.g. if 123 is converted to true, the converter is called once with 123 and again with true.)

Reproducer project: https://codeberg.org/sven.k/picocli-reproducer Main class: https://codeberg.org/sven.k/picocli-reproducer/src/branch/master/app/src/main/java/org/example/ConverterDemo.java

Analysis

Problem 2:

The reason why the converter for java.lang.Boolean is invoked twice: There is special logic for boolean values that determines the boolean value of the command line argument, changes it to its opposite if the option is negatable, converts that boolean value back to a String, and finally again converts that String value to a boolean.

Solution: only convert if necessary (if the option is actually negatable), and otherwise retain the original command line value. (Note that the issue still exists - but seems unavoidable - for negatable options; this should be documented somewhere.)

Problem 1:

Again, the special logic for boolean values is the culprit. When determining the boolean value of the command line argument, the converter used is that for java.lang.Boolean, regardless of the actual type of the option.

Solution: obtain the type of the option to get the converter, instead of hard-coding the converter to be for java.lang.Boolean.

remkop commented 2 months ago

Fixed in main. Thank you for raising this, Sven!