From 42d9d9b7b2526b5680c012d105587c5a26a7e8f8 Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Thu, 2 Jan 2025 14:51:22 -0500 Subject: [PATCH 1/2] Clarify CIP edge case behavior and add type safety guard Also improves ChangerUtils#acceptsChange slightly. --- .../java/ch/njol/skript/classes/Changer.java | 32 +++++++++++++---- .../java/ch/njol/skript/lang/Expression.java | 34 ++++++++++++++----- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/main/java/ch/njol/skript/classes/Changer.java b/src/main/java/ch/njol/skript/classes/Changer.java index 544433b6ee6..74b0040fe15 100644 --- a/src/main/java/ch/njol/skript/classes/Changer.java +++ b/src/main/java/ch/njol/skript/classes/Changer.java @@ -1,6 +1,7 @@ package ch.njol.skript.classes; import org.bukkit.event.Event; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import ch.njol.skript.classes.data.DefaultChangers; @@ -50,7 +51,7 @@ public boolean supportsKeyedChange() { abstract class ChangerUtils { - public static void change(Changer changer, Object[] what, Object @Nullable [] delta, ChangeMode mode) { + public static void change(@NotNull Changer changer, Object[] what, Object @Nullable [] delta, ChangeMode mode) { //noinspection unchecked changer.change((T[]) what, delta, mode); } @@ -63,19 +64,36 @@ public static void change(Changer changer, Object[] what, Object @Nullabl * @param types The types to test for * @return Whether expression.{@link Expression#change(Event, Object[], ChangeMode) change}(event, type[], mode) can be used or not. */ - public static boolean acceptsChange(final Expression expression, final ChangeMode mode, final Class... types) { - final Class[] validTypes = expression.acceptChange(mode); + public static boolean acceptsChange(@NotNull Expression expression, ChangeMode mode, Class... types) { + Class[] validTypes = expression.acceptChange(mode); if (validTypes == null) return false; - for (final Class type : types) { - for (final Class validType : validTypes) { - if (validType.isArray() ? validType.getComponentType().isAssignableFrom(type) : validType.isAssignableFrom(type)) + + for (int i = 0; i < validTypes.length; i++) { + if (validTypes[i].isArray()) + validTypes[i] = validTypes[i].getComponentType(); + } + + return acceptsChangeTypes(validTypes, types); + } + + /** + * Tests whether any of the given types is accepted by the given array of valid types. + * + * @param types The types to test for + * @param validTypes The valid types. All array classes should be unwrapped to their component type before calling. + * @return Whether any of the types is accepted by the valid types. + */ + public static boolean acceptsChangeTypes(Class[] validTypes, Class @NotNull ... types) { + for (Class type : types) { + for (Class validType : validTypes) { + if (validType.isAssignableFrom(type)) return true; } } return false; } - + } } diff --git a/src/main/java/ch/njol/skript/lang/Expression.java b/src/main/java/ch/njol/skript/lang/Expression.java index 42653c83d04..ecd2d9d0b4b 100644 --- a/src/main/java/ch/njol/skript/lang/Expression.java +++ b/src/main/java/ch/njol/skript/lang/Expression.java @@ -308,13 +308,18 @@ default Map[]> getAcceptedChangeModes() { * changing the expression. For example, {@code set vector length of {_v} to 1}, rather than * {@code set {_v} to vector(0,1,0)}. *
- * This is a 1 to 1 transformation and should not add or remove elements. - * For {@link Variable}s, this will retain indices. For non-{@link Variable}s, it will - * evaluate {@link #getArray(Event)}, apply the change function on each, and call - * {@link #change(Event, Object[], ChangeMode)} with the modified values and {@link ChangeMode#SET}. + * This is a 1 to 1 transformation and should not add elements. + * Returning null will remove the element. + * Returning a type not accepted by {@link #acceptChange(ChangeMode)} for {@link ChangeMode#SET} + * will depend on the implementer. The default implementation will remove the element. + *
+ * This expression must support {@link ChangeMode#SET} for this method to work. * * @param event The event to use for local variables and evaluation * @param changeFunction A 1-to-1 function that transforms a single input to a single output. + * Returning null will remove the element. + * Returning a type not accepted by {@link #acceptChange(ChangeMode)} for {@link ChangeMode#SET} + * will depend on the implementer. The default implementation will remove the element. * @param The output type of the change function. Must be a type returned * by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}. */ @@ -330,12 +335,17 @@ default void changeInPlace(Event event, Function changeFunction) { * {@code set {_v} to vector(0,1,0)}. *
* This is a 1 to 1 transformation and should not add or remove elements. - * For {@link Variable}s, this will retain indices. For non-{@link Variable}s, it will - * evaluate the expression, apply the change function on each value, and call - * {@link #change(Event, Object[], ChangeMode)} with the modified values and {@link ChangeMode#SET}. + * Returning null will remove the element. + * Returning a type not accepted by {@link #acceptChange(ChangeMode)} for {@link ChangeMode#SET} + * will depend on the implementer. The default implementation will remove the element. + *
+ * This expression must support {@link ChangeMode#SET} for this method to work. * * @param event The event to use for local variables and evaluation * @param changeFunction A 1-to-1 function that transforms a single input to a single output. + * Returning null will remove the element. + * Returning a type not accepted by {@link #acceptChange(ChangeMode)} for {@link ChangeMode#SET} + * will depend on the implementer. The default implementation will remove the element. * @param getAll Whether to evaluate with {@link #getAll(Event)} or {@link #getArray(Event)}. * @param The output type of the change function. Must be a type returned * by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}. @@ -345,9 +355,17 @@ default void changeInPlace(Event event, Function changeFunction, boole T[] values = getAll ? getAll(event) : getArray(event); if (values.length == 0) return; + + @SuppressWarnings("DataFlowIssue") + Class[] validClasses = Arrays.stream(acceptChange(ChangeMode.SET)) + .map(c -> c.isArray() ? c.getComponentType() : c) + .toArray(Class[]::new); + List newValues = new ArrayList<>(); for (T value : values) { - newValues.add(changeFunction.apply(value)); + R newValue = changeFunction.apply(value); + if (newValue != null && ChangerUtils.acceptsChangeTypes(validClasses, newValue.getClass())) + newValues.add(newValue); } change(event, newValues.toArray(), ChangeMode.SET); } From af89afd4dae52090d84374c145cd6528dac7ccff Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Thu, 2 Jan 2025 14:55:48 -0500 Subject: [PATCH 2/2] Update Expression.java --- src/main/java/ch/njol/skript/lang/Expression.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ch/njol/skript/lang/Expression.java b/src/main/java/ch/njol/skript/lang/Expression.java index ecd2d9d0b4b..6240e674b76 100644 --- a/src/main/java/ch/njol/skript/lang/Expression.java +++ b/src/main/java/ch/njol/skript/lang/Expression.java @@ -334,7 +334,7 @@ default void changeInPlace(Event event, Function changeFunction) { * changing the expression. For example, {@code set vector length of {_v} to 1}, rather than * {@code set {_v} to vector(0,1,0)}. *
- * This is a 1 to 1 transformation and should not add or remove elements. + * This is a 1 to 1 transformation and should not add elements. * Returning null will remove the element. * Returning a type not accepted by {@link #acceptChange(ChangeMode)} for {@link ChangeMode#SET} * will depend on the implementer. The default implementation will remove the element.