diff --git a/src/main/java/ch/njol/skript/conditions/CondCompare.java b/src/main/java/ch/njol/skript/conditions/CondCompare.java index dbcb583436b..707f2626e83 100644 --- a/src/main/java/ch/njol/skript/conditions/CondCompare.java +++ b/src/main/java/ch/njol/skript/conditions/CondCompare.java @@ -22,8 +22,14 @@ import org.eclipse.jdt.annotation.Nullable; import ch.njol.skript.Skript; +import ch.njol.skript.classes.ClassInfo; + import org.skriptlang.skript.lang.comparator.Comparator; +import org.skriptlang.skript.lang.comparator.ComparatorInfo; 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; @@ -31,6 +37,7 @@ 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 +46,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 +98,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 - private Comparator comp; + @SuppressWarnings("rawtypes") + private Comparator comparator; - @SuppressWarnings("null") @Override public boolean init(final Expression[] vars, final int matchedPattern, final Kleenean isDelayed, final ParseResult parser) { first = vars[0]; @@ -134,7 +138,7 @@ public boolean init(final Expression[] vars, final int matchedPattern, final } } @SuppressWarnings("rawtypes") - final Comparator comp = this.comp; + final Comparator comp = this.comparator; if (comp != null) { if (third == null) { if (!relation.isImpliedBy(Relation.EQUAL, Relation.NOT_EQUAL) && !comp.supportsOrdering()) { @@ -158,60 +162,59 @@ public static String f(final Expression e) { return Classes.getSuperClassInfo(e.getReturnType()).getName().withIndefiniteArticle(); } - @SuppressWarnings({"unchecked"}) + @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 = attemptReconstruction((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 = attemptReconstruction((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 = attemptReconstruction((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()); - if (f == Object.class || s == Object.class) + Class firstReturnType = first.getReturnType(); + Class secondReturnType = third == null ? second.getReturnType() : Utils.getSuperType(second.getReturnType(), third.getReturnType()); + if (firstReturnType == Object.class || secondReturnType == 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 + + comparator = Comparators.getComparator(firstReturnType, secondReturnType); + + if (comparator == null) { // Try to re-parse with more context /* * SkriptParser sees that CondCompare takes two objects. Most of the time, * this works fine. However, when there are multiple conflicting literals, @@ -224,23 +227,23 @@ private boolean init(String expr) { * Some damage types not working (issue #2184) would be a good example * of issues that SkriptParser's lack of context can cause. */ - SimpleLiteral reparsedSecond = reparseLiteral(first.getReturnType(), second); + SimpleLiteral reparsedSecond = reparseLiteral(firstReturnType, second); if (reparsedSecond != null) { second = reparsedSecond; - comp = Comparators.getComparator(f, second.getReturnType()); + comparator = Comparators.getComparator(firstReturnType, second.getReturnType()); } else { SimpleLiteral reparsedFirst = reparseLiteral(second.getReturnType(), first); if (reparsedFirst != null) { first = reparsedFirst; - comp = Comparators.getComparator(first.getReturnType(), s); + comparator = Comparators.getComparator(first.getReturnType(), secondReturnType); } } } - - return comp != null; + + return comparator != 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 +254,58 @@ private boolean init(String expr) { * @return A literal value, or null if parsing failed. */ @Nullable - private SimpleLiteral reparseLiteral(Class 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 SimpleLiteral reparseLiteral(Class 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 attemptReconstruction(UnparsedLiteral one, Expression two) { + Expression expression = null; + // Must handle numbers first. + expression = one.getConvertedExpression(Number.class); + if (expression == null) { + for (ClassInfo classinfo : Classes.getClassInfos()) { + if (classinfo.getParser() == null) + continue; + ComparatorInfo comparator = Comparators.getComparatorInfo(two.getReturnType(), classinfo.getC()); + if (comparator == null) + continue; + // We don't care about comparators that take in an object. This just causes more issues accepting and increases iterations by half. + // Let getConvertedExpression deal with it in the end if no other possible reparses against the remaining classinfos exist. + if (comparator.getFirstType() == Object.class) + continue; + expression = reparseLiteral(classinfo.getC(), one); + if (expression != null) + break; + } + } + 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,20 +341,20 @@ private SimpleLiteral reparseLiteral(Class 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) o1 -> second.check(e, (Checker) o2 -> { if (third == null) - return relation.isImpliedBy(comp != null ? comp.compare(o1, o2) : Comparators.compare(o1, o2)); + return relation.isImpliedBy(comparator != null ? comparator.compare(o1, o2) : Comparators.compare(o1, o2)); return third.check(e, (Checker) o3 -> { boolean isBetween; - if (comp != null) { + if (comparator != null) { isBetween = - (Relation.GREATER_OR_EQUAL.isImpliedBy(comp.compare(o1, o2)) && Relation.SMALLER_OR_EQUAL.isImpliedBy(comp.compare(o1, o3))) + (Relation.GREATER_OR_EQUAL.isImpliedBy(comparator.compare(o1, o2)) && Relation.SMALLER_OR_EQUAL.isImpliedBy(comparator.compare(o1, o3))) // Check OPPOSITE (switching o2 / o3) - || (Relation.GREATER_OR_EQUAL.isImpliedBy(comp.compare(o1, o3)) && Relation.SMALLER_OR_EQUAL.isImpliedBy(comp.compare(o1, o2))); + || (Relation.GREATER_OR_EQUAL.isImpliedBy(comparator.compare(o1, o3)) && Relation.SMALLER_OR_EQUAL.isImpliedBy(comparator.compare(o1, o2))); } else { isBetween = (Relation.GREATER_OR_EQUAL.isImpliedBy(Comparators.compare(o1, o2)) && Relation.SMALLER_OR_EQUAL.isImpliedBy(Comparators.compare(o1, o3))) @@ -337,7 +376,7 @@ public String toString(final @Nullable Event e, final boolean debug) { else s = first.toString(e, debug) + " is " + (isNegated() ? "not " : "") + "between " + second.toString(e, debug) + " and " + third.toString(e, debug); if (debug) - s += " (comparator: " + comp + ")"; + s += " (comparator: " + comparator + ")"; return s; } diff --git a/src/main/java/ch/njol/skript/registrations/Classes.java b/src/main/java/ch/njol/skript/registrations/Classes.java index 1b316f97dfb..a2958b4cd04 100644 --- a/src/main/java/ch/njol/skript/registrations/Classes.java +++ b/src/main/java/ch/njol/skript/registrations/Classes.java @@ -504,9 +504,9 @@ public static T parse(final String s, final Class 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; diff --git a/src/main/java/ch/njol/skript/test/runner/EffAssert.java b/src/main/java/ch/njol/skript/test/runner/EffAssert.java index 4f78665295d..d61a19dc9bd 100644 --- a/src/main/java/ch/njol/skript/test/runner/EffAssert.java +++ b/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 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) 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; } diff --git a/src/main/java/ch/njol/skript/test/runner/EvtTestCase.java b/src/main/java/ch/njol/skript/test/runner/EvtTestCase.java index eb6bbd93b4f..cec92f1dadc 100644 --- a/src/main/java/ch/njol/skript/test/runner/EvtTestCase.java +++ b/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,41 @@ 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() { + @Override + @Nullable + public Block get(SkriptTestEvent ignored) { + return SkriptJUnitTest.getBlock(); + } + }, EventValues.TIME_NOW); + EventValues.registerEventValue(SkriptTestEvent.class, Location.class, new Getter() { + @Override + @Nullable + public Location get(SkriptTestEvent ignored) { + return SkriptJUnitTest.getTestLocation(); + } + }, EventValues.TIME_NOW); + } } - - @SuppressWarnings("null") + private Expression name; - + @Nullable private Condition condition; - - @SuppressWarnings({"null", "unchecked"}) + @Override + @SuppressWarnings("unchecked") public boolean init(Literal[] args, int matchedPattern, SkriptParser.ParseResult parseResult) { name = (Expression) args[0]; if (!parseResult.regexes.isEmpty()) { // Do not parse or run unless condition is met @@ -54,27 +72,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"; } + } diff --git a/src/main/java/ch/njol/skript/test/runner/SkriptJUnitTest.java b/src/main/java/ch/njol/skript/test/runner/SkriptJUnitTest.java index ed9fb6d026e..8431dc9a6a3 100644 --- a/src/main/java/ch/njol/skript/test/runner/SkriptJUnitTest.java +++ b/src/main/java/ch/njol/skript/test/runner/SkriptJUnitTest.java @@ -86,14 +86,14 @@ public final void cleanup() { /** * @return the test world. */ - protected World getTestWorld() { + public static World getTestWorld() { return Bukkit.getWorlds().get(0); } /** * @return the testing location at the spawn of the testing world. */ - protected Location getTestLocation() { + public static Location getTestLocation() { return getTestWorld().getSpawnLocation().add(0, 1, 0); } @@ -102,7 +102,7 @@ protected Location getTestLocation() { * * @return Pig that has been spawned. */ - protected Pig spawnTestPig() { + public static Pig spawnTestPig() { if (delay <= 0D) delay = 1; // A single tick allows the piggy to spawn before server shutdown. return (Pig) getTestWorld().spawnEntity(getTestLocation(), EntityType.PIG); @@ -114,7 +114,7 @@ protected Pig spawnTestPig() { * @param material The material to set the block to. * @return the Block after it has been updated. */ - protected Block setBlock(Material material) { + public static Block setBlock(Material material) { Block block = getBlock(); block.setType(material); return block; @@ -125,7 +125,7 @@ protected Block setBlock(Material material) { * * @return the Block after it has been updated. */ - protected Block getBlock() { + public static Block getBlock() { return getTestWorld().getSpawnLocation().add(10, 1, 0).getBlock(); } diff --git a/src/main/java/ch/njol/skript/test/runner/TestTracker.java b/src/main/java/ch/njol/skript/test/runner/TestTracker.java index fb6b2a0df40..dde781c344f 100644 --- a/src/main/java/ch/njol/skript/test/runner/TestTracker.java +++ b/src/main/java/ch/njol/skript/test/runner/TestTracker.java @@ -18,12 +18,14 @@ */ package ch.njol.skript.test.runner; +import java.io.File; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; import org.eclipse.jdt.annotation.Nullable; +import org.skriptlang.skript.lang.script.Script; import ch.njol.skript.test.utils.TestResults; @@ -54,6 +56,12 @@ public static void testFailed(String msg) { failedTests.put(currentTest, msg); } + public static void testFailed(String msg, Script script) { + String file = script.getConfig().getFileName(); + file = file.substring(file.lastIndexOf(File.separator) + 1); + failedTests.put(currentTest, msg + " [" + file + "]"); + } + public static void junitTestFailed(String junit, String msg) { failedTests.put(junit, msg); } diff --git a/src/test/skript/tests/misc/dummy.sk b/src/test/skript/tests/misc/dummy.sk index abf6c02d80e..d31e8fd53b7 100644 --- a/src/test/skript/tests/misc/dummy.sk +++ b/src/test/skript/tests/misc/dummy.sk @@ -1,2 +1,2 @@ test "redundant": - assert "true" is "true" with "logic failed" \ No newline at end of file + assert "true" is "true" with "logic failed" diff --git a/src/test/skript/tests/regressions/2711-item-compares.sk b/src/test/skript/tests/regressions/2711-item-compares.sk new file mode 100644 index 00000000000..064f6cc7cff --- /dev/null +++ b/src/test/skript/tests/regressions/2711-item-compares.sk @@ -0,0 +1,5 @@ +test "item comparison": + set {_test} to a dragon head + assert {_test} is a dragon head with "failed to compare against a head" + set {_test} to a door + assert {_test} is any door, any wood door, birch door or a closed birch door with "failed to compare against a door" diff --git a/src/test/skript/tests/regressions/4769-fire-visualeffect.sk b/src/test/skript/tests/regressions/4769-fire-visualeffect.sk new file mode 100644 index 00000000000..3965e6c877f --- /dev/null +++ b/src/test/skript/tests/regressions/4769-fire-visualeffect.sk @@ -0,0 +1,15 @@ +test "fire visual effect comparison": + assert a block is a block with "failed to compare block classinfo against block classinfo" + assert an itemtype is an itemtype with "failed to compare itemtype classinfo against itemtype classinfo" + assert a diamond is an itemtype with "failed to compare itemtype 'diamond' against itemtype classinfo" + set {_block} to type of block at spawn of world "world" + set block at spawn of world "world" to fire + assert block at spawn of world "world" is fire with "failed to compare fire (itemtype) with a block" + set block at spawn of world "world" to {_block} + play 5 fire at spawn of world "world" + assert "fire" parsed as visual effect is fire with "failed to compare visual effects" + assert "fire" parsed as visual effect is "fire" parsed as itemtype to fail with "failed to compare visual effects ##2" + spawn a chicken at spawn of world "world": + assert event-entity is a chicken with "failed to compare a chicken" + assert event-entity is a "chicken" parsed as itemtype to fail with "failed to compare a chicken ##2" + clear event-entity diff --git a/src/test/skript/tests/regressions/4773-composter-the-imposter.sk b/src/test/skript/tests/regressions/4773-composter-the-imposter.sk new file mode 100644 index 00000000000..7fb0fdc677e --- /dev/null +++ b/src/test/skript/tests/regressions/4773-composter-the-imposter.sk @@ -0,0 +1,5 @@ +test "composter the imposter" when running minecraft "1.14.4": + set {_block} to type of block at spawn of world "world" + set block at spawn of world "world" to composter + assert block at spawn of world "world" is composter with "failed to compare composter (itemtype) with a block" + set block at spawn of world "world" to {_block} diff --git a/src/test/skript/tests/regressions/pull-5566-block iterator being 100 always.sk b/src/test/skript/tests/regressions/pull-5566-block iterator being 100 always.sk index 3eb14b86d94..ef77c2955f5 100644 --- a/src/test/skript/tests/regressions/pull-5566-block iterator being 100 always.sk +++ b/src/test/skript/tests/regressions/pull-5566-block iterator being 100 always.sk @@ -1,5 +1,5 @@ # Test not related to a made issue. Details at pull request. test "100 blocks fix": - assert blocks 5 below spawn of world "world" contains air, grass block, dirt block, dirt block, bedrock block and void air with "Failed to get correct blocks" + assert blocks 5 below spawn of world "world" contains air, grass block, dirt block, dirt block, bedrock block and void air with "Failed to get correct blocks (got '%blocks 5 below spawn of world "world"%')" assert size of blocks 3 below location below spawn of world "world" is 4 with "Failed to match asserted size" diff --git a/src/test/skript/tests/syntaxes/expressions/ExprBlockData.sk b/src/test/skript/tests/syntaxes/expressions/ExprBlockData.sk index 1916c370f95..8fd992f8ce1 100644 --- a/src/test/skript/tests/syntaxes/expressions/ExprBlockData.sk +++ b/src/test/skript/tests/syntaxes/expressions/ExprBlockData.sk @@ -1,4 +1,4 @@ -test "block data" when running minecraft "1.15.2": +test "block data" when running minecraft "1.14.4": set {_b} to block at spawn of world "world" set block at {_b} to campfire[lit=false;waterlogged=true] assert block at {_b} is an unlit campfire with "block at spawn should be an unlit campfire"