Skip to content

Commit 05bc0ad

Browse files
committed
Support GroupCommand interface without static groupCommands annotation (#542)
Commands implementing GroupCommand with dynamic subcommands via getCommands() were broken when @CommandDefinition had no static groupCommands={} attribute. The instanceof GroupCommand check was gated behind provider.isGroupCommand() / groupCommands.length > 0, so getCommands() was never called. AeshCommandContainerBuilder changes: - Check instanceof GroupCommand independently of the annotation - A command is a group if EITHER it has static groupCommands={} OR it implements GroupCommand OR both - Static and dynamic subcommands can now be combined — both are registered (was previously either/or via if/else) - Both processor path (buildFromProvider) and reflection path (doGenerateCommandLineParser) fixed AeshAnnotationProcessor changes: - New implementsGroupCommand(TypeElement) method walks the type hierarchy (interfaces + superclass) to detect GroupCommand - isGroup now true when groupCommands is non-empty OR the class implements GroupCommand - The generated isGroupCommand() returns true for both cases Fixes: #542 980 aesh tests + 63 processor tests, 0 failures.
1 parent f375ba3 commit 05bc0ad

4 files changed

Lines changed: 461 additions & 24 deletions

File tree

aesh-processor/src/main/java/org/aesh/processor/AeshAnnotationProcessor.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,9 @@ private void generateProvider(TypeElement commandElement) throws IOException {
290290
}
291291
String fullMetadataName = packageName.isEmpty() ? metadataClassName : packageName + "." + metadataClassName;
292292

293-
// Check if @CommandDefinition has groupCommands via annotation mirror
294-
// (direct access triggers MirroredTypesException at compile time)
295-
boolean isGroup = hasGroupCommands(commandElement);
293+
// A command is a group if it declares groupCommands={...} in the annotation
294+
// OR if it implements the GroupCommand interface (#542)
295+
boolean isGroup = hasGroupCommands(commandElement) || implementsGroupCommand(commandElement);
296296

297297
List<VariableElement> fields = collectFields(commandElement);
298298

@@ -336,6 +336,29 @@ private boolean hasGroupCommands(TypeElement element) {
336336
return false;
337337
}
338338

339+
/**
340+
* Check if the given type element implements {@code org.aesh.command.GroupCommand},
341+
* either directly or through its supertypes.
342+
*/
343+
private boolean implementsGroupCommand(TypeElement element) {
344+
String groupCommandName = "org.aesh.command.GroupCommand";
345+
for (TypeMirror iface : element.getInterfaces()) {
346+
// Check raw type name (strip generics)
347+
String ifaceName = typeUtils.erasure(iface).toString();
348+
if (ifaceName.equals(groupCommandName))
349+
return true;
350+
}
351+
// Check superclass hierarchy
352+
TypeMirror superclass = element.getSuperclass();
353+
if (superclass.getKind() != javax.lang.model.type.TypeKind.NONE) {
354+
Element superElement = typeUtils.asElement(superclass);
355+
if (superElement instanceof TypeElement) {
356+
return implementsGroupCommand((TypeElement) superElement);
357+
}
358+
}
359+
return false;
360+
}
361+
339362
private void collectPrivateFieldsForReflectConfig(List<VariableElement> fields) {
340363
for (VariableElement field : fields) {
341364
if (field.getModifiers().contains(Modifier.PRIVATE) && hasAeshAnnotation(field)) {

aesh-processor/src/test/java/org/aesh/processor/ProcessorTest.java

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,132 @@ public void testUnifiedCommandDefinitionWithGroupCommands() throws Exception {
446446
assertEquivalence(commandClass, metadataClass);
447447
}
448448

449+
// --- Test: GroupCommand interface detection (#542) ---
450+
451+
private static final String GROUP_COMMAND_INTERFACE_SOURCE = "package test;\n" +
452+
"\n" +
453+
"import java.util.List;\n" +
454+
"import java.util.Collections;\n" +
455+
"import org.aesh.command.Command;\n" +
456+
"import org.aesh.command.CommandDefinition;\n" +
457+
"import org.aesh.command.CommandResult;\n" +
458+
"import org.aesh.command.GroupCommand;\n" +
459+
"import org.aesh.command.invocation.CommandInvocation;\n" +
460+
"\n" +
461+
"@CommandDefinition(name = \"dynamic-group\", description = \"Dynamic group\")\n" +
462+
"public class DynamicGroupCommand implements GroupCommand<CommandInvocation> {\n" +
463+
" @Override\n" +
464+
" public List<Command<CommandInvocation>> getCommands() {\n" +
465+
" return Collections.emptyList();\n" +
466+
" }\n" +
467+
" @Override\n" +
468+
" public CommandResult execute(CommandInvocation ci) {\n" +
469+
" return CommandResult.SUCCESS;\n" +
470+
" }\n" +
471+
"}\n";
472+
473+
@Test
474+
public void testGroupCommandInterfaceDetected() throws Exception {
475+
// A command implementing GroupCommand (no groupCommands={} in annotation)
476+
// should still be detected as a group command by the processor (#542)
477+
CompilationResult result = compileWithProcessor(
478+
new InMemorySource("test.DynamicGroupCommand", GROUP_COMMAND_INTERFACE_SOURCE));
479+
assertTrue("Compilation should succeed: " + result.diagnostics, result.success);
480+
481+
Class<?> metadataClass = result.classLoader.loadClass("test.DynamicGroupCommand_AeshMetadata");
482+
CommandMetadataProvider provider = (CommandMetadataProvider) metadataClass.newInstance();
483+
484+
assertTrue("Should be detected as a group command via GroupCommand interface",
485+
provider.isGroupCommand());
486+
// groupCommandClasses() should be empty (no static subcommands in annotation)
487+
assertEquals("No static group commands", 0, provider.groupCommandClasses().length);
488+
}
489+
490+
private static final String COMBINED_GROUP_INTERFACE_SOURCE = "package test;\n" +
491+
"\n" +
492+
"import java.util.List;\n" +
493+
"import java.util.Collections;\n" +
494+
"import org.aesh.command.Command;\n" +
495+
"import org.aesh.command.CommandDefinition;\n" +
496+
"import org.aesh.command.CommandResult;\n" +
497+
"import org.aesh.command.GroupCommand;\n" +
498+
"import org.aesh.command.invocation.CommandInvocation;\n" +
499+
"\n" +
500+
"@CommandDefinition(name = \"combined\", description = \"Combined group\",\n" +
501+
" groupCommands = {test.SubCommand1.class})\n" +
502+
"public class CombinedGroupCommand implements GroupCommand<CommandInvocation> {\n" +
503+
" @Override\n" +
504+
" public List<Command<CommandInvocation>> getCommands() {\n" +
505+
" return Collections.emptyList();\n" +
506+
" }\n" +
507+
" @Override\n" +
508+
" public CommandResult execute(CommandInvocation ci) {\n" +
509+
" return CommandResult.SUCCESS;\n" +
510+
" }\n" +
511+
"}\n";
512+
513+
@Test
514+
public void testGroupCommandInterfaceWithStaticGroupCommands() throws Exception {
515+
// Combined: GroupCommand interface AND groupCommands={} in annotation (#542)
516+
CompilationResult result = compileWithProcessor(
517+
new InMemorySource("test.SubCommand1", SUB_COMMAND1_SOURCE),
518+
new InMemorySource("test.CombinedGroupCommand", COMBINED_GROUP_INTERFACE_SOURCE));
519+
assertTrue("Compilation should succeed: " + result.diagnostics, result.success);
520+
521+
Class<?> metadataClass = result.classLoader.loadClass("test.CombinedGroupCommand_AeshMetadata");
522+
CommandMetadataProvider provider = (CommandMetadataProvider) metadataClass.newInstance();
523+
524+
assertTrue("Should be a group command (both interface and annotation)",
525+
provider.isGroupCommand());
526+
assertEquals("Should have 1 static subcommand from annotation",
527+
1, provider.groupCommandClasses().length);
528+
}
529+
530+
private static final String INHERITED_GROUP_INTERFACE_SOURCE = "package test;\n" +
531+
"\n" +
532+
"import java.util.List;\n" +
533+
"import java.util.Collections;\n" +
534+
"import org.aesh.command.Command;\n" +
535+
"import org.aesh.command.CommandResult;\n" +
536+
"import org.aesh.command.GroupCommand;\n" +
537+
"import org.aesh.command.invocation.CommandInvocation;\n" +
538+
"\n" +
539+
"public abstract class BaseGroupCommand implements GroupCommand<CommandInvocation> {\n" +
540+
" @Override\n" +
541+
" public List<Command<CommandInvocation>> getCommands() {\n" +
542+
" return Collections.emptyList();\n" +
543+
" }\n" +
544+
"}\n";
545+
546+
private static final String CONCRETE_GROUP_COMMAND_SOURCE = "package test;\n" +
547+
"\n" +
548+
"import org.aesh.command.CommandDefinition;\n" +
549+
"import org.aesh.command.CommandResult;\n" +
550+
"import org.aesh.command.invocation.CommandInvocation;\n" +
551+
"\n" +
552+
"@CommandDefinition(name = \"inherited\", description = \"Inherited group\")\n" +
553+
"public class InheritedGroupCommand extends BaseGroupCommand {\n" +
554+
" @Override\n" +
555+
" public CommandResult execute(CommandInvocation ci) {\n" +
556+
" return CommandResult.SUCCESS;\n" +
557+
" }\n" +
558+
"}\n";
559+
560+
@Test
561+
public void testGroupCommandInterfaceInherited() throws Exception {
562+
// GroupCommand implemented via superclass, not directly (#542)
563+
CompilationResult result = compileWithProcessor(
564+
new InMemorySource("test.BaseGroupCommand", INHERITED_GROUP_INTERFACE_SOURCE),
565+
new InMemorySource("test.InheritedGroupCommand", CONCRETE_GROUP_COMMAND_SOURCE));
566+
assertTrue("Compilation should succeed: " + result.diagnostics, result.success);
567+
568+
Class<?> metadataClass = result.classLoader.loadClass("test.InheritedGroupCommand_AeshMetadata");
569+
CommandMetadataProvider provider = (CommandMetadataProvider) metadataClass.newInstance();
570+
571+
assertTrue("Should detect GroupCommand via superclass",
572+
provider.isGroupCommand());
573+
}
574+
449575
// --- Test: @Mixin support ---
450576

451577
private static final String LOGGING_MIXIN_SOURCE = "package test;\n" +

aesh/src/main/java/org/aesh/command/impl/container/AeshCommandContainerBuilder.java

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,22 @@ private AeshCommandContainer<CI> buildFromProvider(CommandMetadataProvider provi
8888
AeshCommandLineParser<CI> parser = new AeshCommandLineParser<>(processedCommand);
8989
AeshCommandContainer<CI> container = new AeshCommandContainer<>(parser);
9090

91-
if (provider.isGroupCommand()) {
91+
boolean isGroup = provider.isGroupCommand() || command instanceof GroupCommand;
92+
if (isGroup) {
9293
// Set child resolver so lazy children use the same builder (#517)
9394
setChildResolverFromBuilder(parser);
95+
// Add static subcommands from annotation groupCommands={...}
96+
if (provider.isGroupCommand()) {
97+
for (Class<? extends Command> groupClazz : provider.groupCommandClasses()) {
98+
String childName = getCommandName(groupClazz);
99+
if (childName != null) {
100+
addLazyChildWithAliases(container, childName, groupClazz);
101+
} else {
102+
container.addChild(create(groupClazz));
103+
}
104+
}
105+
}
106+
// Add dynamic subcommands from GroupCommand.getCommands() (#542)
94107
if (command instanceof GroupCommand) {
95108
List<Command<CI>> commands = ((GroupCommand<CI>) command).getCommands();
96109
if (commands != null) {
@@ -104,15 +117,6 @@ private AeshCommandContainer<CI> buildFromProvider(CommandMetadataProvider provi
104117
container.addChild(sub);
105118
}
106119
}
107-
} else {
108-
for (Class<? extends Command> groupClazz : provider.groupCommandClasses()) {
109-
String childName = getCommandName(groupClazz);
110-
if (childName != null) {
111-
addLazyChildWithAliases(container, childName, groupClazz);
112-
} else {
113-
container.addChild(create(groupClazz));
114-
}
115-
}
116120
}
117121
}
118122

@@ -123,7 +127,8 @@ private AeshCommandContainer<CI> doGenerateCommandLineParser(Command<CI> command
123127
Class<Command<CI>> clazz = (Class<Command<CI>>) commandObject.getClass();
124128
CommandDefinition command = clazz.getAnnotation(CommandDefinition.class);
125129
if (command != null) {
126-
boolean isGroup = command.groupCommands().length > 0;
130+
boolean hasStaticGroup = command.groupCommands().length > 0;
131+
boolean isGroup = hasStaticGroup || commandObject instanceof GroupCommand;
127132
ProcessedCommand<Command<CI>, CI> processedCommand = ProcessedCommandBuilder.<Command<CI>, CI> builder()
128133
.name(command.name())
129134
.activator(command.activator())
@@ -154,6 +159,19 @@ private AeshCommandContainer<CI> doGenerateCommandLineParser(Command<CI> command
154159
setChildResolverFromBuilder(groupParser);
155160
AeshCommandContainer<CI> groupContainer = new AeshCommandContainer<>(groupParser);
156161

162+
// Add static subcommands from annotation groupCommands={...}
163+
if (hasStaticGroup) {
164+
for (Class<? extends Command> groupClazz : command.groupCommands()) {
165+
String childName = getCommandName(groupClazz);
166+
if (childName != null) {
167+
addLazyChildWithAliases(groupContainer, childName, groupClazz);
168+
} else {
169+
Command<CI> groupInstance = (Command<CI>) ReflectionUtil.newInstance(groupClazz);
170+
groupContainer.addChild(doGenerateCommandLineParser(groupInstance));
171+
}
172+
}
173+
}
174+
// Add dynamic subcommands from GroupCommand.getCommands() (#542)
157175
if (commandObject instanceof GroupCommand) {
158176
List<Command<CI>> commands = ((GroupCommand<CI>) commandObject).getCommands();
159177
if (commands != null) {
@@ -167,16 +185,6 @@ private AeshCommandContainer<CI> doGenerateCommandLineParser(Command<CI> command
167185
groupContainer.addChild(sub);
168186
}
169187
}
170-
} else {
171-
for (Class<? extends Command> groupClazz : command.groupCommands()) {
172-
String childName = getCommandName(groupClazz);
173-
if (childName != null) {
174-
addLazyChildWithAliases(groupContainer, childName, groupClazz);
175-
} else {
176-
Command<CI> groupInstance = (Command<CI>) ReflectionUtil.newInstance(groupClazz);
177-
groupContainer.addChild(doGenerateCommandLineParser(groupInstance));
178-
}
179-
}
180188
}
181189
return groupContainer;
182190
}

0 commit comments

Comments
 (0)