Skip to content

Commit

Permalink
Fix function parsing with ambiguous lists in parameters. (#6579)
Browse files Browse the repository at this point in the history 
Fix function parsing with ambiguous lists.
  • Loading branch information
@Moderocky
Moderocky authored Apr 24, 2024
1 parent 7831c9e commit 3e55190
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 103 deletions.
236 changes: 133 additions & 103 deletions src/main/java/ch/njol/skript/lang/SkriptParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -637,7 +639,6 @@ public <T> Expression<? extends T> parseExpression(Class<? extends T>... 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<? extends T> parsedExpression = parseSingleExpr(true, null, types);
Expand All @@ -647,106 +648,113 @@ public <T> Expression<? extends T> parseExpression(Class<? extends T>... types)
}
log.clear();

List<Expression<? extends T>> parsedExpressions = new ArrayList<>();
Kleenean and = Kleenean.UNKNOWN;
boolean isLiteralList = true;
return this.parseExpressionList(log, types);
} finally {
log.stop();
}
}

List<int[]> 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 <T> Expression<? extends T> parseExpressionList(ParseLogHandler log, Class<? extends T>... types) {
boolean isObject = types.length == 1 && types[0] == Object.class;
List<Expression<? extends T>> parsedExpressions = new ArrayList<>();
Kleenean and = Kleenean.UNKNOWN;
boolean isLiteralList = true;
Expression<? extends T> parsedExpression;

List<int[]> 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<? extends T>) 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<? extends T>) 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<? extends T>[] exprReturnTypes = new Class[parsedExpressions.size()];
for (int i = 0; i < parsedExpressions.size(); i++)
exprReturnTypes[i] = parsedExpressions.get(i).getReturnType();
Class<? extends T>[] exprReturnTypes = new Class[parsedExpressions.size()];
for (int i = 0; i < parsedExpressions.size(); i++)
exprReturnTypes[i] = parsedExpressions.get(i).getReturnType();

if (isLiteralList) {
Literal<T>[] literals = parsedExpressions.toArray(new Literal[parsedExpressions.size()]);
return new LiteralList<>(literals, (Class<T>) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse());
} else {
Expression<T>[] expressions = parsedExpressions.toArray(new Expression[parsedExpressions.size()]);
return new ExpressionList<>(expressions, (Class<T>) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse());
}
} finally {
log.stop();
if (isLiteralList) {
Literal<T>[] literals = parsedExpressions.toArray(new Literal[parsedExpressions.size()]);
return new LiteralList<>(literals, (Class<T>) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse());
} else {
Expression<T>[] expressions = parsedExpressions.toArray(new Expression[parsedExpressions.size()]);
return new ExpressionList<>(expressions, (Class<T>) Classes.getSuperClassInfo(exprReturnTypes).getC(), !and.isFalse());
}
}

Expand Down Expand Up @@ -898,6 +906,7 @@ public <T> FunctionReference<T> parseFunction(@Nullable Class<? extends T>... 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()) {
Expand All @@ -923,32 +932,30 @@ public <T> FunctionReference<T> parseFunction(@Nullable Class<? extends T>... 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<T> 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;
Expand All @@ -960,6 +967,29 @@ public <T> FunctionReference<T> parseFunction(@Nullable Class<? extends T>... ty
}
}

private Expression<?> @Nullable [] getFunctionArguments(Supplier<Expression<?>> 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)
*/
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/ch/njol/skript/lang/function/FunctionReference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
36 changes: 36 additions & 0 deletions src/test/skript/tests/regressions/4988-function uuid multiple parameters.sk
View file
Original file line number Diff line number Diff line change
@@ -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}%"

0 comments on commit 3e55190

Please sign in to comment.