Document static vs non-static nested classes (was: Kotlin: failure when declaring IParameterPreprocessor as inner class)

[deining]

Minimum working example which runs flawlessly:

1  import picocli.CommandLine
2  import picocli.CommandLine.Option
3  import picocli.CommandLine.Model.*
4  import java.util.*
5  import kotlin.system.exitProcess
6
7  class AccessInner : Runnable {
8
9      @Option(names = ["-a"],
10         // preprocessor = MyPreprocessor::class
11     )
12     var foo = 42
13
14     override fun run() {
15         println("Hi, ${MyPreprocessor().bar}")
16     }
17
18     inner class MyPreprocessor : CommandLine.IParameterPreprocessor {
19
20         val bar : Int = foo // works only with inner class
21 
22         override fun preprocess(
23             args: Stack<String>?,
24             commandSpec: CommandSpec?,
25             argSpec: ArgSpec?,
26             info: MutableMap<String, Any>?
27         ): Boolean {
28             return false
29         }
30     }
31 }
32 
33 fun main(args: Array<String>): Unit = exitProcess(CommandLine(AccessInner()).execute(*args))

As soon as I enable the preprocessor for option -a by uncommenting line 10, I'm getting an error:

Exception in thread "main" picocli.CommandLine$InitializationException: Cannot instantiate ParameterProcessor.AccessInner$MyPreprocessor: the class has no constructor
	at picocli.CommandLine$DefaultFactory.create(CommandLine.java:5514)
	at picocli.CommandLine$Model$ArgSpec$Builder.<init>(CommandLine.java:9248)
	at picocli.CommandLine$Model$OptionSpec$Builder.<init>(CommandLine.java:9790)
	at picocli.CommandLine$Model$OptionSpec$Builder.<init>(CommandLine.java:9769)
	at picocli.CommandLine$Model$OptionSpec.builder(CommandLine.java:9646)
	at picocli.CommandLine$Model$CommandReflection.buildArgForMember(CommandLine.java:11616)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedTypedMembers(CommandLine.java:11520)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedFields(CommandLine.java:11466)
	at picocli.CommandLine$Model$CommandReflection.extractCommandSpec(CommandLine.java:11399)
	at picocli.CommandLine$Model$CommandSpec.forAnnotatedObject(CommandLine.java:6202)
	at picocli.CommandLine.<init>(CommandLine.java:227)
	at picocli.CommandLine.<init>(CommandLine.java:221)
	at picocli.CommandLine.<init>(CommandLine.java:196)
	at ParameterProcessor.AccessInnerKt.main(AccessInner.kt:35)
Caused by: java.lang.NoSuchMethodException: ParameterProcessor.AccessInner$MyPreprocessor.<init>()
	at java.base/java.lang.Class.getConstructor0(Class.java:3350)
	at java.base/java.lang.Class.getDeclaredConstructor(Class.java:2554)
	at picocli.CommandLine$DefaultFactory.create(CommandLine.java:5489)
	at picocli.CommandLine$DefaultFactory.create(CommandLine.java:5512)
	... 13 more

The error is due to the fact that MyPreprocessor is declared as inner class. As soon as remove the inner keyword, everything is fine again. However, since there might be good reasons to make use of an inner class, I'm asking myself if there is a way to make that work. I didn't dig into this further, but I found this resource that might be helpful.
Any help is appreciated!

[remkop]

I believe this is similar to inner classes vs static inner classes in Java, except that in Kotlin, nested classes are static by default, whereas in Java they require the static keyword.

In Kotlin, nested classes that are marked as inner carry a reference to an object of an outer class. (see docs)

So, where in Java, static inner classes work out of the box, in Kotlin, nested classes without inner should work out of the box - which matches your observations.

Inner classes can still be made to work in picocli, but require a factory that provides the reference to the outer class.
The picocli tests have such a factory: InnerClassFactory.java. I believe the above example will work when this factory is specified.

(This is a test Factory. In reality, the problem is that this factory requires an instance of the outer class, which may not be constructed yet. This could be fixed by making the factory return the specified instance of the outer class, so the outer class effectively becomes a singleton. But I suspect this configuration will only be useful to applications in some rare cases...)

[deining]

Thanks for sheding more light on this!

But I suspect this configuration will only be useful to applications in some rare cases...)

I agree. I tend to think that there are more important issues in the backlog/ issue list. It's up to you to decide whether we want to pursue this ticket or not. If we won't get a solution in the long run, this should be documented somewhere IMHO (chapter 30.2 Kotlin of the user manual would be a good place).

[remkop]

... this should be documented somewhere IMHO (chapter 30.2 Kotlin of the user manual would be a good place).

Not sure: I don't think this is a Kotlin problem, this is more general.
All languages that support non-static nested classes (having a reference to a particular instance of the enclosing class) will have this issue.

For example, in Java:

class Outer {
    @Option(names = "--option", preprocessor = Outer.InnerPreprocessor.class)
    String value;

    // non-static
    class InnerPreprocessor implements IParameterPreprocessor {
       public boolean preprocess(Stack<String> args, CommandSpec cmd, ArgSpec arg, Map<String, Object> info) {
          return false;
      }
    }

    public static void main(String... args) {
        new CommandLine(new Outer()).parseArgs(args); // this will fail with "Cannot instantiate InnerPreprocessor"

        // To instantiate the above `InnerPreprocessor` in a Java program, we need to use this strange syntax:
        // Outer o = new Outer(); 
        // InnerPreprocessor inner = o.new InnerPreprocessor();
        //
        // Reflective access needs to use the synthetic constructor that
        // takes an instance of the enclosing class as a parameter.
    }
}

This would give the same "Cannot instantiate" problem as the Kotlin example above.
(And here also, if InnerPreprocessor was a static nested class, everything would work fine.)

It would be very rare for an application to use a non-static inner class for picocli's "plugin" components like preprocessor, type converter, subcommands, version providers, default providers, etc.

For cases where it truly is necessary to share information between a nested class and its enclosing instance:

• one way is to use a custom factory. (This can be done with the current version of picocli.)
• Another idea would be to modify picocli to support injecting @Spec CommandSpec spec into all "plugin" components. Currently this is only supported for version providers, but it may make sense to do this for all "plugin" components. This would allow the plugin component to access the command where it is used via CommandSpec::userObject, and would work for non-nested classes too.
  • However, implementing this may be quite involved and there is some interaction with @Command(scope = INHERIT) commands where plugin instances are copied to subcommands.

I have not heard anyone requesting this feature, so it probably is not a common requirement.

[remkop]

Looking at recent comments on #803, it may be a good idea to add a few words about static vs non-static nested classes in the user manual.

[remkop]

@pedrolamarao, @deining I added some CAUTION admonitions to the Custom Parameter Processing and Multi Parameter Type Converters sections. Feel free to suggest more if you can think of any.

[deining]

Feel free to suggest more if you can think of any.

With PR #1307, I added two more CAUTION admonitions.

Two more thoughts on this:

• 21.5. Custom Factory: what about rewriting / adding the code sample in this chapter in the sense that it shows how to use a custom factory to make non static inner class work? This way, the sample (and the whole chapter) would become more meaningful IMHO.
• Without any doubt, the chosen approach of adding CAUTION admonitions is valid and helpful. My initial thought was different, though, I reflected on adding a new chapter 22. Known limitations and caveats that list (amongst others) the limitation of nested classes. Despite we have admonitions now in various places, I tend to think this still would be an enhancement to the documentation.

Just my two cents ...

[remkop]

I reflected on adding a new chapter 22. Known limitations and caveats that list (amongst others) the limitation of nested classes. Despite we have admonitions now in various places, I tend to think this still would be an enhancement to the documentation.

Yes, I like that idea. Between "Tips & Tricks" and "Dependency Injection" is probably a good insertion point.
I like short titles, how about just "Known Limitations"?

Some limitations are already documented, like Variable Arity Limitations, Argument Group Limitations, Limitations of Variable Interpolation, default values for annotated fields, interactive positional parameters, Optional Parameter Limitations.

For some of these it may make sense to repeat the content or expand on the content with an example, for other limitations that already have extensive documentation, perhaps we can just link from the new chapter to the existing section.

21.5. Custom Factory: what about rewriting / adding the code sample in this chapter in the sense that it shows how to use a custom factory to make non static inner class work? This way, the sample (and the whole chapter) would become more meaningful IMHO.

Can we come up with a good example of something where a static nested class would not be enough? (I am still struggling to find a good use case...) I am happy to add examples to illustrate the role of the IFactory interface in picocli, but I also like to give examples that people can relate to.

With PR #1307, I added two more CAUTION admonitions.

Merged. Thank you! 🙏

[remkop]

I separated out #1317 for an additional "Known Limitations" chapter.

After some more pondering I don't think that creating custom factories for non-static nested classes is something that should be encouraged, so I prefer to not mention this idea in the Custom Factories section.
If a subcommand needs to access the parent command's state, there are several simpler alternatives:

• use a @Command annotated method
• use the @ParentCommand annotation
• use the @Spec annotation

@deining Are you okay to close this ticket?