Introduce new command DSL

[piotrooo]

Below is a new, improved DSL that addresses #1127. You’ll find the usage of individual methods (all of them are listed) to make it easier to review everything clearly.

CommandRegistration.builder()
	.command("command")
	.description("description")
	.group("group")

	.interactionMode(InteractionMode.NONINTERACTIVE)
	.isInteractive()
	.isNonInteractive()
	.availability(Availability::available)

	.hidden(true)
	.hidden()

	.withOption(option -> option
			.longNames("arg")
			.shortNames('a')

			.type(String.class)
			.type(ResolvableType.forType(String.class))

			.description("some arg")
			.label("label")
			.defaultValue("defaultValue")
			.nameModifier(String::toUpperCase)

			.required()
			.required(true)

			.position(0)

			.arity(1, 2)
			.arity(OptionArity.ZERO_OR_MORE)

			.completion(completionContext -> List.of())
	)
	.defaultOptionNameModifier(String::toLowerCase)

	.withAlias(alias -> alias
			.command("alias")
			.group("Alias Group")
	)
	.withTarget(target -> target
			.function(function -> function)
			.consumer(commandContext -> {})

			.method(new Object(), "method", String.class)
			.method(new Object(), "method")
	)
	.withExitCode(exitCode -> exitCode
			.map(RuntimeException.class, 1)
			.map(throwable -> 2)
	)
	.withErrorHandling(errorHandling -> errorHandling
			.resolver(ex -> CommandHandlingResult.of("msg"))
	)

	.build();

Important

Since command registration is the heart of Spring Shell, it's very important to carefully review the contracts of the introduced changes. Modifying this API in the future may be problematic, so we need to be sure that this structure is acceptable, as it may stay with us for a long time.

However, when I look at the resulting code — and the existing code in the codebase — I'm not entirely satisfied. The code feels quite clumsy. 😭

I have a few topics I'd like to bring up.

Is the naming of these objects good?

I see an analogy here to BeanDefinition and BeanDefinitionRegistry. Right now, we have the name CommandRegistration. But what exactly is this mysterious Registration? I have the impression that it's not a good name. It’s not very descriptive and might be misleading.
Maybe we should approach it similarly to the Spring Framework itself and have definitions and registries for commands?

In the definition, we could have a command object with all its metadata — such as names, aliases, and options.

Should we use interfaces?

I'm not sure what the benefit is of having CommandRegistration as an interface (and the rest of objects too). I don't really feel that this approach fits well.

If we had multiple implementations (1..n) of different CommandRegistrations, then an interface might make sense. Here, it seems unnecessary.

Another question, how should the objects that hold command definition information look?

Should we use the Spec objects everywhere, or only during the build phase — where the result would be an object holder?
For example: OptionSpec could create an OptionDefinition object (which could be a simple record).

Aggressiveness of changes

I'm also not sure how far-reaching these changes should be. We're changing the baseline — but are the modifications I’ve introduced acceptable? Could we possibly discuss further, larger changes? Do we need to maintain backward compatibility?

Summary

Overall, this image perfectly captures how I feel when walking through the classes and trying to understand how commands are created.

Caution

Without good and clear manual command registration, it will be very difficult to build reliable automatic detection and registration later on.

I'd be happy 🙏 to continue driving this topic, but I need answers to the above questions so I don't go down a dead-end path — and to know how far I can go with the refactor.

[fmbenhassine]

Thank you for the PR!

I see an analogy here to BeanDefinition and BeanDefinitionRegistry. Right now, we have the name CommandRegistration. But what exactly is this mysterious Registration? I have the impression that it's not a good name. It’s not very descriptive and might be misleading.
Maybe we should approach it similarly to the Spring Framework itself and have definitions and registries for commands?

I agree, the name is confusing (naming is hard..). I recently renamed CommandCatalog to CommandRegistry (see 3d4dce2). So If we rename Command to CommandDefinition, we would end up with CommandDefinition and CommandDefinitionRegistry just like BeanDefinition and BeanDefinitionRegistry (which is a good thing).

However, as mentioned in #1209 (reply in thread), we might not need to rename Command to CommandDefinition, Command is already a good name and very appealing as you mentioned, so it can be well used in a CommandRegistry (after the recent rename). Do you agree?

We can still use CommandDefinition (similar to CommandSpec in Picocli for example) with the DSL to create Command objects. But that is a nice-to-have feature, not the single entry point.

I think if we redesign Command to contain a command definition (and not just be an empty interface), we are good to go to use that in place of CommandRegistration.

I'm not sure what the benefit is of having CommandRegistration as an interface (and the rest of objects too). I don't really feel that this approach fits well. If we had multiple implementations (1..n) of different CommandRegistrations, then an interface might make sense. Here, it seems unnecessary.

Yes, I am thinking of introducing a new programming model based on Command and ditch CommandRegistry altogether (but I need to try that beforehand, you can give it a try if you want as well).

Another question, how should the objects that hold command definition information look?

I think of something as simple as:

public interface Command {

    String getName();
    String getDescription();

    // other command properties
  
    // a method to define the business logic of the command
    // input/output types can be reviewed here, I am thinking of CommandExitCode ?
    String execute(String[] args) throws Exception;

}

Should we use the Spec objects everywhere, or only during the build phase — where the result would be an object holder? For example: OptionSpec could create an OptionDefinition object (which could be a simple record).

No need for Specs and Definitions, only Definitions. And that would be only used by the DSL.

I'm also not sure how far-reaching these changes should be. We're changing the baseline — but are the modifications I’ve introduced acceptable? Could we possibly discuss further, larger changes? Do we need to maintain backward compatibility?

Yes, the modifications you introduced are acceptable, and there is no need to maintain backward compatibility since we are designing a major release.