Argument exceptions2

[willkroboth]

This PR remakes #370. The original argument-exceptions branch had fallen quite far behind, so instead of trying to rebase that mess, I just re-coded it all here, with a few modifications. I had mostly stopped working on that because I wasn't sure how to solve the problem where developers had to interpret the exception's String to figure out why it was thrown. I think I have finally figured that out, but, I'm getting ahead of myself. There is a lot to explain in this PR before getting to that.

(I feel like a lot of the PRs I've been writing lately have too much reading, but it has to happen. I'm partly glad I learned about the <details> tag because it makes all this easier to organize, but it also enables me to write too much :P.)

What is this?

When using the CommandAPI, there are two parsing steps. First, Brigadier argument nodes use various implementations of ArgumentType to turn the command String into Minecraft objects, which are stored into a CommandContext object. I refer to this as the "initial parse". Once a valid executable command is found, Brigadier runs the CommandAPI's executor. The CommandAPI parses the CommandContext and creates Bukkit objects, which are stored in a CommandArguments object. I call this the "argument parse". Once all the Arguments are processed, the CommandAPI calls the developer's executor.

Both of these parsing steps may throw CommandSyntaxExceptions. This PR adds API methods and the necessary backend to catch exceptions in both steps and pass them to the developer if they want to implement special handling. Developers can inspect the information given to them, and use this opportunity to throw a custom exception or supply a replacement value.

For example, here are the two test cases I added as proof of concept:

new IntegerArgument("rating", 0, 10)
        .withInitialParseExceptionHandler(context ->
                switch (context.exceptionInformation().type()) {
                    // Integer too low, move to 0
                    case INTEGER_TOO_LOW -> context.exceptionInformation().minimum();
                    // Integer too high, cap to 10
                    case INTEGER_TOO_HIGH -> context.exceptionInformation().maximum();
                    // Integer wasn't entered, use original exception
                    default -> throw context.exception();
                }),

new ListArgumentBuilder<Player>("players")
        .withList(() -> (Collection<Player>) Bukkit.getOnlinePlayers())
        .withMapper(Player::getName)
        .buildGreedy()
        .withArgumentParseExceptionHandler(context -> {
            throw switch (context.exceptionInformation().type()) {
                case INVALID_ITEM -> CommandAPI.failWithString(
                        "Could not find player: " +  context.exceptionInformation().currentItem()
                );
                default -> context.exception();
            };
        })

The first example shows an IntegerArgument with a minimum value of 0 and a maximum value of 10. Usually, an exception is thrown if the given int is outside the bounds, and this bounds check happens during the initial parse. Instead, an exception handler is used to round the value to the closest valid int. For example, if 20 was given, the parse result would become 10. If -5 was given, 0 would be substituted. If the exception was thrown because there simply wasn't a valid int given, the original exception is re-thrown. While we would prefer the command sender to give us an int between 0 and 10, we can still recover with an exception handler and use a reasonable value instead.

The second example is a ListArgument that can be given any player on the server. Usually, an exception that says Item is not allowed in list is thrown when a String is given that can't be turned into a Player. Instead, an exception handler replaces the exception with Could not find player: (String that wasn't a Player name). This exception message makes a lot more sense in this situation, and developers can generally expand upon the exceptions defined by the CommandAPI.

How does it work?

The API classes for this are mostly duplicates of each other -- one for the initial parse and again for the argument parse -- so I'll talk about those together. Intercepting exceptions works differently, so that is separated. Here's all the parts that make this work:

ParseExceptionContext records

The records InitialParseExceptionContext and ArgumentParseExceptionContext store any available information about the exception, similar to ExecutionInfo for command executors. Those records look like this:

public record InitialParseExceptionContext<ExceptionInformation>(
        WrapperCommandSyntaxException exception,
        ExceptionInformation exceptionInformation,
        WrapperStringReader stringReader) {
}

public record ArgumentParseExceptionContext<Raw, ExceptionInformation, CommandSender>(
        WrapperCommandSyntaxException exception,
        ExceptionInformation exceptionInformation,
        CommandSender sender,
        Raw input,
        CommandArguments previousArguments) {
}

Both records store the CommandSyntaxException that caused them as a WrapperCommandSyntaxException so the developer doesn't have to import Brigadier. They also both have an ExceptionInformation object. ExceptionInformation is a generic type for both classes, and its implementation details will be explained later. It just allows these contexts to be flexible and provide argument-specific information about the exception.

InitialParseExceptionContext also stores a WrapperStringReader, which wraps Brigadier's StringReader in a similar way to WrapperCommandSyntaxException. This StringReader is the one the ArgumentType was using when the exception failed to parse, which allows developers to inspect the command String around the failed parse.

ArgumentParseExceptionContext has 3 additional fields. sender is the source of the command, input is the result of Brigadier's parse, and previousArguments stores the arguments that have already been parsed. Note that since these records exist in commandapi-core, CommandSender is a generic type and does not necessarily refer to Bukkit's CommandSender. input also uses the generic type Raw, since it depends on the Argument being used.

I believe these records sum up all the information that could be given to the developer. However, since they are records, they could easily be expanded in the future to include any other information.

ParseExceptionHandler FunctionalInterfaces

InitialParseExceptionHandler and ArgumentParseExceptionHandler are FunctionalInterfaces that developers implement to define their exception handling behavior, similar to CommandExecutor for command executors. They look like this:

@FunctionalInterface
public interface InitialParseExceptionHandler<T, ExceptionInformation> {
    T handleException(InitialParseExceptionContext<ExceptionInformation> context) throws WrapperCommandSyntaxException;
}

@FunctionalInterface
public interface ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender> {
    T handleException(ArgumentParseExceptionContext<Raw, ExceptionInformation, CommandSender> context) throws WrapperCommandSyntaxException;
}

The methods are quite simple. They input the appropriate parse exception context, return a substitute value with type T, and may throw a WrapperCommandSyntaxException. There's nothing else to say, other than to highlight how these have one more type parameter than their respective contexts: T, the type of the substitute value.

ParseExceptionArgument interfaces

The final copy-classes are InitialParseExceptionArgument and ArgumentParseExceptionArgument. If an Argument wants to support parse exception handlers, it must implement these interfaces. The classes look like this:

public interface InitialParseExceptionArgument<T, ExceptionInformation, Impl extends AbstractArgument<?, Impl, ?, ?>> extends ChainableBuilder<Impl> {
    Map<InitialParseExceptionArgument<?, ?, ?>, InitialParseExceptionHandler<?, ?>> exceptionHandlers = new HashMap<>();

    default Impl withInitialParseExceptionHandler(InitialParseExceptionHandler<T, ExceptionInformation> exceptionHandler) {
        exceptionHandlers.put(this, exceptionHandler);
        return instance();
    }

    default Optional<InitialParseExceptionHandler<T, ExceptionInformation>> getInitialParseExceptionHandler() {
        return Optional.ofNullable((InitialParseExceptionHandler<T, ExceptionInformation>) exceptionHandlers.get(this));
    }

    ExceptionInformation parseInitialParseException(CommandSyntaxException exception, StringReader reader);
}

public interface ArgumentParseExceptionArgument<T, Raw, ExceptionInformation, Impl extends AbstractArgument<?, Impl, ?, CommandSender>, CommandSender> extends ChainableBuilder<Impl> {
    Map<ArgumentParseExceptionArgument<?, ?, ?, ?, ?>, ArgumentParseExceptionHandler<?, ?, ?, ?>> exceptionHandlers = new HashMap<>();

    default Impl withArgumentParseExceptionHandler(ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender> exceptionHandler) {
        exceptionHandlers.put(this, exceptionHandler);
        return instance();
    }

    default Optional<ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender>> getArgumentParseExceptionHandler() {
        return Optional.ofNullable(
            (ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender>) exceptionHandlers.get(this)
        );
    }

    default T handleArgumentParseException(...) throws CommandSyntaxException {
        // Don't worry about this for now
    }
}

These classes provide the API methods with(Initial/Argument)ParseExceptionHandler that the developer uses to specify their exception handlers. There are also the get(Initial/Argument)ParseExceptionHandler methods for retrieving the current exception handler, or an empty Optional if none was set. These classes add another type parameter over their respective exception handlers: Impl - the return type for chained method calls.

These classes have one special method each, parseInitialParseException and handleArgumentParseException respectively. They are used on the implementation side when catching and handling exceptions, so their implementation details will be discussed later. These methods are not expected to be used by developers.

One annoying implementation detail here are the exceptionHandlers maps in both classes. These map an instance of the interface to its exception handler. Ideally, there would be an instance variable exceptionHandler that would do this instead. However, Java doesn't allow instance variables in interfaces. Here are the possible solutions to this problem I've considered:

• Use an abstract class. Abstract classes can have instance variables. However, every Argument already extends the Argument class. Since Java also doesn't have multi-class inheritance, this doesn't work.
• Add the instance variables to each subclass. The (with/get)(Initial/Argument)ParseExceptionHandler methods could be abstract instead of default. The implementing subclasses can have instance variables, so they could implement these methods using those. This works, but you would have to implement the methods in every subclass. There's like 70+ Arguments in the CommandAPI, so that repetition doesn't really make sense.
• Add the instance variable to Argument. Instead of implementing the methods in every subclass, you could just implement the methods in their command class, Argument. However, it doesn't really make sense for every argument to have an initial/argument parse handler, since some of them don't throw any errors. Besides, the Raw and ExceptionInformation type parameters would need to be added to Argument, which would unnecessarily break code that used Argument<?>.
• Use a static map in the interface to map instances to their 'instance variable'. This is the solution I chose, but it also has downsides. Variables in interfaces are not only always static, but they are also always public. This gives no encapsulation, which SonarCloud certainly does not like. The map also doesn't store type information, so handlers retrieved from the map need an unsafe cast.

So, yeah. This is a little weird. I think that if interfaces can have default methods, they should also have instance variables, but oh well. The map solution is the most code smell, but it is also the least code repetition, which I prefer.

Intercepting initial parse exceptions

So, now that we have an argument with an InitialParseExceptionHandler, we need to give it some exceptions to handle. I think this is the hardest part of this PR because you need to inject custom behavior into Brigadier's system. But, it works, and it starts with ExceptionHandlingArgumentType:

public record ExceptionHandlingArgumentType<T, ExceptionInformation>(
        ArgumentType<T> baseType,
        InitialParseExceptionHandler<T, ExceptionInformation> exceptionHandler,
        InitialParseExceptionParser<ExceptionInformation> exceptionParser
) implements ArgumentType<T> {

    @Override
    public T parse(StringReader stringReader) throws CommandSyntaxException {
        try {
            return baseType.parse(stringReader);
        } catch (CommandSyntaxException original) {
            try {
                return exceptionHandler.handleException(new InitialParseExceptionContext<>(
                        new WrapperCommandSyntaxException(original),
                        exceptionParser.parse(original, stringReader),
                        new WrapperStringReader(stringReader)
                ));
            } catch (WrapperCommandSyntaxException newException) {
                throw newException.getException();
            }
        }
    }

    @Override
    public Collection<String> getExamples() {
        return baseType.getExamples();
    }

    @Override
    public <S> CompletableFuture<Suggestions> listSuggestions(CommandContext<S> context, SuggestionsBuilder builder) {
        return baseType.listSuggestions(context, builder);
    }
}

This class implements Brigadier's ArgumentType, so it can be used as the parser on an argument node. One of its parameters is ArgumentType<T> baseType, and this is just the ArgumentType being used by the original argument. The getExamples and listSuggestions methods are just passed directly to this baseType since we don't need to modify either of those.

When ExceptionHandlingArgumentType#parse is called, it can catch any CommandSyntaxException thrown when the baseType is parsed. That's what allows the CommandAPI to actually intercept the initial parse exceptions. It creates an InitialParseExceptionContext and handles it with the InitialParseExceptionHandler<T, ExceptionInformation> exceptionHandler that is active for this argument.

In order to create the ExceptionInformation object for the InitialParseExceptionContext, the InitialParseExceptionParser<ExceptionInformation> exceptionParser parameter is used. InitialParseExceptionParser is just a simple FunctionalInterface that looks like this:

@FunctionalInterface
public interface InitialParseExceptionParser<ExceptionInformation> {
    ExceptionInformation parse(CommandSyntaxException exception, StringReader reader);
}

It takes in the exception that was thrown and the StringReader being used, and gives whatever information it can find. The implementation used for this method depends on the Argument as will be seen later.

These ExceptionHandlingArgumentTypes are added to the command tree when CommandAPIHandler builds commands. Specifically, this is managed by CommandAPIHandler#wrapArgumentType:

public class CommandAPIHandler ...{
    RequiredArgumentBuilder<Source, ?> getRequiredArgumentBuilderWithProvider(Argument argument, Argument[]args, SuggestionProvider<Source> provider){
        // ...

        RequiredArgumentBuilder<Source, ?> requiredArgumentBuilder = RequiredArgumentBuilder
        .argument(argument.getNodeName(), wrapArgumentType(argument, argument.getRawType()));

        // ...
    }

    <T, EI> ArgumentType<T> wrapArgumentType(Argument argument,ArgumentType<T> rawType){
        if (argument instanceof WrapperArgument) {
            // A WrapperArgument should set its raw type to baseArgument's raw type, so that is already correct
            return wrapArgumentType(((WrapperArgument<Argument>) argument).getBaseArgument(), rawType);
        }

        if(!(argument instanceof InitialParseExceptionArgument)) return rawType;

        InitialParseExceptionArgument<T, EI, ?> iPEA = (InitialParseExceptionArgument<T, EI, ?>) argument.instance();

        Optional<InitialParseExceptionHandler<T, EI>> handler = iPEA.getInitialParseExceptionHandler();
        if (handler.isEmpty()) return rawType;
        return new ExceptionHandlingArgumentType<>(rawType, handler.get(), iPEA::parseInitialParseException);
    }
}

When CommandAPIHandler makes a Brigadier required argument node, wrapArgumentType will check if the argument has an InitialParseExceptionHandler. If it does, instead of using the original ArgumentType for the argument, it will insert an ExceptionHandlingArgumentType with the defined InitialParseExceptionHandler and IntialParseExceptionParser.

This code also shows a new interface, WrapperArgument. This interface represents arguments that wrap other arguments. Right now, the only argument that does that is CustomArgument. CustomArgument uses the raw type of another argument, so it should use the InitialParseExceptionHandler of another argument. CustomArgument is platform-specific though, so WrapperArgument just allows commandapi-core to get that information, similar to the Literal and MultiLiteral interfaces.

Here, you can see that the exception parser comes from the abstract method parseInitialParseException. Each implementation of InitialParseExceptionArgument will define the generic type parameter ExceptionInformation, and this method defines how to create that information object. This will be explored more in the next section about implementing initial parse exception arguments.

The final detail for this section is making this work with Minecraft clients. When a client joins a server, the Commands packet will be sent to inform them about the structure of the server's Brigadier command tree. Each node in the tree is encoded with a specific format, and there are special classes for each ArgumentType to handle that conversion. So, in order for the player to receive a tree that uses the ExeceptionHandlingArgumentType, we need to set up its own serializer class. On Bukkit, this is a version-specific thing without an API (shocking :|). The NMS#registerCustomArgumentType method was added to deal with this.

The implementation details here aren't super important. You can look at the NMS classes for details. Basically, each NMS version has a new ExceptionHandlingArgumentSerializer_(VERSION) (1.15 to 1.18) or ExceptionHandlingArgumentInfo_(VERSION) (1.19+) class that handles formatting our custom argument type into packet. Unfortunately, since the client doesn't know about the ExceptionHandlingArgumentType, it can't handle our nodes and would end up disconnecting with an error. To get around this, the argument serializers remove their own data and insert the data for the base ArgumentType instead. So, when an ExceptionHandlingArgumentType is wrapped around a node on the server, the client doesn't actually see any changes.

Implementing new InitialParseExceptionArguments

In order for an Argument to have an InitialParseExceptionHandler, it needs to implement InitialParseExceptionArgument. For the proof of concept, I made IntegerArgument an InitialParseExceptionArgument, and it now looks like this:

public class IntegerArgument extends SafeOverrideableArgument<Integer, Integer>
    implements InitialParseExceptionArgument<Integer, IntegerArgument.InitialParseExceptionInformation, Argument<Integer>> {
    
    // Constructors and normal Argument stuff

    public record InitialParseExceptionInformation(
            Exceptions type, String rawInput,
            int input, int minimum, int maximum
    ) {
        public enum Exceptions {
            EXPECTED_INTEGER,
            INVALID_INTEGER,
            INTEGER_TOO_LOW,
            INTEGER_TOO_HIGH
        }
    }

    private static String getRawIntInput(StringReader reader) {
        // Copied from the first half to StringReader#readInt
        int start = reader.getCursor();
        while (reader.canRead() && StringReader.isAllowedNumber(reader.peek())) {
            reader.skip();
        }
        return reader.getString().substring(start, reader.getCursor());
    }

    @Override
    public InitialParseExceptionInformation parseInitialParseException(CommandSyntaxException exception, StringReader reader) {
        String key = CommandAPIBukkit.get().extractTranslationKey(exception);
        if (key == null) {
            throw new IllegalStateException("Unexpected null translation key for IntegerArgument initial parse", exception);
        }
        IntegerArgumentType baseType = (IntegerArgumentType) this.getRawType();
        int min = baseType.getMinimum();
        int max = baseType.getMaximum();
        return switch (key) {
            case "parsing.int.expected" -> new InitialParseExceptionInformation(
                    InitialParseExceptionInformation.Exceptions.EXPECTED_INTEGER,
                    "", 0, min, max
            );
            case "parsing.int.invalid" -> new InitialParseExceptionInformation(
                    InitialParseExceptionInformation.Exceptions.INVALID_INTEGER,
                    getRawIntInput(reader), 0, min, max
            );
            case "argument.integer.low" -> {
                String rawInput = getRawIntInput(reader);
                yield new InitialParseExceptionInformation(
                        InitialParseExceptionInformation.Exceptions.INTEGER_TOO_LOW,
                        rawInput, Integer.parseInt(rawInput), min, max
                );
            }
            case "argument.integer.big" -> {
                String rawInput = getRawIntInput(reader);
                yield new InitialParseExceptionInformation(
                        InitialParseExceptionInformation.Exceptions.INTEGER_TOO_HIGH,
                        rawInput, Integer.parseInt(rawInput), min, max
                );
            }
            default -> throw new IllegalStateException("Unexpected translation key for IntegerArgument initial parse: " + key, exception);
        };
    }
}

Remember that the generic types for InitialParseExceptionArgument are <T, ExceptionInformation, Impl>. The IntegerArgument gives <Integer, IntegerArgument.InitialParseExceptionInformation, Argument<Integer>>. So, each of these mean:

• The type returned when Brigadier parses this argument is Integer.
• The object that holds argument-specific information about the exception is IntegerArgument.InitialParseExceptionInformation.
• The class that is returned when building this Argument is Argument<Integer>.

InitialParseExceptionArgument also needs an implementation for its abstract method ExceptionInformation parseInitialParseException(CommandSyntaxException exception, StringReader reader), which interprets why the parse exception was thrown by Brigadier. As defined by the type parameter, IntegerArgument's exception information is stored in the inner record InitialParseExceptionInformation. This record store 5 pieces of information:

• Exceptions type - the type of exception that was thrown. Exceptions is an enum with the values EXPECTED_INTEGER, INVALID_INTEGER, INTEGER_TOO_LOW, and INTEGER_TOO_HIGH, which are the 4 reasons why Brigadier might fail to parse and IntegerArgumentType.
• String rawInput - The string given in the command that represents this argument. This will be empty when the exception type is EXPECTED_INTEGER, because that exception happens when no number was given.
• int input - The int given in the command for this argument. This will default to 0 when the exception type is EXPECTED_INTEGER or INVALID_INTEGER, since those exceptions happen when the given String could not be parsed as an int.
• int minimum - The minimum value set for this IntegerArgument.
• int maximum - The maximum value set for this IntegerArgument.

IntegerArgument#parseInitialParseException(CommandSyntaxException, StringReader) is responsible for extracting these 5 pieces of information. String rawInput can be found using the StringReader, int input comes from evaluating Integer.parseInt(rawInput), and minimum and maximum are stored by the reference to IntegerArgumentType stored in Argument when the IntegerArgument was constructed.

Figuring out the proper value to put for Exceptions type is a bit trickier. All there is to go off is the CommandSyntaxException parameter. Luckily, all the builtin exception messages are translatable, so they have a consistent translation key. However, the classes involved are NMS, and the component structure changed in 1.19, so the method NMS#extractTranslationKey was added to deal with this. Once IntegerArgument gets the translation key, a simple switch statement finds the proper Execeptions value for the information record.

Intercepting argument parse exceptions and Implementing new ArgumentParseExceptionArguments

Since the argument parse is handled by CommandAPI code, it is much easier to intercept the exceptions and extract their data. An argument that implements ArgumentParseExceptionArgument simply needs to modify its parsing code to use the exception handler. So, this section covers the exception interception and argument implementation. For the proof of concept, I made ListArgumentCommon an ArgumentParseExceptionArgument, and it now looks like this:

public class ListArgumentCommon<T> extends Argument<List> implements
    ArgumentParseExceptionArgument<T, String, ListArgumentCommon.ArgumentParseExceptionInformation<T>, Argument<List>, CommandSender> {
    
    // Constructors and normal argument stuff

    public record ArgumentParseExceptionInformation<T>(
            Exceptions type, List<T> listSoFar, String currentItem
    ) {
        public enum Exceptions {
            DUPLICATES_NOT_ALLOWED,
            INVALID_ITEM
        }
    }

    @Override
    public <CommandSourceStack> List<T> parseArgument(CommandContext<CommandSourceStack> cmdCtx, String key, CommandArguments previousArgs) throws CommandSyntaxException {
        
        // setup stuff
        
        for (String str : strArr) {
            // Parsing parsing parsing...
            
            // Hm, we found a duplicate when those are not allowed
            list.add(handleArgumentParseException(cmdCtx, key, previousArgs,
                    new SimpleCommandExceptionType(new LiteralMessage(
                            "Duplicate arguments are not allowed"
                    )).createWithContext(context),
                    new ArgumentParseExceptionInformation<>(
                            ArgumentParseExceptionInformation.Exceptions.DUPLICATES_NOT_ALLOWED, list, str
                    )
            ));
            
            // Hm, we found an invalid item
            list.add(handleArgumentParseException(cmdCtx, key, previousArgs,
                    new SimpleCommandExceptionType(new LiteralMessage(
                            "Item is not allowed in list"
                    )).createWithContext(context),
                    new ArgumentParseExceptionInformation<>(
                            ArgumentParseExceptionInformation.Exceptions.INVALID_ITEM, list, str
                    )
            ));
        }
        // Finishing up
    }
}

Remember that the generic types for InitialParseExceptionArgument are <T, Raw, ExceptionInformation, Impl, CommandSender>. The ListArgumentCommon gives <T, String, ListArgumentCommon.ArgumentParseExceptionInformation<T>, Argument<List>, CommandSender>. So, each of these mean:

• The substitute value returned by the ArgumentParseExceptionHandler is T. List arguments return List<T> when parsed, so this exception handler actually works on each item in the list, which we'll see later.
• The type returned when Brigadier parses this argument is String. The list argument either uses a greedy string or text string. Most ArgumentTypes return an NMS object, but in this case we can pass the raw String to the developer.
• The object that holds argument-specific information about the exception is ListArgumentCommon.ArgumentParseExceptionInformation<T>.
• The class that is returned when building this Argument is Argument<List>.
• The class that sends commands is CommandSender.

As defined by the type parameter, a list argument's exception information is stored in the inner record ArgumentParseExceptionInformation. This record store 3 pieces of information:

• Exceptions type - the type of exception that was thrown. Exceptions is an enum with the values DUPLICATES_NOT_ALLOWED and INVALID_ITEM, which are the 2 reasons a list argument may fail to parse.
• List<T> listSoFar - A list of the items that have already been parsed.
• String currentItem - The current string that was being parsed.

Since we control the code that throws argument parse exceptions, this information is easy to get. In the two places that ListArgumentCommon#parseArgument used to throw an exception, a new ArgumentParseExceptionInformation is created with the relevant information and handled.

Something interesting to note about the list argument is that exceptions are only thrown for individual items. This means that instead of substituting the entire list when an exception happens, each individual item can be substituted. So, in the code, the result of the exception handling is passed into List#add. If an exception is thrown it will still bubble up, but a single item with type T can also be substituted.

To handle the exception, the default method handleArgumentParseException provided by ArgumentParseExceptionArgument is used. I skipped over the details here before, but this is what that method looks like:

public interface ArgumentParseExceptionArgument<T, Raw, ExceptionInformation, Impl extends AbstractArgument<?, Impl, ?, CommandSender>, CommandSender> extends ChainableBuilder<Impl> {
    default <Source, A extends AbstractArgument<?, ?, A, CommandSender>>
    T handleArgumentParseException(
        CommandContext<Source> cmdCtx, String key, CommandArguments previousArgs,
        CommandSyntaxException original, ExceptionInformation exceptionInformation
    ) throws CommandSyntaxException {
        ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender> exceptionHandler =
                getArgumentParseExceptionHandler().orElseThrow(() -> original);

        try {
            return exceptionHandler.handleException(new ArgumentParseExceptionContext<>(
                    new WrapperCommandSyntaxException(original),
                    exceptionInformation,
                    CommandAPIHandler.<A, CommandSender, Source>getInstance().getPlatform()
                            .getCommandSenderFromCommandSource(cmdCtx.getSource()).getSource(),
                    (Raw) cmdCtx.getArgument(key, Object.class),
                    previousArgs
            ));
        } catch (WrapperCommandSyntaxException newException) {
            throw newException.getException();
        }
    }
}

If this argument does not have an exception handler, the original exception is immediately thrown, keeping the old behavior. If there is a handler, then the ArgumentParseExceptionContext is constructed and passed to the handler. The handler may return a substitute value, or it can throw a WrapperCommandSyntaxException that is unwrapped and thrown.

This method mostly helps construct the ArgumentParseExceptionContext. The extra CommandContext<Source> cmdCtx, String key, and CommandArguments previousArgs parameters of the method are used for this purpose.

---

So, that took a long time to explain. Definitely ask questions and leave code review. As you can see with the TODO list below, there many things I still want to work on. However, I think I'm done with most of the main systems, so feedback on the API and backend would still be great.

TODO

• Implement more (Initial/Argument)ParseExceptionArguments
  • Implement an argument that can handle both types of exceptions (ListTextArgument)
• Expand tests
  • Test substitution with ArgumentParseExceptionHandler
  • Add specific tests for each exception that should be generated by implemented (Initial/Argument)ParseExceptionArguments
• Make sure it works outside the CommandAPI project (without Brigadier dependency)
  • Maybe create a simple example project for compile testing
• Todos in the code

  • CommandAPI/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/ExceptionHandlingArgumentInfo_1_19_Common.java

    Line 13 in 73b2d12

    // TODO: Maybe there could be a common serializer for these ArgumentTypeInfo classes?

  • CommandAPI/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/ArgumentParseExceptionArgument.java

    Lines 32 to 34 in 73b2d12

    	// TODO: Maybe this can be a WeakHashMap, so once the Argument objects aren't being used anywhere else we can forget
    	// about them and not store them anymore. I'm not entirely sure that is what WeakHashMap does though. Are Arguments
    	// ever GC'd anyway, or do they stick around somewhere?

  • CommandAPI/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.15/src/main/java/dev/jorel/commandapi/nms/ExceptionHandlingArgumentSerializer_1_15.java

    Lines 16 to 18 in 73b2d12

    	// All the ? here should actually be ArgumentRegistry.a, but that is a private inner class. That makes everything really annoying.
    	// TODO: We want to check this reflection, but we can't give ArgumentRegistry.a to the @RequireField annotation
    	// Hopefully something works out, but the preprocessor needs to be expanded first

Test NMS code on real servers

• 1.15, 1.15.1, 1.15.2
• 1.16.1
• 1.16.2, 1.16.3
• 1.16.4
• 1.16.5
• 1.17
• 1.17.1
• 1.18, 1.18.1
• 1.18.2
• 1.19
• 1.19.1, 1.19.2
• 1.19.3
• 1.19.4
• 1.20, 1.20.1

• Documentation

Maybe todo

• Make it work on Velocity

[DerEchtePilz]

Before I leave the review you requested, here's something I didn't quite understand:
If I understood all this correctly, that means that the (Initatial/Argument)ParseExceptionHandlers will always intercept the exception thrown by the base argument if an error occurs.
Isn't this unexpected if the user never calls the API methods?

[DerEchtePilz]

Just a few things I noticed that would (in my opinion) be great if they were changed.
I've also probably missed a lot (or not :D) because I rushed this a bit so I'll probably leave another review soon

[willkroboth]

@DerEchtePilz

If I understood all this correctly, that means that the (Initatial/Argument)ParseExceptionHandlers will always intercept the exception thrown by the base argument if an error occurs.
Isn't this unexpected if the user never calls the API methods?

Yeah, that's a good question. If the API methods are not called, then the exception will not be intercepted. If an exception handler is not defined, then the original exception will pass through.

For the initial parse, an argument node with the ExceptionHandlingArgumentType will always intercept the exception. However, this ArgumentType is only used when an exception handler has been defined. This code decides if the CommandAPI needs to use an ExceptionHandlingArgumentType:

CommandAPI/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java

Lines 816 to 829 in 73b2d12

	<T, EI> ArgumentType<T> wrapArgumentType(Argument argument, ArgumentType<T> rawType) {
	if (argument instanceof WrapperArgument) {
	// A WrapperArgument should set its raw type to baseArgument's raw type, so that is already correct
	return wrapArgumentType(((WrapperArgument<Argument>) argument).getBaseArgument(), rawType);
	}
	if (!(argument instanceof InitialParseExceptionArgument)) return rawType;
	InitialParseExceptionArgument<T, EI, ?> iPEA = (InitialParseExceptionArgument<T, EI, ?>) argument.instance();
	Optional<InitialParseExceptionHandler<T, EI>> handler = iPEA.getInitialParseExceptionHandler();
	if (handler.isEmpty()) return rawType;
	return new ExceptionHandlingArgumentType<>(rawType, handler.get(), iPEA::parseInitialParseException);
	}

If the Argument is not an InitialParseExceptionArgument, then its normal ArgumentType<T> rawType is returned (line 822). If this is an InitialParseExceptionArgument, but there is not an InitialParseExceptionHandler, the rawType is also used (line 827). An ExceptionHandlingArgumentType is only used if there is an exception handler, so exceptions will only be intercepted if there is something to handle it.

For the argument parse, this code is always run:

CommandAPI/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/ArgumentParseExceptionArgument.java

Lines 60 to 80 in 73b2d12

	default <Source, A extends AbstractArgument<?, ?, A, CommandSender>>
	T handleArgumentParseException(
	CommandContext<Source> cmdCtx, String key, CommandArguments previousArgs,
	CommandSyntaxException original, ExceptionInformation exceptionInformation
	) throws CommandSyntaxException {
	ArgumentParseExceptionHandler<T, Raw, ExceptionInformation, CommandSender> exceptionHandler =
	getArgumentParseExceptionHandler().orElseThrow(() -> original);
	try {
	return exceptionHandler.handleException(new ArgumentParseExceptionContext<>(
	new WrapperCommandSyntaxException(original),
	exceptionInformation,
	CommandAPIHandler.<A, CommandSender, Source>getInstance().getPlatform()
	.getCommandSenderFromCommandSource(cmdCtx.getSource()).getSource(),
	(Raw) cmdCtx.getArgument(key, Object.class),
	previousArgs
	));
	} catch (WrapperCommandSyntaxException newException) {
	throw newException.getException();
	}
	}

If the Optional<ArgumentParseExceptionHandler> is empty, then orElseThrow(() -> original) will throw the original exception (line 66). It can only handle the exception if it finds an exception handler.

[willkroboth]

Currently waiting on dev/command-build-rewrite (currently 1011fd5 to 22c2ffa). I believe the changes there will allow the InitialParseExceptionArgument to become NMS independent and platform agnostic.