Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typo in error message from AbstractCommandSpecProcessor#extractTypedMember(Element, String)? #1476

Closed
rgoldberg opened this issue Nov 23, 2021 · 6 comments
Labels
theme: annotation-proc An issue or change related to the annotation processor type: enhancement ✨

Comments

@rgoldberg
Copy link
Contributor

rgoldberg commented Nov 23, 2021

I think there's another typo in the error message from AbstractCommandSpecProcessor#extractTypedMember(Element, String).

It currently outputs:

"Can only process %s annotations on fields, methods and method parameters, not on %s"

But the element.getKind() == ElementKind.PARAMETER check is commented, so I think the correct message should be:

"Can only process %s annotations on fields and methods, not on %s"

Or maybe the element.getKind() == ElementKind.PARAMETER should be uncommented…

As an aside, I also think the code should use a switch statement instead of chained ifs.

I can make a PR, but just wanted to check first.

@remkop
Copy link
Owner

remkop commented Nov 23, 2021

Thank you for raising this!
I'll take a look when I get to my PC.

@remkop remkop added theme: annotation-proc An issue or change related to the annotation processor type: enhancement ✨ labels Nov 25, 2021
@remkop
Copy link
Owner

remkop commented Nov 25, 2021

I had a quick look, and it seems that @Option-annotated method parameters are handled in AbstractCommandSpecProcessor#buildOptionsAndPositionalsFromMethodParameters.

I suspect that the extractTypedMember method is not called for annotated elements other than fields and methods.
There is a test for @Option-annotated method parameters: picocli.annotation.processing.tests.AbstractCommandSpecProcessorTest#testCommandWithMixin, and this test does not result in a compiler error.

I did not look in more detail, however.

I am going to leave this error message as it is for now; the error message itself is correct (for example, if a class is annotated with @Option, this error message would be shown, and it would be useful to have it state all valid places where the @Option annotation can be used).

@rgoldberg
Copy link
Contributor Author

I got the error when I annotated a Kotlin primary constructor parameter with @Parameters:

error: Cannot only process @Parameters annotations on fields, methods and method parameters, not on PARAMETER
    java.lang.String software, @org.jetbrains.annotations.NotNull

It's saying that @Parameters can only be processed on a, b & method parameters, but not on PARAMETER. Since PARAMETER is a method parameter, the message is incorrect.

@remkop
Copy link
Owner

remkop commented Nov 26, 2021

I got the error when I annotated a Kotlin primary constructor parameter with @Parameters:

Oh I see, interesting. So this is related to #1195.

You could try uncommenting the || element.getKind() == ElementKind.PARAMETER) { condition in extractTypedMember.

Also, (and this may not be important, but) buildOptionsAndPositionalsFromMethodParameters has some extra logic for calculating position for @Parameters elements that do not have an explicit index set. The extractTypedMember does not have that logic. (But it may not be needed, not sure). If it is possible to invoke buildOptionsAndPositionalsFromMethodParameters for constructors, that may be good - but this is just a random thought, I have not looked at the implications of doing this.

@rgoldberg
Copy link
Contributor Author

rgoldberg commented Nov 27, 2021

Yep, I saw this while investigating #1195.

I'm not trying to change this now to implement #1195.

The current error output must be incorrect, since it says that the annotation can be processed on method parameters, but then says it cannot be processed on PARAMETER. But PARAMETER is a method parameter.

Unless I've completely missed something…

@remkop
Copy link
Owner

remkop commented Nov 27, 2021

Yes, good point.
I improved the error message to clarify that currently only parameters in @Command-annotated methods are supported.

@remkop remkop closed this as completed Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: annotation-proc An issue or change related to the annotation processor type: enhancement ✨
Projects
None yet
Development

No branches or pull requests

2 participants