Skip to content

Commit 214e1e9

Browse files
APickledWalrusAPickledWalrus
committed
Improve native argument validation
1 parent 004968e commit 214e1e9

4 files changed

Lines changed: 98 additions & 26 deletions

File tree

‎src/main/java/org/skriptlang/skript/bukkit/command/custom/ScriptArgumentType.java‎

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -41,37 +41,66 @@
4141
public class ScriptArgumentType<T> implements CustomArgumentType.Converted<Object, String> {
4242

4343
/**
44-
* Pre-defined mappings of types that are acceptable to map to other native argument types.
44+
* Data about an available native argument type.
45+
* A native argument type is one that is recognized by the client, providing enhanced validation features.
46+
* @param supportsPlural Whether this argument type supports returning multiple values.
47+
* @param supportsRange Whether this argument type supports minimum or maximum values.
48+
* @param mapper A function for obtaining the native argument type from an argument data.
4549
*/
46-
public static final Map<Class<?>, Function<ArgumentData<?>, ArgumentType<?>>> ARGUMENT_TYPE_MAPPINGS = Map.of(
47-
Boolean.class, data -> data.isSingle() ? BoolArgumentType.bool() : null,
48-
Long.class, data -> {
49-
if (!data.isSingle()) {
50-
// TODO should error here if min/max is used
51-
return null;
50+
public record NativeArgumentData(
51+
boolean supportsPlural,
52+
boolean supportsRange,
53+
Function<ArgumentData<?>, ArgumentType<?>> mapper
54+
) {
55+
56+
public NativeArgumentData {
57+
if (!supportsPlural) {
58+
final var inputMapper = mapper;
59+
mapper = data -> data.isSingle() ? inputMapper.apply(data) : null;
5260
}
53-
if (data.max() == null) {
54-
if (data.min() == null) {
61+
}
62+
63+
public NativeArgumentData(Function<ArgumentData<?>, ArgumentType<?>> mapper) {
64+
this(false, false, mapper);
65+
}
66+
67+
}
68+
69+
/**
70+
* Pre-defined mappings of types that are acceptable to map to other native argument types.
71+
*/
72+
public static final Map<Class<?>, NativeArgumentData> ARGUMENT_TYPE_MAPPINGS = Map.of(
73+
Boolean.class, new NativeArgumentData(ignored -> BoolArgumentType.bool()),
74+
Long.class, new NativeArgumentData(false, true, data -> {
75+
Long min = (Long) data.min();
76+
Long max = (Long) data.max();
77+
if (min == null) {
78+
if (max == null) {
5579
return LongArgumentType.longArg();
5680
}
57-
return LongArgumentType.longArg((long) data.min());
58-
}
59-
return LongArgumentType.longArg((long) data.min(), (long) data.max());
60-
},
61-
Number.class, data -> {
62-
if (!data.isSingle()) {
63-
return null;
81+
return LongArgumentType.longArg(Long.MIN_VALUE, max);
82+
} else if (max == null) {
83+
return LongArgumentType.longArg(min);
6484
}
65-
if (data.max() == null) {
66-
if (data.min() == null) {
85+
return LongArgumentType.longArg(min, max);
86+
}),
87+
Number.class, new NativeArgumentData(false, true, data -> {
88+
Number min = (Number) data.min();
89+
Number max = (Number) data.max();
90+
if (min == null) {
91+
if (max == null) {
6792
return DoubleArgumentType.doubleArg();
6893
}
69-
return DoubleArgumentType.doubleArg((double) data.min());
94+
return DoubleArgumentType.doubleArg(Double.MIN_VALUE, max.doubleValue());
95+
} else if (max == null) {
96+
return DoubleArgumentType.doubleArg(min.doubleValue());
7097
}
71-
return DoubleArgumentType.doubleArg((double) data.min(), (double) data.max());
72-
},
73-
Player.class, data -> data.isSingle() ? ArgumentTypes.player() : ArgumentTypes.players(),
74-
Entity.class, data -> data.isSingle() ? ArgumentTypes.entity() : ArgumentTypes.entities()
98+
return DoubleArgumentType.doubleArg(min.doubleValue(), max.doubleValue());
99+
}),
100+
Player.class, new NativeArgumentData(true, false, data ->
101+
data.isSingle() ? ArgumentTypes.player() : ArgumentTypes.players()),
102+
Entity.class, new NativeArgumentData(true, false, data ->
103+
data.isSingle() ? ArgumentTypes.entity() : ArgumentTypes.entities())
75104
);
76105

77106
private static final DynamicCommandExceptionType ERROR_PARSER_ERROR = new DynamicCommandExceptionType(

‎src/main/java/org/skriptlang/skript/bukkit/command/elements/structures/util/CommandCompiler.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import ch.njol.skript.util.Utils;
1111
import org.jetbrains.annotations.Nullable;
1212
import org.skriptlang.skript.bukkit.command.custom.ArgumentData;
13+
import org.skriptlang.skript.bukkit.command.custom.ScriptArgumentType;
14+
import org.skriptlang.skript.bukkit.command.custom.ScriptArgumentType.NativeArgumentData;
1315

1416
import java.util.ArrayList;
1517
import java.util.Collection;
@@ -362,7 +364,7 @@ private static List<LiteralCommandElement> appendToLiterals(Collection<LiteralCo
362364
Pattern.compile("^\\s*(?:([^>]+?)\\s*:\\s*)?(.+?)\\s*(?:=\\s*(" + SkriptParser.WILDCARD + "))?\\s*$");
363365

364366
private static final Pattern TYPE_PATTERN =
365-
Pattern.compile("^(.+?)\\s*(?: from (.+?)(?: to (.+?))?)?$");
367+
Pattern.compile("^(.+?)\\s*(?: (?:from|above|between) (.+?))?(?:(?: (?:to|(?:and )?below|and) (.+?))?)?$");
366368

367369
private static @Nullable ArgumentData<?> parseArgument(String argument, List<ArgumentData<?>> arguments) {
368370
Matcher argumentMatcher = ARGUMENT_PATTERN.matcher(argument);
@@ -403,6 +405,13 @@ private static List<LiteralCommandElement> appendToLiterals(Collection<LiteralCo
403405
return null;
404406
}
405407
}
408+
// type validation
409+
NativeArgumentData nativeMapping = ScriptArgumentType.ARGUMENT_TYPE_MAPPINGS.get(type.getC());
410+
if ((min != null || max != null) && (nativeMapping == null || !nativeMapping.supportsRange())) {
411+
String typeName = plural.plural() ? type.getName().getPlural() : type.getName().getSingular();
412+
Skript.error(typeName + " arguments do not support minimum or maximum values.");
413+
return null;
414+
}
406415

407416
// next, parse the name
408417
String name = argumentMatcher.group(1);

‎src/main/java/org/skriptlang/skript/bukkit/command/elements/structures/util/SubCommandEntryData.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.skriptlang.skript.bukkit.command.custom.CommandParsingData.ExecutorData;
2525
import org.skriptlang.skript.bukkit.command.custom.CooldownManager;
2626
import org.skriptlang.skript.bukkit.command.custom.ExecutableBy;
27+
import org.skriptlang.skript.bukkit.command.custom.ScriptArgumentType.NativeArgumentData;
2728
import org.skriptlang.skript.bukkit.command.custom.ScriptCommandEvent;
2829
import org.skriptlang.skript.bukkit.command.elements.structures.util.CommandCompiler.ArgumentCommandElement;
2930
import org.skriptlang.skript.bukkit.command.elements.structures.util.CommandCompiler.CommandElement;
@@ -368,9 +369,9 @@ public boolean canCreateWith(Node node) {
368369
ArgumentData<?> data = ((ArgumentCommandElement) commandElement).argument();
369370

370371
ArgumentType<?> nativeType = null;
371-
var nativeMapping = ScriptArgumentType.ARGUMENT_TYPE_MAPPINGS.get(data.type().getC());
372+
NativeArgumentData nativeMapping = ScriptArgumentType.ARGUMENT_TYPE_MAPPINGS.get(data.type().getC());
372373
if (nativeMapping != null) { // native argument type may be available
373-
nativeType = nativeMapping.apply(data);
374+
nativeType = nativeMapping.mapper().apply(data);
374375
}
375376
if (nativeType == null) {
376377
if (children.isEmpty() && subcommands.isEmpty()) { // last argument can be greedy

‎src/test/skript/tests/bukkit/command/StructCommand.sk‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ using local variable type hints
33
options:
44
command: skriptcommand
55

6+
# expect error for invalid executable by for subcommand
67
parse:
78
results: {-StructCommand::entries::1::*}
89
code:
@@ -15,20 +16,39 @@ parse:
1516
trigger:
1617
stop
1718

19+
# expect error for duplicate argument names
1820
parse:
1921
results: {-StructCommand::args::1::*}
2022
code:
2123
command /args_test_1 <x:number> <x:number>:
2224
trigger:
2325
stop
2426

27+
# expect error for type not supporting range
28+
parse:
29+
results: {-StructCommand::args::2::*}
30+
code:
31+
command /args_test_2 <x: text between "hello" and "world">:
32+
trigger:
33+
stop
34+
35+
# expect no errors
36+
parse:
37+
results: {-StructCommand::args::3::*}
38+
code:
39+
command /args_test_3 <x: number above 5> <y: number below 5> <z: number between 5 and 5>:
40+
trigger:
41+
stop
42+
43+
# expect error that {_x} is not a text
2544
parse:
2645
results: {-StructCommand::hints::1::*}
2746
code:
2847
command /hint_test_1 <x:number>:
2948
trigger:
3049
set {_a} to {_x} in lowercase
3150

51+
# expect error that {_x::*} is not a text
3252
parse:
3353
results: {-StructCommand::hints::2::*}
3454
code:
@@ -47,6 +67,10 @@ command /test-args-plural <x: numbers>:
4767
trigger:
4868
set {-test-args-plural::*} to {_x::*}
4969

70+
command /test-args-ranged <x: number above 5 and below 10>:
71+
trigger:
72+
set {-test-args-ranged::*} to {_x}
73+
5074
command {@command} <text> [<text>] [<itemtype = %dirt named "steve"%>]:
5175
trigger:
5276
set {-test-{@command}::arg1} to arg-1
@@ -66,6 +90,8 @@ test "StructCommand":
6690
# parsing
6791
assert {-StructCommand::entries::1::*} is "It is not possible to restrict execution to the console as the parent command is only executable by players."
6892
assert {-StructCommand::args::1::*} is "The argument name 'x' was already used."
93+
assert {-StructCommand::args::2::*} is "text arguments do not support minimum or maximum values."
94+
assert {-StructCommand::args::3::*} is not set
6995
# parsing type hints
7096
assert {-StructCommand::hints::1::*} is "Expected variable '{_x}' to be a text, but it is a number" with "Hint failed (%{StructCommand::hints::1::*}%)"
7197
assert {-StructCommand::hints::2::*} is "Expected variable '{_x::*}' to be a text, but it is a number" with "Hint failed (%{StructCommand::hints::2::*}%)"
@@ -81,6 +107,13 @@ test "StructCommand":
81107
execute command "test-args-plural 10 and 2"
82108
assert {-test-args-plural::*} is 10 and 2 with "plural input to plural args did not work"
83109

110+
execute command "test-args-ranged 6"
111+
assert {-test-args-ranged::*} is 6 with "argument in range was not accepted"
112+
execute command "test-args-ranged 4"
113+
assert {-test-args-ranged::*} is 6 with "argument below range was accepted"
114+
execute command "test-args-ranged 11"
115+
assert {-test-args-ranged::*} is 6 with "argument above range was accepted"
116+
84117
execute command "{@command} taco"
85118
assert {-test-{@command}::arg1} is "taco" with "arg-1 test failed"
86119
assert {-test-{@command}::arg2} is not set with "arg-2 test failed"

0 commit comments

Comments
 (0)