Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix function parsing with ambiguous lists in parameters. #6579

Merged
merged 3 commits into from
Apr 24, 2024
Merged

Fix function parsing with ambiguous lists in parameters. #6579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4dff908
Fix function parsing with ambiguous lists.
Moderocky Apr 17, 2024
f53fcf0
Merge branch 'dev/patch' into multi-uuid-issue
Moderocky Apr 23, 2024
d800f83
Merge branch 'dev/patch' into multi-uuid-issue
Moderocky Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
236 changes: 133 additions & 103 deletions src/main/java/ch/njol/skript/lang/SkriptParser.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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}%"
Moderocky marked this conversation as resolved.
Show resolved Hide resolved

Loading