From 2ad7c8ea8f3c890ae2736b49f6e9a8de6df96775 Mon Sep 17 00:00:00 2001 From: Sharang Gupta Date: Tue, 6 Oct 2026 22:24:00 +0100 Subject: [PATCH] [#2430] Bugfix: annotation processor should allow `@Spec` in ArgGroup classes The annotation processor raised the compile error "@Spec must be enclosed in a @Command, or in a class that implements IVersionProvider" when a `@Spec`-annotated field was declared in an `@ArgGroup` class. At runtime picocli has supported `@Spec` in ArgGroup classes since 4.6, so the compile-time check rejected valid code. Before 4.7.7 the check happened to pass for ArgGroup classes because options inside such a class were looked up by the `@ArgGroup` variable rather than by its type, missed the group, and fell through to `getOrCreateCommandSpecForArg`, which registered a `CommandSpec` for the ArgGroup class as a side effect. 4.7.7 fixed that lookup, and the latent check started to fire. Remove the check, as the maintainer suggested in the issue: `@Spec` is valid in several places that are not commands (ArgGroup classes, mixins, IVersionProvider implementations), and picocli injects the `CommandSpec` at runtime without the processor needing to attach it to a model object. Elements that are not enclosed in a command are now traced at FINE level instead of being reported as errors. Closes #2430 --- .../processing/tests/Issue2430Test.java | 37 +++++++++++ .../resources/picocli/issue2430/Main.java | 63 +++++++++++++++++++ .../picocli/issue2430/SpecInArgGroup.java | 22 +++++++ .../AbstractCommandSpecProcessor.java | 20 +----- 4 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 picocli-codegen-tests-java9plus/src/test/java/picocli/annotation/processing/tests/Issue2430Test.java create mode 100644 picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/Main.java create mode 100644 picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/SpecInArgGroup.java diff --git a/picocli-codegen-tests-java9plus/src/test/java/picocli/annotation/processing/tests/Issue2430Test.java b/picocli-codegen-tests-java9plus/src/test/java/picocli/annotation/processing/tests/Issue2430Test.java new file mode 100644 index 000000000..9bdc720bb --- /dev/null +++ b/picocli-codegen-tests-java9plus/src/test/java/picocli/annotation/processing/tests/Issue2430Test.java @@ -0,0 +1,37 @@ +package picocli.annotation.processing.tests; + +import com.google.testing.compile.Compilation; +import com.google.testing.compile.JavaFileObjects; +import org.junit.Test; + +import javax.annotation.processing.Processor; + +import static com.google.testing.compile.CompilationSubject.assertThat; +import static com.google.testing.compile.Compiler.javac; + +public class Issue2430Test +{ + @Test + public void testSpecInArgGroupClassUsedViaMixin() { + Processor processor = new AnnotatedCommandSourceGeneratorProcessor(); + Compilation compilation = + javac() + .withProcessors(processor) + .compile(JavaFileObjects.forResource( + "picocli/issue2430/Main.java")); + + assertThat(compilation).succeeded(); + } + + @Test + public void testSpecInArgGroupClassUsedDirectlyInCommand() { + Processor processor = new AnnotatedCommandSourceGeneratorProcessor(); + Compilation compilation = + javac() + .withProcessors(processor) + .compile(JavaFileObjects.forResource( + "picocli/issue2430/SpecInArgGroup.java")); + + assertThat(compilation).succeeded(); + } +} diff --git a/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/Main.java b/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/Main.java new file mode 100644 index 000000000..d13819cdd --- /dev/null +++ b/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/Main.java @@ -0,0 +1,63 @@ +package picocli.issue2430; + +import picocli.CommandLine; +import picocli.CommandLine.ArgGroup; +import picocli.CommandLine.Command; +import picocli.CommandLine.Mixin; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.Option; +import picocli.CommandLine.ParameterException; +import picocli.CommandLine.Spec; + +import java.util.concurrent.Callable; + +@Command(name = "main") +class Main implements Callable { + + static class Options { + @Spec + CommandSpec spec; + + int option1 = 1; + int option2 = 2; + + @Option(names = "-o1") + void setOption1(int value) { + if (value > 10) { + throw new ParameterException(spec.commandLine(), "option1 must be <= 10"); + } + this.option1 = value; + } + + @Option(names = "-o2") + void setOption2(int value) { + if (value > 10) { + throw new ParameterException(spec.commandLine(), "option2 must be <= 10"); + } + this.option2 = value; + } + } + + static class SharedOptions { + + @ArgGroup(exclusive = false, heading = "Shared options%n") + Options options = new Options(); + } + + @Mixin + SharedOptions sharedOptions = new SharedOptions(); + + @Override + public Integer call() throws Exception { + System.out.printf("Hello world: o1 = %s, o2 = %s%n", + sharedOptions.options.option1, + sharedOptions.options.option2 + ); + return 0; + } + + public static void main(String[] args) { + int exitCode = new CommandLine(new Main()).execute(args); + System.exit(exitCode); + } +} diff --git a/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/SpecInArgGroup.java b/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/SpecInArgGroup.java new file mode 100644 index 000000000..2bb215188 --- /dev/null +++ b/picocli-codegen-tests-java9plus/src/test/resources/picocli/issue2430/SpecInArgGroup.java @@ -0,0 +1,22 @@ +package picocli.issue2430; + +import picocli.CommandLine.ArgGroup; +import picocli.CommandLine.Command; +import picocli.CommandLine.Model.CommandSpec; +import picocli.CommandLine.Option; +import picocli.CommandLine.Spec; + +@Command(name = "spec-in-arggroup") +class SpecInArgGroup { + + static class Options { + @Spec + CommandSpec spec; + + @Option(names = "-x") + int x; + } + + @ArgGroup(exclusive = false) + Options options = new Options(); +} diff --git a/picocli-codegen/src/main/java/picocli/codegen/annotation/processing/AbstractCommandSpecProcessor.java b/picocli-codegen/src/main/java/picocli/codegen/annotation/processing/AbstractCommandSpecProcessor.java index b5b7576a2..e7fc22866 100644 --- a/picocli-codegen/src/main/java/picocli/codegen/annotation/processing/AbstractCommandSpecProcessor.java +++ b/picocli-codegen/src/main/java/picocli/codegen/annotation/processing/AbstractCommandSpecProcessor.java @@ -929,23 +929,9 @@ private void connectModel(AbstractCommandSpecProcessor proc) { logger.fine("Adding " + entry + " to commandSpec " + commandSpec1); commandSpec1.addSpecElement(entry.getValue()); } else { - Element enclosingElement = entry.getKey().getEnclosingElement(); - if (enclosingElement.getKind() == ElementKind.CLASS || enclosingElement.getKind() == ENUM) { - TypeMirror typeMirror = enclosingElement.asType(); - TypeElement typeElement = (TypeElement) ((DeclaredType) typeMirror).asElement(); - List interfaces = typeElement.getInterfaces(); - boolean valid = false; - for (TypeMirror interf : interfaces) { - if (interf.toString().equals("picocli.CommandLine.IVersionProvider")) { - valid = true; - } - } - if (!valid) { - proc.error(entry.getKey(), "@Spec must be enclosed in a @Command, or in a class that implements IVersionProvider but was %s: %s", entry.getKey().getEnclosingElement(), entry.getKey().getEnclosingElement().getSimpleName()); - } - } else { - proc.error(entry.getKey(), "@Spec must be enclosed in a @Command, but was %s: %s", entry.getKey().getEnclosingElement(), entry.getKey().getEnclosingElement().getSimpleName()); - } + // #2430 @Spec is also valid outside a @Command, for example in ArgGroup classes + // or in IVersionProvider implementations; picocli injects the CommandSpec at runtime + logger.fine("Not adding " + entry + " to any commandSpec: enclosing element " + entry.getKey().getEnclosingElement() + " is not a @Command"); } } for (Map.Entry entry : parentCommandElements.entrySet()) {