Merge pull request #7329 from SkriptLang/feature/pre-merge-patch

Feature/pre merge patch

src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java

@@ -244,7 +244,7 @@ public Player get(final BlockIgniteEvent e) {
 			@Override
 			@Nullable
 			public Block get(final BlockIgniteEvent e) {
-				return e.getIgnitingBlock();
+				return e.getBlock();
 			}
 		}, 0);
 		// BlockDispenseEvent

src/main/java/ch/njol/skript/entity/BoatData.java

@@ -50,7 +50,7 @@ public BoatData(@Nullable Boat.Type type){
 	private BoatData(int type) {
 		matchedPattern = type;
 	}
-	
+
 	@Override
 	protected boolean init(Literal<?>[] exprs, int matchedPattern, ParseResult parseResult) {
 		return true;

src/main/java/ch/njol/skript/expressions/ExprXOf.java

@@ -18,6 +18,8 @@
 import org.bukkit.inventory.ItemStack;
 import org.jetbrains.annotations.Nullable;
 
+import java.lang.reflect.Array;
+
 @Name("X of Item")
 @Description("An expression to be able to use a certain amount of items where the amount can be any expression. Please note that this expression is not stable and might be replaced in the future.")
 @Examples("give level of player of pickaxes to the player")
@@ -44,7 +46,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 	protected Object[] get(Event e, Object[] source) {
 		Number a = amount.getSingle(e);
 		if (a == null)
-			return new Object[0];
+			return (Object[]) Array.newInstance(getReturnType(), 0);
 
 		return get(source, o -> {
 			if (o instanceof ItemStack) {

src/main/java/ch/njol/skript/lang/SkriptParser.java

@@ -351,9 +351,9 @@ public boolean hasTag(String tag) {
 			if ((flags & PARSE_EXPRESSIONS) != 0) {
 				Expression<?> parsedExpression = parseExpression(types, expr);
 				if (parsedExpression != null) { // Expression/VariableString parsing success
+					Class<?> parsedReturnType = parsedExpression.getReturnType();
 					for (Class<? extends T> type : types) {
-						// Check return type against everything that expression accepts
-						if (parsedExpression.canReturn(type)) {
+						if (type.isAssignableFrom(parsedReturnType)) {
 							log.printLog();
 							return (Expression<? extends T>) parsedExpression;
 						}
@@ -519,13 +519,14 @@ public boolean hasTag(String tag) {
 			if ((flags & PARSE_EXPRESSIONS) != 0) {
 				Expression<?> parsedExpression = parseExpression(types, expr);
 				if (parsedExpression != null) { // Expression/VariableString parsing success
+					Class<?> parsedReturnType = parsedExpression.getReturnType();
 					for (int i = 0; i < types.length; i++) {
 						Class<?> type = types[i];
 						if (type == null) // Ignore invalid (null) types
 							continue;
 
-						// Check return type against everything that expression accepts
-						if (parsedExpression.canReturn(type)) {
+						// Check return type against the expression's return type
+						if (type.isAssignableFrom(parsedReturnType)) {
 							if (!exprInfo.isPlural[i] && !parsedExpression.isSingle()) { // Wrong number of arguments
 								if (context == ParseContext.COMMAND) {
 									Skript.error(Commands.m_too_many_arguments.toString(exprInfo.classes[i].getName().getIndefiniteArticle(), exprInfo.classes[i].getName().toString()), ErrorQuality.SEMANTIC_ERROR);

src/main/java/ch/njol/skript/lang/Variable.java

@@ -555,14 +555,15 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro
 					if (mode == ChangeMode.REMOVE) {
 						if (map == null)
 							return;
-						ArrayList<String> toRemove = new ArrayList<>(); // prevents CMEs
+						Set<String> toRemove = new HashSet<>(); // prevents CMEs
 						for (Object value : delta) {
 							for (Entry<String, Object> entry : map.entrySet()) {
+								String key = entry.getKey();
+								if (key == null)
+									continue; // This is NOT a part of list variable
+								if (toRemove.contains(key))
+									continue; // Skip if we've already marked this key to be removed
 								if (Relation.EQUAL.isImpliedBy(Comparators.compare(entry.getValue(), value))) {
-									String key = entry.getKey();
-									if (key == null)
-										continue; // This is NOT a part of list variable
-
 									// Otherwise, we'll mark that key to be set to null
 									toRemove.add(key);
 									break;
@@ -576,7 +577,7 @@ public void change(Event event, Object @Nullable [] delta, ChangeMode mode) thro
 					} else if (mode == ChangeMode.REMOVE_ALL) {
 						if (map == null)
 							return;
-						ArrayList<String> toRemove = new ArrayList<>(); // prevents CMEs
+						Set<String> toRemove = new HashSet<>(); // prevents CMEs
 						for (Entry<String, Object> i : map.entrySet()) {
 							for (Object value : delta) {
 								if (Relation.EQUAL.isImpliedBy(Comparators.compare(i.getValue(), value)))

src/main/java/ch/njol/skript/lang/util/ConvertedExpression.java

@@ -6,6 +6,7 @@
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.registrations.Classes;
+import ch.njol.skript.util.Utils;
 import ch.njol.util.Checker;
 import ch.njol.util.Kleenean;
 import ch.njol.util.coll.CollectionUtils;
@@ -77,24 +78,31 @@ public ConvertedExpression(Expression<? extends F> source, Class<T> to, Collecti
 	@SafeVarargs
 	public static <F, T> @Nullable ConvertedExpression<F, T> newInstance(Expression<F> from, Class<T>... to) {
 		assert !CollectionUtils.containsSuperclass(to, from.getReturnType());
-		// we track a list of converters that may work
-		List<ConverterInfo<? super F, ? extends T>> converters = new ArrayList<>();
-		for (Class<T> type : to) { // REMIND try more converters? -> also change WrapperExpression (and maybe ExprLoopValue)
-			assert type != null;
-			// casting <? super ? extends F> to <? super F> is wrong, but since the converter is only used for values returned by the expression
-			// (which are instances of "<? extends F>") this won't result in any ClassCastExceptions.
-			for (Class<? extends F> checking : from.possibleReturnTypes()) {
-				//noinspection unchecked
-				ConverterInfo<? super F, ? extends T> converter = (ConverterInfo<? super F, ? extends T>) Converters.getConverterInfo(checking, type);
-				if (converter != null)
-					converters.add(converter);
+
+		// we might be able to cast some (or all) of the possible return types to T
+		// for possible return types that can't be directly cast, regular converters will be used
+		List<ConverterInfo<? extends F, ? extends T>> infos = new ArrayList<>();
+		for (Class<? extends F> type : from.possibleReturnTypes()) {
+			if (CollectionUtils.containsSuperclass(to, type)) { // this type is of T, build a converter simply casting
+				// noinspection unchecked - 'type' is a desired type in 'to'
+				Class<T> toType = (Class<T>) type;
+				infos.add(new ConverterInfo<>(type, toType, toType::cast, 0));
+			} else { // this possible return type is not included in 'to'
+				// build all converters for converting the possible return type into any of the types of 'to'
+				for (Class<T> toType : to) {
+					ConverterInfo<? extends F, T> converter = Converters.getConverterInfo(type, toType);
+					if (converter != null)
+						infos.add(converter);
+				}
 			}
-			int size = converters.size();
-			if (size == 1) // if there is only one info, there is no need to wrap it in a list
-				return new ConvertedExpression<>(from, type, converters.get(0));
-			if (size > 1)
-				return new ConvertedExpression<>(from, type, converters, true);
 		}
+		if (!infos.isEmpty()) { // there are converters for (at least some of) the return types
+			// a note: casting <? extends F> to <? super F> is wrong, but since the converter is used only for values
+			//         returned by the expression (which are instances of <? extends F>), this won't result in any CCEs
+			// noinspection rawtypes, unchecked
+			return new ConvertedExpression(from, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, true);
+		}
+
 		return null;
 	}
 

src/main/java/ch/njol/skript/lang/util/SimpleExpression.java

@@ -19,13 +19,10 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.skriptlang.skript.lang.converter.Converter;
-import org.skriptlang.skript.lang.converter.ConverterInfo;
 
 import java.lang.reflect.Array;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
-import java.util.List;
 
 /**
  * An implementation of the {@link Expression} interface. You should usually extend this class to make a new expression.
@@ -189,33 +186,6 @@ public static <T> boolean check(T @Nullable [] values, Checker<? super T> checke
 		// check whether this expression is already of type R
 		if (CollectionUtils.containsSuperclass(to, getReturnType()))
 			return (Expression<? extends R>) this;
-
-		// we might be to cast some of the possible return types to R
-		List<ConverterInfo<? extends T, R>> infos = new ArrayList<>();
-		for (Class<? extends T> type : this.possibleReturnTypes()) {
-			if (CollectionUtils.containsSuperclass(to, type)) { // this type is of R
-				// build a converter that for casting to R
-				// safety check is present in the event that we do not get this type at runtime
-				final Class<R> toType = (Class<R>) type;
-				infos.add(new ConverterInfo<>(getReturnType(), toType, fromObject -> {
-					if (toType.isInstance(fromObject))
-						return (R) fromObject;
-					return null;
-				}, 0));
-			}
-		}
-		int size = infos.size();
-		if (size == 1) { // if there is only one info, there is no need to wrap it in a list
-			ConverterInfo<? extends T, R> info = infos.get(0);
-			//noinspection rawtypes
-			return new ConvertedExpression(this, info.getTo(), info);
-		}
-		if (size > 1) {
-			//noinspection rawtypes
-			return new ConvertedExpression(this, Utils.getSuperType(infos.stream().map(ConverterInfo::getTo).toArray(Class[]::new)), infos, false);
-		}
-
-		// attempt traditional conversion with proper converters
 		return this.getConvertedExpr(to);
 	}
 

src/test/skript/tests/regressions/6817-null of x incorrect type.sk

@@ -0,0 +1,3 @@
+test "null of x incorrect type":
+	# would cause an exception
+	spawn {_null} of pig above {_null}

src/test/skript/tests/regressions/7162-removing from variable skips duplicates.sk

@@ -0,0 +1,6 @@
+test "removing from variables skips duplicates":
+	set {_list::*} to 1, 1, 2, 3, 4, 5, 6 and 6
+	remove 1 and 1 from {_list::*}
+	assert {_list::*} is 2, 3, 4, 5, 6 and 6 with "didn't remove all instances of 1 from the list"
+	remove first 6 elements out of {_list::*} from {_list::*}
+	assert {_list::*} is not set with "didn't remove all elements"

src/test/skript/tests/regressions/7164-expressions sometimes dont convert.sk

@@ -0,0 +1,13 @@
+test "expressions sometimes dont convert":
+	assert formatted ({_foo} + {_bar}) is not set with "formatting nothing shouldn't throw an error"
+
+	set {_foo} to "test"
+	assert formatted ({_foo} + {_bar}) is not set with "formatting string + none shouldn't throw an error"
+
+	set {_foo} to 1
+	set {_bar} to 2
+	assert formatted ({_foo} + {_bar}) is not set with "formatting number + number shouldn't throw an error"
+
+	set {_foo} to "foo"
+	set {_bar} to "bar"
+	assert formatted ({_foo} + {_bar}) is "foobar" with "formatting strings doesn't work"