diff --git a/src/main/java/ch/njol/skript/lang/SkriptParser.java b/src/main/java/ch/njol/skript/lang/SkriptParser.java index 4bffc2a4225..f16de9686d3 100644 --- a/src/main/java/ch/njol/skript/lang/SkriptParser.java +++ b/src/main/java/ch/njol/skript/lang/SkriptParser.java @@ -64,6 +64,8 @@ import java.util.Locale; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import java.util.regex.MatchResult; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -637,7 +639,6 @@ public Expression parseExpression(Class... types) assert types != null && types.length > 0; assert types.length == 1 || !CollectionUtils.contains(types, Object.class); - boolean isObject = types.length == 1 && types[0] == Object.class; ParseLogHandler log = SkriptLogger.startParseLogHandler(); try { Expression parsedExpression = parseSingleExpr(true, null, types); @@ -647,106 +648,113 @@ public Expression parseExpression(Class... types) } log.clear(); - List> parsedExpressions = new ArrayList<>(); - Kleenean and = Kleenean.UNKNOWN; - boolean isLiteralList = true; + return this.parseExpressionList(log, types); + } finally { + log.stop(); + } + } - List pieces = new ArrayList<>(); - { - Matcher matcher = LIST_SPLIT_PATTERN.matcher(expr); - int i = 0, j = 0; - for (; i >= 0 && i <= expr.length(); i = next(expr, i, context)) { - if (i == expr.length() || matcher.region(i, expr.length()).lookingAt()) { - pieces.add(new int[] {j, i}); - if (i == expr.length()) - break; - j = i = matcher.end(); - } - } - if (i != expr.length()) { - assert i == -1 && context != ParseContext.COMMAND && context != ParseContext.PARSE : i + "; " + expr; - log.printError("Invalid brackets/variables/text in '" + expr + "'", ErrorQuality.NOT_AN_EXPRESSION); - return null; + @Nullable + private Expression parseExpressionList(ParseLogHandler log, Class... types) { + boolean isObject = types.length == 1 && types[0] == Object.class; + List> parsedExpressions = new ArrayList<>(); + Kleenean and = Kleenean.UNKNOWN; + boolean isLiteralList = true; + Expression parsedExpression; + + List pieces = new ArrayList<>(); + { + Matcher matcher = LIST_SPLIT_PATTERN.matcher(expr); + int i = 0, j = 0; + for (; i >= 0 && i <= expr.length(); i = next(expr, i, context)) { + if (i == expr.length() || matcher.region(i, expr.length()).lookingAt()) { + pieces.add(new int[] {j, i}); + if (i == expr.length()) + break; + j = i = matcher.end(); } } - - if (pieces.size() == 1) { // not a list of expressions, and a single one has failed to parse above - if (expr.startsWith("(") && expr.endsWith(")") && next(expr, 0, context) == expr.length()) { - log.clear(); - return new SkriptParser(this, "" + expr.substring(1, expr.length() - 1)).parseExpression(types); - } - if (isObject && (flags & PARSE_LITERALS) != 0) { // single expression - can return an UnparsedLiteral now - log.clear(); - return (Expression) new UnparsedLiteral(expr, log.getError()); - } - // results in useless errors most of the time -// log.printError("'" + expr + "' " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); - log.printError(); + if (i != expr.length()) { + assert i == -1 && context != ParseContext.COMMAND && context != ParseContext.PARSE : i + "; " + expr; + log.printError("Invalid brackets/variables/text in '" + expr + "'", ErrorQuality.NOT_AN_EXPRESSION); return null; } + } - outer: for (int first = 0; first < pieces.size();) { - for (int last = 1; last <= pieces.size() - first; last++) { - if (first == 0 && last == pieces.size()) // i.e. the whole expression - already tried to parse above - continue; - int start = pieces.get(first)[0], end = pieces.get(first + last - 1)[1]; - String subExpr = "" + expr.substring(start, end).trim(); - assert subExpr.length() < expr.length() : subExpr; + if (pieces.size() == 1) { // not a list of expressions, and a single one has failed to parse above + if (expr.startsWith("(") && expr.endsWith(")") && next(expr, 0, context) == expr.length()) { + log.clear(); + return new SkriptParser(this, "" + expr.substring(1, expr.length() - 1)).parseExpression(types); + } + if (isObject && (flags & PARSE_LITERALS) != 0) { // single expression - can return an UnparsedLiteral now + log.clear(); + return (Expression) new UnparsedLiteral(expr, log.getError()); + } + // results in useless errors most of the time +// log.printError("'" + expr + "' " + Language.get("is") + " " + notOfType(types), ErrorQuality.NOT_AN_EXPRESSION); + log.printError(); + return null; + } - if (subExpr.startsWith("(") && subExpr.endsWith(")") && next(subExpr, 0, context) == subExpr.length()) - parsedExpression = new SkriptParser(this, subExpr).parseExpression(types); // only parse as possible expression list if its surrounded by brackets - else - parsedExpression = new SkriptParser(this, subExpr).parseSingleExpr(last == 1, log.getError(), types); // otherwise parse as a single expression only - if (parsedExpression != null) { - isLiteralList &= parsedExpression instanceof Literal; - parsedExpressions.add(parsedExpression); - if (first != 0) { - String delimiter = expr.substring(pieces.get(first - 1)[1], start).trim().toLowerCase(Locale.ENGLISH); - if (!delimiter.equals(",")) { - boolean or = !delimiter.contains("nor") && delimiter.endsWith("or"); - if (and.isUnknown()) { - and = Kleenean.get(!or); // nor is and - } else { - if (and != Kleenean.get(!or)) { - Skript.warning(MULTIPLE_AND_OR + " List: " + expr); - and = Kleenean.TRUE; - } + outer: for (int first = 0; first < pieces.size();) { + for (int last = 1; last <= pieces.size() - first; last++) { + if (first == 0 && last == pieces.size()) // i.e. the whole expression - already tried to parse above + continue; + int start = pieces.get(first)[0], end = pieces.get(first + last - 1)[1]; + String subExpr = "" + expr.substring(start, end).trim(); + assert subExpr.length() < expr.length() : subExpr; + + if (subExpr.startsWith("(") && subExpr.endsWith(")") && next(subExpr, 0, context) == subExpr.length()) + parsedExpression = new SkriptParser(this, subExpr).parseExpression(types); // only parse as possible expression list if its surrounded by brackets + else + parsedExpression = new SkriptParser(this, subExpr).parseSingleExpr(last == 1, log.getError(), types); // otherwise parse as a single expression only + if (parsedExpression != null) { + isLiteralList &= parsedExpression instanceof Literal; + parsedExpressions.add(parsedExpression); + if (first != 0) { + String delimiter = expr.substring(pieces.get(first - 1)[1], start).trim().toLowerCase(Locale.ENGLISH); + if (!delimiter.equals(",")) { + boolean or = !delimiter.contains("nor") && delimiter.endsWith("or"); + if (and.isUnknown()) { + and = Kleenean.get(!or); // nor is and + } else { + if (and != Kleenean.get(!or)) { + Skript.warning(MULTIPLE_AND_OR + " List: " + expr); + and = Kleenean.TRUE; } } } - first += last; - continue outer; } + first += last; + continue outer; } - log.printError(); - return null; } + log.printError(); + return null; + } - log.printLog(false); + log.printLog(false); - if (parsedExpressions.size() == 1) - return parsedExpressions.get(0); + if (parsedExpressions.size() == 1) + return parsedExpressions.get(0); - if (and.isUnknown() && !suppressMissingAndOrWarnings) { - ParserInstance parser = getParser(); - Script currentScript = parser.isActive() ? parser.getCurrentScript() : null; - if (currentScript == null || !currentScript.suppressesWarning(ScriptWarning.MISSING_CONJUNCTION)) - Skript.warning(MISSING_AND_OR + ": " + expr); - } + if (and.isUnknown() && !suppressMissingAndOrWarnings) { + ParserInstance parser = getParser(); + Script currentScript = parser.isActive() ? parser.getCurrentScript() : null; + if (currentScript == null || !currentScript.suppressesWarning(ScriptWarning.MISSING_CONJUNCTION)) + Skript.warning(MISSING_AND_OR + ": " + expr); + } - Class[] exprReturnTypes = new Class[parsedExpressions.size()]; - for (int i = 0; i < parsedExpressions.size(); i++) - exprReturnTypes[i] = parsedExpressions.get(i).getReturnType(); + Class[] exprReturnTypes = new Class[parsedExpressions.size()]; + for (int i = 0; i < parsedExpressions.size(); i++) + exprReturnTypes[i] = parsedExpressions.get(i).getReturnType(); - if (isLiteralList) { - Literal[] literals = parsedExpressions.toArray(new Literal[parsedExpressions.size()]); - return new LiteralList<>(literals, (Class) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse()); - } else { - Expression[] expressions = parsedExpressions.toArray(new Expression[parsedExpressions.size()]); - return new ExpressionList<>(expressions, (Class) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse()); - } - } finally { - log.stop(); + if (isLiteralList) { + Literal[] literals = parsedExpressions.toArray(new Literal[parsedExpressions.size()]); + return new LiteralList<>(literals, (Class) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse()); + } else { + Expression[] expressions = parsedExpressions.toArray(new Expression[parsedExpressions.size()]); + return new ExpressionList<>(expressions, (Class) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse()); } } @@ -898,6 +906,7 @@ public FunctionReference parseFunction(@Nullable Class... ty if (context != ParseContext.DEFAULT && context != ParseContext.EVENT) return null; ParseLogHandler log = SkriptLogger.startParseLogHandler(); + AtomicBoolean unaryArgument = new AtomicBoolean(false); try { Matcher matcher = FUNCTION_CALL_PATTERN.matcher(expr); if (!matcher.matches()) { @@ -923,32 +932,30 @@ public FunctionReference parseFunction(@Nullable Class... ty log.printError(); return null; } - - if (args.length() != 0) { - Expression parsedExpression = new SkriptParser(args, flags | PARSE_LITERALS, context).suppressMissingAndOrWarnings().parseExpression(Object.class); - if (parsedExpression == null) { - log.printError(); - return null; - } - if (parsedExpression instanceof ExpressionList) { - if (!parsedExpression.getAnd()) { - Skript.error("Function arguments must be separated by commas and optionally an 'and', but not an 'or'." - + " Put the 'or' into a second set of parentheses if you want to make it a single parameter, e.g. 'give(player, (sword or axe))'"); - log.printError(); - return null; - } - params = ((ExpressionList) parsedExpression).getExpressions(); - } else { - params = new Expression[] {parsedExpression}; - } - } else { - params = new Expression[0]; + final SkriptParser skriptParser = new SkriptParser(args, flags | PARSE_LITERALS, context); + params = this.getFunctionArguments(() -> skriptParser.suppressMissingAndOrWarnings().parseExpression(Object.class), args, unaryArgument); + if (params == null) { + log.printError(); + return null; } ParserInstance parser = getParser(); Script currentScript = parser.isActive() ? parser.getCurrentScript() : null; FunctionReference functionReference = new FunctionReference<>(functionName, SkriptLogger.getNode(), currentScript != null ? currentScript.getConfig().getFileName() : null, types, params);//.toArray(new Expression[params.size()])); + attempt_list_parse: + if (unaryArgument.get() && !functionReference.validateParameterArity(true)) { + try (ParseLogHandler ignored = SkriptLogger.startParseLogHandler()) { + SkriptParser alternative = new SkriptParser(args, flags | PARSE_LITERALS, context); + params = this.getFunctionArguments(() -> alternative.suppressMissingAndOrWarnings() + .parseExpressionList(ignored, Object.class), args, unaryArgument); + ignored.clear(); + if (params == null) + break attempt_list_parse; + } + functionReference = new FunctionReference<>(functionName, SkriptLogger.getNode(), + currentScript != null ? currentScript.getConfig().getFileName() : null, types, params); + } if (!functionReference.validateFunction(true)) { log.printError(); return null; @@ -960,6 +967,29 @@ public FunctionReference parseFunction(@Nullable Class... ty } } + private Expression @Nullable [] getFunctionArguments(Supplier> parsing, String args, AtomicBoolean unary) { + Expression[] params; + if (args.length() != 0) { + Expression parsedExpression = parsing.get(); + if (parsedExpression == null) + return null; + if (parsedExpression instanceof ExpressionList) { + if (!parsedExpression.getAnd()) { + Skript.error("Function arguments must be separated by commas and optionally an 'and', but not an 'or'." + + " Put the 'or' into a second set of parentheses if you want to make it a single parameter, e.g. 'give(player, (sword or axe))'"); + return null; + } + params = ((ExpressionList) parsedExpression).getExpressions(); + } else { + unary.set(true); + params = new Expression[] {parsedExpression}; + } + } else { + params = new Expression[0]; + } + return params; + } + /** * Prints parse errors (i.e. must start a ParseLog before calling this method) */ diff --git a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java index a12bd8f735f..01feacaf2a5 100644 --- a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java +++ b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java @@ -108,6 +108,16 @@ public FunctionReference( this.returnTypes = returnTypes; parameters = params; } + + public boolean validateParameterArity(boolean first) { + if (!first && script == null) + return false; + Signature sign = Functions.getSignature(functionName, script); + if (sign == null) + return false; + // Not enough parameters + return parameters.length >= sign.getMinParameters(); + } /** * Validates this function reference. Prints errors if needed. diff --git a/src/test/skript/tests/regressions/4988-function uuid multiple parameters.sk b/src/test/skript/tests/regressions/4988-function uuid multiple parameters.sk new file mode 100644 index 00000000000..891acea46c5 --- /dev/null +++ b/src/test/skript/tests/regressions/4988-function uuid multiple parameters.sk @@ -0,0 +1,36 @@ + +# The exact function from the issue +local function test(uuid: string, s: string) :: boolean: + if {_uuid} is set: + if {_s} is set: + return true + return false + +local function return_a(a: string, b: string) :: string: + return {_a} + +local function return_b(a: string, b: string) :: string: + return {_b} + +test "4988 function uuid multiple parameters": + + set {_var} to return_a(uuid of {_nothing}, "hello") + assert {_var} doesn't exist with "first parameter was wrong: %{_var}%" + + set {_var} to return_b(uuid of {_nothing}, "hello") + assert {_var} is "hello" with "second parameter was wrong: %{_var}%" + delete {_var} + + # This failed to parse in the original issue + set {_var} to test(uuid of {_nothing}, "foo") + assert {_var} is false with "function parameters were set (1): %{_var}%" + + # This should parse fine + set {_uuid} to uuid of {_nothing} + set {_var} to test({_uuid}, "foo") + assert {_var} is false with "function parameters were set (2): %{_var}%" + + # This should also parse fine + set {_var} to test((uuid of {_nothing}), "foo") + assert {_var} is false with "function parameters were set (3) %{_var}%" +