Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify CIP edge case behavior and add type safety guard #7363

Merged
merged 3 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/main/java/ch/njol/skript/classes/Changer.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -50,7 +51,7 @@ public boolean supportsKeyedChange() {

abstract class ChangerUtils {

public static <T> void change(Changer<T> changer, Object[] what, Object @Nullable [] delta, ChangeMode mode) {
public static <T> void change(@NotNull Changer<T> changer, Object[] what, Object @Nullable [] delta, ChangeMode mode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the contract for what[]?

Copy link
Member Author

@sovdeeth sovdeeth Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk i just clicked the intellij button to add notnull
I presume it's a notnull array with notnull values though

//noinspection unchecked
changer.change((T[]) what, delta, mode);
}
Expand All @@ -63,19 +64,36 @@ public static <T> void change(Changer<T> changer, Object[] what, Object @Nullabl
* @param types The types to test for
* @return Whether <tt>expression.{@link Expression#change(Event, Object[], ChangeMode) change}(event, type[], mode)</tt> 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;
}

}

}
36 changes: 27 additions & 9 deletions src/main/java/ch/njol/skript/lang/Expression.java
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,18 @@ default Map<ChangeMode, Class<?>[]> getAcceptedChangeModes() {
* changing the expression. For example, {@code set vector length of {_v} to 1}, rather than
* {@code set {_v} to vector(0,1,0)}.
* <br>
* 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.
* <br>
* 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 <R> The output type of the change function. Must be a type returned
* by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}.
*/
Expand All @@ -329,13 +334,18 @@ default <R> void changeInPlace(Event event, Function<T, R> changeFunction) {
* changing the expression. For example, {@code set vector length of {_v} to 1}, rather than
* {@code set {_v} to vector(0,1,0)}.
* <br>
* 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}.
* 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.
* <br>
* 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 <R> The output type of the change function. Must be a type returned
* by {{@link #acceptChange(ChangeMode)}} for {@link ChangeMode#SET}.
Expand All @@ -345,9 +355,17 @@ default <R> void changeInPlace(Event event, Function<T, R> 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<R> 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);
}
Expand Down
Loading