Pluggable enum conversion facility for picocli.

[garretwilson]

Pluggable enum conversion facility for #1804 .

[remkop]

Thank you for the pull request. I will look in more detail later (so I may have more feedback later) but overall it looks good.

Please add documentation and tests and I think this can be merged.

Please add a section "Custom Enum Type Converters" to the user manual
(located here: docs/index.adoc - HTML is auto-generated from this).
I suggest after section === Handling Invalid Input and
before section === Option-specific Type Converters.

Also, please provide JUnit tests that verify at least

• that registering a custom enum type converter will result in that enum type converter being used for options and positional parameters with that enum type
• that the custom enum type converter will NOT be used if a custom ITypeConverter is registered for that enum type
• a custom enum type converter falling back to the default enum type converter
• setting CommandLine::setCaseInsensitiveEnumValuesAllowed to true prior to parsing will result in the custom enum type converter being called with value true
• not calling CommandLine::setCaseInsensitiveEnumValuesAllowed prior to parsing will result in the custom enum type converter being called with value false
• throwing a TypeConversionException from the registered custom enum type converter will result in a ParameterException with a user-friendly message from CommandLine::parseArgs
• registering a null custom enum type converter is rejected immediately with a NullPointerException (not silently accepted, causing problems during parsing)
• calling CommandLine::registerEnumConverter will register the custom enum type converter for that command and all its subcommands and sub-subcommands

(most tests are here: src/test/java/picocli)

[garretwilson]

Thank you for considering this.

Also, please provide JUnit tests that verify at least …

As I explained in #1804, unfortunately I do not have time to try to work with a build system that does not behave properly with the latest Java version. If the build configuration allowed me to clone the repository and then successfully run existing tests by simply invoking mvn test or gradle test using Java 24 and the latest Maven/Gradle version, then I would be happy to add tests. However the current build system breaks when running mvn test or gradle test using Java 24 and the latest Maven/Gradle version.

I do not have the time to try to fix the build system or to downgrade my toolchain to try to coax the build system into working. Thus I will not be providing any tests for this pull request unless the build system is fixed. I'm sorry.

[garretwilson]

I just noticed this formatting oddity and thought I had made a typo, but then I saw it was in the original code, so I'll leave it as-is for the moment.

[remkop]

I noticed it too in the original code. Looks like a newline was accidentally deleted. It was not intentional, please fix in the PR.

[remkop]

@garretwilson The project now builds correctly on Java 24 and Gradle 8.14. The Github CI build now also includes Java 24.
Thank you for raising this. Hopefully this addresses your concerns.

[garretwilson]

The project now builds correctly on Java 24 and Gradle 8.14.

I pulled the latest from upstream and then tried gradle test. I got much further this time! But it looks like there's a failing test. I get the following both on this branch and on main:

…
> Task :test

Issue1125_1538_OptionNameOrSubcommandAsOptionValue > testAmbiguousOptionsDefault FAILED
    org.junit.ComparisonFailure at Issue1125_1538_OptionNameOrSubcommandAsOptionValue.java:132

stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address
stty: /dev/tty: No such device or address

> Task :test FAILED

2274 tests completed, 1 failed, 56 skipped

…

BUILD FAILED in 18s
6 actionable tasks: 6 executed

Did you want to look into that failing test?

[garretwilson]

Also note that importing this project into both Eclipse and VS Code with Java support raised the exact same error.

The supplied phased action failed with an exception.
Could not create task ':bundle'.
Cannot change attributes of configuration ':bundleCompileClasspath' after it has been locked for mutation

I'll see if I can work around that, but it's something else you probably should look into.

[garretwilson]

Unfortunately, even adding a JUnit 4 @Ignore to Issue1125_1538_OptionNameOrSubcommandAsOptionValue, Gradle doesn't complete tests:

…
> Task :picocli-codegen-tests-java9plus:test

Issue1380Test > testingWithExclusiveTrue FAILED
    org.junit.ComparisonFailure at Issue1380Test.java:68

Issue1380Test > testingWithExclusiveFalse FAILED
    org.junit.ComparisonFailure at Issue1380Test.java:87
…

It's at this point I realize again that going in circles is going to waste the whole morning, so I'm stopping this for today.

Could you make sure that gradle test works from the command line on Java 24 so that I can have a working suite to start with before adding to the tests? I don't want to be fighting the failing tests at every turn. Thanks.

[remkop]

Strange, all tests pass with Java 24 on my machine, and also on Github, both in the main branch and on your branch (macos build failures for Java 9 are ignored).

The GitHub tests cover a wide range of Java versions and operating systems, I wonder what is different about your environment.

The two failing tests for Issue1380Test verify the usage help output. Could you share the details of the diff from the test failure? (How is the expected output different from the actual output?) Similarly for Issue1125_1538_OptionNameOrSubcommandAsOptionValue > testAmbiguousOptionsDefault; this test also verifies usage help output.

[remkop]

I am guessing that the failing tests were because the usage help message had ANSI escape codes in the output in your environment.

I fixed the tests to ensure that the comparison is without the escape codes regardless of the environment.

[garretwilson]

Good news — I am now able to run gradle test on this branch and it succeeds. Thank you for fixing those tests.

[garretwilson]

FYI, using gradle test with Gradle 8.14.3 still works with new LTS Java 25! But there are a significant number of API deprecation warnings, so I recommend you start building with a higher Java version and addressing these. Here are a couple of examples:

…\picocli-examples\src\main\java\picocli\examples\i18n\I18NDemo.java:22: warning: [deprecation] URL(String) in URL has been deprecated
        try (java.util.Scanner s = new java.util.Scanner(new java.net.URL(URL).openStream())) {
                                                         ^
…\picocli-examples\src\main\java\picocli\examples\i18n\localecontrol\LocaleControl.java:21: warning: [deprecation] Locale(String) in Locale has been deprecated
        Locale.setDefault(new Locale(locale));

[garretwilson]

@remkop I've added the unit tests you've requested. As I mentioned in issue #1804, I started with tests an LLM generated, and I haven't looked at the tests super closely. However I did glance over them just now and tweaked them a little.

It's amazing, after I got the LLM aligned with what I was looking for (e.g. specifying a Java 8 build, JUnit 4, consistency with existing picocli tests, etc) how little tweaking I needed to do. Here are some example:

• I changed received.set(Boolean.valueOf(caseInsensitiveEnumValuesAllowed)) to received.set(caseInsensitiveEnumValuesAllowed) to use autoboxing, because I didn't see the point of spelling it out the conversion.
• In one JUnit test that expected a NullPointerException, for some reason it had used the ancient-style of catching and ignoring an exception while using fail() if the exception did not happen. I simplified this using @Test(expected = NullPointerException.class).
• In the last test it was wrapping the parseArgs() calls in a try … catch (Exception ignored) { }, and I removed these because they aren't useful or even appropriate here (and ignoring exceptions in general are 99.999% of the time not what should be done anyway). In further conversations with the LLM it confirmed these try … catch blocks weren't appropriate. (Why it put them there in the first place illustrates some interesting aspects of LLMs that we can discuss over a drink some day.) Frankly the fact that an LLM could put that reasonably complex test together based upon such a vague request, understanding the code and not only testing inheritance but also using a technique to distinguish among subcommands, is amazing in itself.

I haven't updated any documentation; I'll have to do that on another day when I can find another few minutes free. In the meantime please take a look at the tests and tell me if they're OK. Please also respond to the earlier question I had in the main ticket about how you want to test a "user-friendly error message". Thanks and have a great weekend.

[garretwilson]

Please add a section "Custom Enum Type Converters" to the user manual
(located here: docs/index.adoc - HTML is auto-generated from this).
I suggest after section === Handling Invalid Input and
before section === Option-specific Type Converters.

I took a look at the docs, and section "4.2. Custom Type Converters" is the one about converters, so I'm thinking right before section === Handling Invalid Input (i.e. at the end of the "Custom Type Converters" section) might be better. (Technically before "Multi Parameter Type Converters" might have even been better, but I was afraid of changing the numbering.)

Take a look and let me know what you think. I think these docs are the last thing to finalize for this PR.