Fix the legendary compare issue with two same literals

src/main/java/ch/njol/skript/conditions/CondCompare.java

@@ -22,15 +22,20 @@
 import org.eclipse.jdt.annotation.Nullable;
 
 import ch.njol.skript.Skript;
+
 import org.skriptlang.skript.lang.comparator.Comparator;
 import org.skriptlang.skript.lang.comparator.Relation;
+import org.skriptlang.skript.lang.converter.ConverterInfo;
+import org.skriptlang.skript.lang.converter.Converters;
+
 import ch.njol.skript.doc.Description;
 import ch.njol.skript.doc.Examples;
 import ch.njol.skript.doc.Name;
 import ch.njol.skript.doc.Since;
 import ch.njol.skript.lang.Condition;
 import ch.njol.skript.lang.Expression;
 import ch.njol.skript.lang.ExpressionList;
+import ch.njol.skript.lang.Literal;
 import ch.njol.skript.lang.ParseContext;
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.lang.UnparsedLiteral;
@@ -39,15 +44,13 @@
 import ch.njol.skript.log.RetainingLogHandler;
 import ch.njol.skript.log.SkriptLogger;
 import ch.njol.skript.registrations.Classes;
+
 import org.skriptlang.skript.lang.comparator.Comparators;
 import ch.njol.skript.util.Patterns;
 import ch.njol.skript.util.Utils;
 import ch.njol.util.Checker;
 import ch.njol.util.Kleenean;
 
-/**
- * @author Peter Güttinger
- */
 @Name("Comparison")
 @Description({"A very general condition, it simply compares two values. Usually you can only compare for equality (e.g. block is/isn't of <type>), " +
 		"but some values can also be compared using greater than/less than. In that case you can also test for whether an object is between two others.",
@@ -93,19 +96,18 @@ public class CondCompare extends Condition {
 		Skript.registerCondition(CondCompare.class, patterns.getPatterns());
 	}
 
-	@SuppressWarnings("null")
 	private Expression<?> first;
-	@SuppressWarnings("null")
 	private Expression<?> second;
+
 	@Nullable
 	private Expression<?> third;
-	@SuppressWarnings("null")
+
 	private Relation relation;
-	@SuppressWarnings("rawtypes")
+
 	@Nullable
+	@SuppressWarnings("rawtypes")
 	private Comparator comp;
 
-	@SuppressWarnings("null")
 	@Override
 	public boolean init(final Expression<?>[] vars, final int matchedPattern, final Kleenean isDelayed, final ParseResult parser) {
 		first = vars[0];
@@ -160,57 +162,55 @@ public static String f(final Expression<?> e) {
 
 	@SuppressWarnings({"unchecked"})
 	private boolean init(String expr) {
-		final RetainingLogHandler log = SkriptLogger.startRetainingLog();
+		RetainingLogHandler log = SkriptLogger.startRetainingLog();
 		Expression<?> third = this.third;
 		try {
 			if (first.getReturnType() == Object.class) {
-				final Expression<?> e = first.getConvertedExpression(Object.class);
-				if (e == null) {
+				Expression<?> expression = null;
+				if (first instanceof UnparsedLiteral)
+					expression = attemptReconstruct((UnparsedLiteral) first, second);
+				if (expression == null)
+					expression = first.getConvertedExpression(Object.class);
+				if (expression == null) {
 					log.printErrors();
 					return false;
 				}
-				first = e;
+				first = expression;
 			}
 			if (second.getReturnType() == Object.class) {
-				final Expression<?> e = second.getConvertedExpression(Object.class);
-				if (e == null) {
+				Expression<?> expression = null;
+				if (second instanceof UnparsedLiteral)
+					expression = attemptReconstruct((UnparsedLiteral) second, first);
+				if (expression == null)
+					expression = second.getConvertedExpression(Object.class);
+				if (expression == null) {
 					log.printErrors();
 					return false;
 				}
-				second = e;
+				second = expression;
 			}
 			if (third != null && third.getReturnType() == Object.class) {
-				final Expression<?> e = third.getConvertedExpression(Object.class);
-				if (e == null) {
+				Expression<?> expression = null;
+				if (third instanceof UnparsedLiteral)
+					expression = attemptReconstruct((UnparsedLiteral) third, first);
+				if (expression == null)
+					expression = third.getConvertedExpression(Object.class);
+				if (expression == null) {
 					log.printErrors();
 					return false;
 				}
-				this.third = third = e;
+				this.third = third = expression;
 			}
 			log.printLog();
 		} finally {
 			log.stop();
 		}
-		final Class<?> f = first.getReturnType(), s = third == null ? second.getReturnType() : Utils.getSuperType(second.getReturnType(), third.getReturnType());
+		Class<?> f = first.getReturnType(), s = third == null ? second.getReturnType() : Utils.getSuperType(second.getReturnType(), third.getReturnType());
 		if (f == Object.class || s == Object.class)
 			return true;
-		/*
-		 * https://github.com/Mirreski/Skript/issues/10
-		 * 
-		if(Entity.class.isAssignableFrom(s)){
-			String[] split = expr.split(" ");
-			System.out.println(expr);
-			if(!split[split.length - 1].equalsIgnoreCase("player") && EntityData.parseWithoutIndefiniteArticle(split[split.length - 1]) != null){
-				comp = Comparators.getComparator(f, EntityData.class);
-				second = SkriptParser.parseLiteral(split[split.length - 1], EntityData.class, ParseContext.DEFAULT);
-			}else
-				comp = Comparators.getComparator(f, s);
-
-		}
-		*///else
-
+
 		comp = Comparators.getComparator(f, s);
-		
+
 		if (comp == null) { // Try to re-parse with more context
 			/*
 			 * SkriptParser sees that CondCompare takes two objects. Most of the time,
@@ -237,10 +237,10 @@ private boolean init(String expr) {
 			}
 
 		}
-		
+
 		return comp != null;
 	}
-	
+
 	/**
 	 * Attempts to parse given expression again as a literal of given type.
 	 * This will only work if the expression is a literal and its unparsed
@@ -251,22 +251,53 @@ private boolean init(String expr) {
 	 * @return A literal value, or null if parsing failed.
 	 */
 	@Nullable
-	private <T> SimpleLiteral<T> reparseLiteral(Class<T> type, Expression<?> expr) {
-		if (expr instanceof SimpleLiteral) { // Only works for simple literals
-			Expression<?> source = expr.getSource();
-
-			// Try to get access to unparsed content of it
-			if (source instanceof UnparsedLiteral) {
-				String unparsed = ((UnparsedLiteral) source).getData();
-				T data = Classes.parse(unparsed, type, ParseContext.DEFAULT);
-				if (data != null) { // Success, let's make a literal of it
-					return new SimpleLiteral<>(data, false, new UnparsedLiteral(unparsed));
-				}
+	private <T> SimpleLiteral<T> reparseLiteral(Class<T> type, Expression<?> expression) {
+		Expression<?> source = expression;
+		if (expression instanceof SimpleLiteral) // Only works for simple literals
+			source = expression.getSource();
+
+		// Try to get access to unparsed content of it
+		if (source instanceof UnparsedLiteral) {
+			String unparsed = ((UnparsedLiteral) source).getData();
+			T data = Classes.parse(unparsed, type, ParseContext.DEFAULT);
+			if (data != null) { // Success, let's make a literal of it
+				return new SimpleLiteral<>(data, false, new UnparsedLiteral(unparsed));
 			}
 		}
 		return null; // Context-sensitive parsing failed; can't really help it
 	}
-
+
+	/**
+	 * Attempts to transform an UnparsedLiteral into a type that is comparable to another.
+	 * For example 'fire' will be VisualEffect without this, but if the user attempts to compare 'fire'
+	 * with a block. This method will see if 'fire' can be compared to the block, and it will find ItemType.
+	 * Essentially solving something a human sees as comparable, but Skript doesn't understand.
+	 * 
+	 * @param one The UnparsedLiteral expression to attempt to reconstruct.
+	 * @param two any expression to grab the return type from.
+	 * @return The newly formed Literal, will be SimpleLiteral in most cases.
+	 */
+	@SuppressWarnings("unchecked")
+	private Literal<?> attemptReconstruct(UnparsedLiteral one, Expression<?> two) {
+		Expression<?> expression = null;
+		// Must handle numbers first.
+		expression = one.getConvertedExpression(Number.class);
+		if (expression == null) {
+			for (ConverterInfo<?, ?> converter : Converters.getConverterInfo()) {
+				// We're comparing the 'two' expression, so we need the 'from' for comparing.
+				if (!converter.getFrom().isAssignableFrom(two.getReturnType()))
+					continue;
+				// Must be comparable if we're going to attempt to convert this UnparsedLiteral. Otherwise why attempt something that won't work.
+				if (Comparators.getComparator(two.getReturnType(), converter.getTo()) == null)
+					continue;
+				expression = reparseLiteral(converter.getTo(), one);
+			}
+		}
+		if (expression == null)
+			expression = one.getConvertedExpression(two.getReturnType());
+		return (Literal<?>) expression;
+	}
+
 	/*
 	 * # := condition (e.g. is, is less than, contains, is enchanted with, has permission, etc.)
 	 * !# := not #
@@ -302,7 +333,7 @@ private <T> SimpleLiteral<T> reparseLiteral(Class<T> type, Expression<?> expr) {
 	 * neither a nor b # x or y === a !# x or y && b !# x or y			// nor = and
 	 */
 	@Override
-	@SuppressWarnings({"null", "unchecked"})
+	@SuppressWarnings("unchecked")
 	public boolean check(final Event e) {
 		final Expression<?> third = this.third;
 		return first.check(e, (Checker<Object>) o1 ->

src/main/java/ch/njol/skript/registrations/Classes.java

@@ -504,9 +504,9 @@ public static <T> T parse(final String s, final Class<T> c, final ParseContext c
 					continue;
 				if (c.isAssignableFrom(conv.getTo())) {
 					log.clear();
-					final Object o = parseSimple(s, conv.getFrom(), context);
-					if (o != null) {
-						t = (T) ((Converter) conv.getConverter()).convert(o);
+					Object object = parseSimple(s, conv.getFrom(), context);
+					if (object != null) {
+						t = (T) ((Converter) conv.getConverter()).convert(object);
 						if (t != null) {
 							log.printLog();
 							return t;

src/main/java/ch/njol/skript/test/runner/EffAssert.java

@@ -20,6 +20,7 @@
 
 import org.bukkit.event.Event;
 import org.eclipse.jdt.annotation.Nullable;
+import org.skriptlang.skript.lang.script.Script;
 
 import ch.njol.skript.Skript;
 import ch.njol.skript.doc.Description;
@@ -46,6 +47,7 @@ public class EffAssert extends Effect  {
 
 	@Nullable
 	private Condition condition;
+	private Script script;
 
 	private Expression<String> errorMsg;
 	private boolean shouldFail;
@@ -56,6 +58,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 		String conditionString = parseResult.regexes.get(0).group();
 		errorMsg = (Expression<String>) exprs[0];
 		shouldFail = parseResult.mark != 0;
+		script = getParser().getCurrentScript();
 
 		ParseLogHandler logHandler = SkriptLogger.startParseLogHandler();
 		try {
@@ -91,7 +94,7 @@ public TriggerItem walk(Event event) {
 			if (SkriptJUnitTest.getCurrentJUnitTest() != null) {
 				TestTracker.junitTestFailed(SkriptJUnitTest.getCurrentJUnitTest(), message);
 			} else {
-				TestTracker.testFailed(message);
+				TestTracker.testFailed(message, script);
 			}
 			return null;
 		}

src/main/java/ch/njol/skript/test/runner/EvtTestCase.java

@@ -18,6 +18,8 @@
  */
 package ch.njol.skript.test.runner;
 
+import org.bukkit.Location;
+import org.bukkit.block.Block;
 import org.bukkit.event.Event;
 import org.eclipse.jdt.annotation.Nullable;
 
@@ -27,25 +29,39 @@
 import ch.njol.skript.lang.Literal;
 import ch.njol.skript.lang.SkriptEvent;
 import ch.njol.skript.lang.SkriptParser;
+import ch.njol.skript.registrations.EventValues;
+import ch.njol.skript.util.Getter;
 
 public class EvtTestCase extends SkriptEvent {
-	
+
 	static {
-		if (TestMode.ENABLED)
+		if (TestMode.ENABLED) {
 			Skript.registerEvent("Test Case", EvtTestCase.class, SkriptTestEvent.class, "test %string% [when <.+>]")
-				.description("Contents represent one test case.")
-				.examples("")
-				.since("2.5");
+					.description("Contents represent one test case.")
+					.examples("")
+					.since("2.5");
+			EventValues.registerEventValue(SkriptTestEvent.class, Block.class, new Getter<Block, SkriptTestEvent>() {
+				@Override
+				public @Nullable Block get(SkriptTestEvent ignored) {
+					return SkriptJUnitTest.getBlock();
+				}
+			}, EventValues.TIME_NOW);
+			EventValues.registerEventValue(SkriptTestEvent.class, Location.class, new Getter<Location, SkriptTestEvent>() {
+				@Override
+				public @Nullable Location get(SkriptTestEvent ignored) {
+					return SkriptJUnitTest.getTestLocation();
+				}
+			}, EventValues.TIME_NOW);
+		}
 	}
-
-	@SuppressWarnings("null")
+
 	private Expression<String> name;
-	
+
 	@Nullable
 	private Condition condition;
-
-	@SuppressWarnings({"null", "unchecked"})
+
 	@Override
+	@SuppressWarnings("unchecked")
 	public boolean init(Literal<?>[] args, int matchedPattern, SkriptParser.ParseResult parseResult) {
 		name = (Expression<String>) args[0];
 		if (!parseResult.regexes.isEmpty()) { // Do not parse or run unless condition is met
@@ -54,27 +70,27 @@ public boolean init(Literal<?>[] args, int matchedPattern, SkriptParser.ParseRes
 		}
 		return true;
 	}
-	
+
 	@Override
-	public boolean check(Event e) {
-		String n = name.getSingle(e);
-		if (n == null) {
+	public boolean check(Event event) {
+		String n = name.getSingle(event);
+		if (n == null)
 			return false;
-		}
 		Skript.info("Running test case " + n);
 		TestTracker.testStarted(n);
 		return true;
 	}
-	
+
 	@Override
 	public boolean shouldLoadEvent() {
 		return condition != null ? condition.check(new SkriptTestEvent()) : true;
 	}
-	
+
 	@Override
-	public String toString(@Nullable Event e, boolean debug) {
-		if (e != null)
-			return "test " + name.getSingle(e);
+	public String toString(@Nullable Event event, boolean debug) {
+		if (event != null)
+			return "test " + name.getSingle(event);
 		return "test case";
 	}
+
 }