diff --git a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java index baa160890c8..0f72b2cb87e 100644 --- a/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java +++ b/src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java @@ -28,6 +28,7 @@ import ch.njol.skript.lang.ExpressionType; import ch.njol.skript.lang.Literal; import ch.njol.skript.lang.SkriptParser.ParseResult; +import ch.njol.skript.lang.UnparsedLiteral; import ch.njol.skript.lang.util.SimpleExpression; import ch.njol.skript.lang.util.SimpleLiteral; import ch.njol.skript.registrations.Classes; @@ -117,43 +118,151 @@ public PatternInfo(Operator operator, boolean leftGrouped, boolean rightGrouped) private boolean leftGrouped, rightGrouped; @Override - @SuppressWarnings({"ConstantConditions", "unchecked"}) + @SuppressWarnings({"ConstantConditions", "rawtypes", "unchecked"}) public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) { - first = LiteralUtils.defendExpression(exprs[0]); - second = LiteralUtils.defendExpression(exprs[1]); - - if (!LiteralUtils.canInitSafely(first, second)) - return false; - - Class firstClass = first.getReturnType(); - Class secondClass = second.getReturnType(); + first = (Expression) exprs[0]; + second = (Expression) exprs[1]; PatternInfo patternInfo = patterns.getInfo(matchedPattern); leftGrouped = patternInfo.leftGrouped; rightGrouped = patternInfo.rightGrouped; operator = patternInfo.operator; - if (firstClass != Object.class && secondClass == Object.class && Arithmetics.getOperations(operator, firstClass).isEmpty()) { - // If the first class is known but doesn't have any operations, then we fail - return error(firstClass, secondClass); - } else if (firstClass == Object.class && secondClass != Object.class && Arithmetics.getOperations(operator).stream() - .noneMatch(info -> info.getRight().isAssignableFrom(secondClass))) { - // If the second class is known but doesn't have any operations, then we fail - return error(firstClass, secondClass); + /* + * Step 1: UnparsedLiteral Resolving + * + * Since Arithmetic may be performed on a variety of types, it is possible that 'first' or 'second' + * will represent unparsed literals. That is, the parser could not determine what their literal contents represent. + * Thus, it is now up to this expression to determine what they mean. + * + * If there are no unparsed literals, nothing happens at this step. + * If there are unparsed literals, one of three possible execution flows will occur: + * + * Case 1. 'first' and 'second' are unparsed literals + * In this case, there is not a lot of information to work with. + * 'first' and 'second' are attempted to be converted to fit one of all operations using 'operator'. + * If they cannot be matched with the types of a known operation, init will fail. + * + * Case 2. 'first' is an unparsed literal, 'second' is not + * In this case, 'first' needs to be converted into the "left" type of + * any operation using 'operator' with the type of 'second' as the right type. + * If 'first' cannot be converted, init will fail. + * If no operations are found for converting 'first', init will fail, UNLESS the type of 'second' is object, + * where operations will be searched again later with the context of the type of first. + * TODO When 'first' can represent multiple literals, it might be worth checking which of those can work with 'operator' and 'second' + * + * Case 3. 'second' is an unparsed literal, 'first' is not + * In this case, 'second' needs to be converted into the "right" type of + * any operation using 'operator' with the type of 'first' as the "left" type. + * If 'second' cannot be converted, init will fail. + * If no operations are found for converting 'second', init will fail, UNLESS the type of 'first' is object, + * where operations will be searched again later with the context of the type of second. + * TODO When 'second' can represent multiple literals, it might be worth checking which of those can work with 'first' and 'operator' + */ + + if (first instanceof UnparsedLiteral) { + if (second instanceof UnparsedLiteral) { // first and second need converting + for (OperationInfo operation : Arithmetics.getOperations(operator)) { + // match left type with 'first' + Expression convertedFirst = first.getConvertedExpression(operation.getLeft()); + if (convertedFirst == null) + continue; + // match right type with 'second' + Expression convertedSecond = second.getConvertedExpression(operation.getRight()); + if (convertedSecond == null) + continue; + // success, set the values + first = (Expression) convertedFirst; + second = (Expression) convertedSecond; + returnType = (Class) operation.getReturnType(); + } + } else { // first needs converting + // attempt to convert to types that make valid operations with + Class secondClass = second.getReturnType(); + Class[] leftTypes = Arithmetics.getOperations(operator).stream() + .filter(info -> info.getRight().isAssignableFrom(secondClass)) + .map(OperationInfo::getLeft) + .toArray(Class[]::new); + if (leftTypes.length == 0) { // no known operations with second's type + if (secondClass != Object.class) // there won't be any operations + return error(first.getReturnType(), secondClass); + first = (Expression) first.getConvertedExpression(Object.class); + } else { + first = (Expression) first.getConvertedExpression(leftTypes); + } + } + } else if (second instanceof UnparsedLiteral) { // second needs converting + // attempt to convert to types that make valid operations with + Class firstClass = first.getReturnType(); + List> operations = Arithmetics.getOperations(operator, firstClass); + if (operations.isEmpty()) { // no known operations with first's type + if (firstClass != Object.class) // there won't be any operations + return error(firstClass, second.getReturnType()); + second = (Expression) second.getConvertedExpression(Object.class); + } else { + second = (Expression) second.getConvertedExpression(operations.stream() + .map(OperationInfo::getRight) + .toArray(Class[]::new) + ); + } } - OperationInfo operationInfo; + if (!LiteralUtils.canInitSafely(first, second)) // checks if there are still unparsed literals present + return false; + + /* + * Step 2: Return Type Calculation + * + * After the first step, everything that can be known about 'first' and 'second' during parsing is known. + * As a result, it is time to determine the return type of the operation. + * + * If the types of 'first' or 'second' are object, it is possible that multiple operations with different return types + * will be found. If that is the case, the supertype of these operations will be the return type (can be object). + * If the types of both are object (e.g. variables), the return type will be object (have to wait until runtime and hope it works). + * Of course, if no operations are found, init will fail. + * + * After these checks, it is safe to assume returnType has a value, as init should have failed by now if not. + * One final check is performed specifically for numerical types. + * Any numerical operation involving division or exponents have a return type of Double. + * Other operations will also return Double, UNLESS 'first' and 'second' are of integer types, in which case the return type will be Long. + * + * If the types of both are something meaningful, the search for a registered operation commences. + * If no operation can be found, init will fail. + */ + + Class firstClass = first.getReturnType(); + Class secondClass = second.getReturnType(); + if (firstClass == Object.class || secondClass == Object.class) { - // If either of the types is unknown, then we resolve the operation at runtime - operationInfo = null; - } else { - operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, firstClass, secondClass); - if (operationInfo == null) // We error if we couldn't find an operation between the two types + // if either of the types is unknown, then we resolve the operation at runtime + Class[] returnTypes = null; + if (!(firstClass == Object.class && secondClass == Object.class)) { // both aren't object + if (firstClass == Object.class) { + returnTypes = Arithmetics.getOperations(operator).stream() + .filter(info -> info.getRight().isAssignableFrom(secondClass)) + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + } else { // secondClass is Object + returnTypes = Arithmetics.getOperations(operator, firstClass).stream() + .map(OperationInfo::getReturnType) + .toArray(Class[]::new); + } + } + if (returnTypes == null) { // both are object; can't determine anything + returnType = (Class) Object.class; + } else if (returnTypes.length == 0) { // one of the classes is known but doesn't have any operations + return error(firstClass, secondClass); + } else { + returnType = (Class) Classes.getSuperClassInfo(returnTypes).getC(); + } + } else if (returnType == null) { // lookup + OperationInfo operationInfo = (OperationInfo) Arithmetics.lookupOperationInfo(operator, firstClass, secondClass); + if (operationInfo == null) // we error if we couldn't find an operation between the two types return error(firstClass, secondClass); + returnType = operationInfo.getReturnType(); } - returnType = operationInfo == null ? (Class) Object.class : operationInfo.getReturnType(); - + // ensure proper return types for numerical operations if (Number.class.isAssignableFrom(returnType)) { if (operator == Operator.DIVISION || operator == Operator.EXPONENTIATION) { returnType = (Class) Double.class; @@ -164,19 +273,31 @@ public boolean init(Expression[] exprs, int matchedPattern, Kleenean isDelaye firstIsInt |= i.isAssignableFrom(first.getReturnType()); secondIsInt |= i.isAssignableFrom(second.getReturnType()); } - returnType = (Class) (firstIsInt && secondIsInt ? Long.class : Double.class); } } - // Chaining - if (first instanceof ExprArithmetic && !leftGrouped) { + /* + * Step 3: Chaining and Parsing + * + * This step builds the arithmetic chain that will be parsed into an ordered operation to be executed at runtime. + * With larger operations, it is possible that 'first' or 'second' will be instances of ExprArithmetic. + * As a result, their chains need to be incorporated into this instance's chain. + * This is to ensure that, during parsing, a "gettable" that follows the order of operations is built. + * However, in the case of parentheses, the chains will not be combined as the + * order of operations dictates that the result of that chain be determined first. + * + * The chain (a list of values and operators) will then be parsed into a "gettable" that + * can be evaluated during runtime for a final result. + */ + + if (first instanceof ExprArithmetic && !leftGrouped) { // combine chain of 'first' if we do not have parentheses chain.addAll(((ExprArithmetic) first).chain); } else { chain.add(first); } chain.add(operator); - if (second instanceof ExprArithmetic && !rightGrouped) { + if (second instanceof ExprArithmetic && !rightGrouped) { // combine chain of 'second' if we do not have parentheses chain.addAll(((ExprArithmetic) second).chain); } else { chain.add(second); @@ -197,7 +318,8 @@ protected T[] get(Event event) { private boolean error(Class firstClass, Class secondClass) { ClassInfo first = Classes.getSuperClassInfo(firstClass), second = Classes.getSuperClassInfo(secondClass); - Skript.error(operator.getName() + " can't be performed on " + first.getName().withIndefiniteArticle() + " and " + second.getName().withIndefiniteArticle()); + if (first.getC() != Object.class && second.getC() != Object.class) // errors with "object" are not very useful and often misleading + Skript.error(operator.getName() + " can't be performed on " + first.getName().withIndefiniteArticle() + " and " + second.getName().withIndefiniteArticle()); return false; } diff --git a/src/main/java/ch/njol/skript/log/ParseLogHandler.java b/src/main/java/ch/njol/skript/log/ParseLogHandler.java index dea3cc3cc8e..716464da53f 100644 --- a/src/main/java/ch/njol/skript/log/ParseLogHandler.java +++ b/src/main/java/ch/njol/skript/log/ParseLogHandler.java @@ -18,8 +18,9 @@ */ package ch.njol.skript.log; -import ch.njol.skript.Skript; import org.eclipse.jdt.annotation.Nullable; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.Contract; import java.util.ArrayList; import java.util.List; @@ -31,6 +32,29 @@ public class ParseLogHandler extends LogHandler { private LogEntry error = null; private final List log = new ArrayList<>(); + + /** + * Internal method for creating a backup of this log. + * @return A new ParseLogHandler containing the contents of this ParseLogHandler. + */ + @ApiStatus.Internal + @Contract("-> new") + public ParseLogHandler backup() { + ParseLogHandler copy = new ParseLogHandler(); + copy.error = this.error; + copy.log.addAll(this.log); + return copy; + } + + /** + * Internal method for restoring a backup of this log. + */ + @ApiStatus.Internal + public void restore(ParseLogHandler parseLogHandler) { + this.error = parseLogHandler.error; + this.log.clear(); + this.log.addAll(parseLogHandler.log); + } @Override public LogResult log(LogEntry entry) { diff --git a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java index f3cbdb5f666..1ecdfee11cc 100644 --- a/src/main/java/ch/njol/skript/patterns/TypePatternElement.java +++ b/src/main/java/ch/njol/skript/patterns/TypePatternElement.java @@ -24,6 +24,7 @@ import ch.njol.skript.lang.Literal; import ch.njol.skript.lang.SkriptParser; import ch.njol.skript.lang.SkriptParser.ExprInfo; +import ch.njol.skript.lang.UnparsedLiteral; import ch.njol.skript.lang.parser.ParserInstance; import ch.njol.skript.log.ErrorQuality; import ch.njol.skript.log.ParseLogHandler; @@ -138,6 +139,10 @@ public MatchResult match(String expr, MatchResult matchResult) { ExprInfo exprInfo = getExprInfo(); + MatchResult matchBackup = null; + ParseLogHandler loopLogHandlerBackup = null; + ParseLogHandler expressionLogHandlerBackup = null; + ParseLogHandler loopLogHandler = SkriptLogger.startParseLogHandler(); try { while (newExprOffset != -1) { @@ -168,11 +173,37 @@ public MatchResult match(String expr, MatchResult matchResult) { } } - expressionLogHandler.printLog(); - loopLogHandler.printLog(); - newMatchResult.expressions[expressionIndex] = expression; - return newMatchResult; + + /* + * the parser will return unparsed literals in cases where it cannot interpret an input and object is the desired return type. + * in those cases, it is up to the expression to interpret the input. + * however, this presents a problem for input that is not intended as being one of these object-accepting expressions. + * these object-accepting expressions will be matched instead but their parsing will fail as they cannot interpret the unparsed literals. + * even though it can't interpret them, this loop will have returned a match and thus parsing has ended (and the correct interpretation never attempted). + * to avoid this issue, while also permitting unparsed literals in cases where they are justified, + * the code below forces the loop to continue in hopes of finding a match without unparsed literals. + * if it is unsuccessful, a backup of the first successful match (with unparsed literals) is saved to be returned. + */ + boolean hasUnparsedLiteral = false; + for (int i = expressionIndex + 1; i < newMatchResult.expressions.length; i++) { + if (newMatchResult.expressions[i] instanceof UnparsedLiteral) { + hasUnparsedLiteral = Classes.parse(((UnparsedLiteral) newMatchResult.expressions[i]).getData(), Object.class, newMatchResult.parseContext) == null; + if (hasUnparsedLiteral) { + break; + } + } + } + + if (!hasUnparsedLiteral) { + expressionLogHandler.printLog(); + loopLogHandler.printLog(); + return newMatchResult; + } else if (matchBackup == null) { // only backup the first occurrence of unparsed literals + matchBackup = newMatchResult; + loopLogHandlerBackup = loopLogHandler.backup(); + expressionLogHandlerBackup = expressionLogHandler.backup(); + } } } finally { expressionLogHandler.printError(); @@ -193,11 +224,19 @@ public MatchResult match(String expr, MatchResult matchResult) { } } } finally { - if (!loopLogHandler.isStopped()) + if (loopLogHandlerBackup != null) { // print backup logs if applicable + loopLogHandler.restore(loopLogHandlerBackup); + assert expressionLogHandlerBackup != null; + expressionLogHandlerBackup.printLog(); + } + if (!loopLogHandler.isStopped()) { loopLogHandler.printError(); + } } - return null; + // if there were unparsed literals, we will return the backup now + // if there were not, this returns null + return matchBackup; } @Override diff --git a/src/test/skript/tests/misc/invalid unparsed literals.sk b/src/test/skript/tests/misc/invalid unparsed literals.sk new file mode 100644 index 00000000000..6d076a7e079 --- /dev/null +++ b/src/test/skript/tests/misc/invalid unparsed literals.sk @@ -0,0 +1,6 @@ +test "invalid unparsed literals": + + parse: + loop 9 times: + set {_i} to loop-value - 1 + assert last parse logs is not set with "skript should be able to understand 'loop-value - 1' but did not" diff --git a/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk b/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk index f035aecd41e..16a526d5f7e 100644 --- a/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk +++ b/src/test/skript/tests/syntaxes/expressions/ExprArithmetic.sk @@ -255,3 +255,10 @@ local function component_equals(a: number, b: number) :: boolean: then: return true return true if {_a} is {_b}, else false + +test "arithmetic return types": + # the issue below is that is now returned for the function + # skript interprets this as "y-coordinate of ({_location} - 4)" which is valid due to Object.class return types + # however, we can get more specific return types by returning the superclass of the return types of all Object-Number operations + set {_location} to location(0,10,0,"world") + assert (y-coordinate of {_location} - 4) is 6 with "y-coordinate of {_location} - 4 is not 6 (got '%y-coordinate of {_location} - 4%')"