Fix entity casting issue and add regression test. (#6614)

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

@@ -29,6 +29,7 @@
 import ch.njol.skript.lang.ExpressionType;
 import ch.njol.skript.lang.Literal;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
+import ch.njol.skript.util.Utils;
 import ch.njol.util.Kleenean;
 import ch.njol.util.StringUtils;
 import org.bukkit.Location;
@@ -37,6 +38,7 @@
 import org.bukkit.event.Event;
 import org.eclipse.jdt.annotation.Nullable;
 
+import java.lang.reflect.Array;
 import java.util.Arrays;
 
 @Name("Nearest Entity")
@@ -80,8 +82,8 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 	protected Entity[] get(Event event) {
 		Object relativeTo = this.relativeTo.getSingle(event);
 		if (relativeTo == null || (relativeTo instanceof Location && ((Location) relativeTo).getWorld() == null))
-			return new Entity[0];
-		Entity[] nearestEntities = new Entity[entityDatas.length];
+			return (Entity[]) Array.newInstance(this.getReturnType(), 0);;
+		Entity[] nearestEntities = (Entity[]) Array.newInstance(this.getReturnType(), entityDatas.length);
 		for (int i = 0; i < nearestEntities.length; i++) {
 			if (relativeTo instanceof Entity) {
 				nearestEntities[i] = getNearestEntity(entityDatas[i], ((Entity) relativeTo).getLocation(), (Entity) relativeTo);
@@ -97,9 +99,17 @@ public boolean isSingle() {
 		return entityDatas.length == 1;
 	}
 
+	private transient @Nullable Class<? extends Entity> knownReturnType;
+
 	@Override
 	public Class<? extends Entity> getReturnType() {
-		return entityDatas.length == 1 ? entityDatas[0].getType() : Entity.class;
+		if (knownReturnType != null)
+			return knownReturnType;
+		Class<? extends Entity>[] types = new Class[entityDatas.length];
+		for (int i = 0; i < types.length; i++) {
+			types[i] = entityDatas[i].getType();
+		}
+		return knownReturnType = Utils.highestDenominator(Entity.class, types);
 	}
 
 	@Override

src/main/java/ch/njol/skript/util/Utils.java

@@ -62,6 +62,7 @@
 import ch.njol.util.coll.CollectionUtils;
 import ch.njol.util.coll.iterator.EnumerationIterable;
 import net.md_5.bungee.api.ChatColor;
+import org.jetbrains.annotations.NotNull;
 
 /**
  * Utility class.
@@ -673,41 +674,64 @@ public static int random(final int start, final int end) {
 			throw new IllegalArgumentException("end (" + end + ") must be > start (" + start + ")");
 		return start + random.nextInt(end - start);
 	}
-
-	// TODO improve
-	public static Class<?> getSuperType(final Class<?>... cs) {
-		assert cs.length > 0;
-		Class<?> r = cs[0];
-		assert r != null;
-		outer: for (final Class<?> c : cs) {
-			assert c != null && !c.isArray() && !c.isPrimitive() : c;
-			if (c.isAssignableFrom(r)) {
-				r = c;
+
+	/**
+	 * @see #highestDenominator(Class, Class[])
+	 */
+	public static Class<?> getSuperType(final Class<?>... classes) {
+		return highestDenominator(Object.class, classes);
+	}
+
+	/**
+	 * Searches for the highest common denominator of the given types;
+	 * in other words, the first supertype they all share.
+	 *
+	 * <h3>Arbitrary Selection</h3>
+	 * Classes may have <b>multiple</b> highest common denominators: interfaces that they share
+	 * which do not extend each other.
+	 * This method selects a <b>superclass</b> first (where possible)
+	 * but its selection of interfaces is quite random.
+	 * For this reason, it is advised to specify a "best guess" class as the first parameter, which will be selected if
+	 * it's appropriate.
+	 * Note that if the "best guess" is <i>not</i> a real supertype, it can never be selected.
+	 *
+	 * @param bestGuess The fallback class to guess
+	 * @param classes The types to check
+	 * @return The most appropriate common class of all provided
+	 * @param <Found> The highest common denominator found
+	 * @param <Type> The input type spread
+	 */
+	@SafeVarargs
+	@SuppressWarnings("unchecked")
+	public static <Found, Type extends Found> Class<Found> highestDenominator(Class<? super Found> bestGuess, @NotNull Class<? extends Type> @NotNull ... classes) {
+		assert classes.length > 0;
+		Class<?> chosen = classes[0];
+		outer:
+		for (final Class<?> checking : classes) {
+			assert checking != null && !checking.isArray() && !checking.isPrimitive() : checking;
+			if (chosen.isAssignableFrom(checking))
 				continue;
+			Class<?> superType = checking;
+			do if (superType != Object.class && superType.isAssignableFrom(chosen)) {
+				chosen = superType;
+				continue outer;
 			}
-			if (!r.isAssignableFrom(c)) {
-				Class<?> s = c;
-				while ((s = s.getSuperclass()) != null) {
-					if (s != Object.class && s.isAssignableFrom(r)) {
-						r = s;
-						continue outer;
-					}
+			while ((superType = superType.getSuperclass()) != null);
+			for (final Class<?> anInterface : checking.getInterfaces()) {
+				superType = highestDenominator(Object.class, anInterface, chosen);
+				if (superType != Object.class) {
+					chosen = superType;
+					continue outer;
 				}
-				for (final Class<?> i : c.getInterfaces()) {
-					s = getSuperType(i, r);
-					if (s != Object.class) {
-						r = s;
-						continue outer;
-					}
-				}
-				return Object.class;
 			}
+			return (Class<Found>) bestGuess;
 		}
-
+		if (!bestGuess.isAssignableFrom(chosen)) // we struck out on a type we don't want
+			return (Class<Found>) bestGuess;
 		// Cloneable is about as useful as object as super type
 		// However, it lacks special handling used for Object supertype
 		// See #1747 to learn how it broke returning items from functions
-		return r.equals(Cloneable.class) ? Object.class : r;
+		return (Class<Found>) (chosen == Cloneable.class ? bestGuess : chosen == Object.class ? bestGuess : chosen);
 	}
 
 	/**

src/test/skript/tests/regressions/6612-nearest entity cast error.sk

@@ -0,0 +1,18 @@
+test "charge creeper nearest entity cast":
+	set {_location} to spawn of world "world"
+	spawn a creeper at {_location}
+	set {_creeper} to last spawned creeper
+	assert {_creeper} is not charged with "spawning a normal creeper shouldn't spawn a charged one"
+	make (nearest creeper to {_location}) charged # this used to throw an exception
+	assert {_creeper} is charged with "a creeper should be charged after it is set as charged"
+	uncharge the nearest creeper to {_location} # this would also throw an exception
+	assert {_creeper} is not charged with "uncharging a charged creeper should uncharge it"
+	delete the last spawned creeper
+
+test "poison nearest entity cast":
+	set {_location} to spawn of world "world"
+	spawn a creeper at {_location}
+	set {_creeper} to last spawned creeper
+	poison (nearest entity to {_location}) # this never threw an exception
+	poison (nearest creeper to {_location}) # this used to throw an exception
+	delete the last spawned creeper