Skip to content

Function package rework and named function arguments #8112

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.

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

Open
wants to merge 56 commits into
base: dev/feature
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
c47f5f6
add stuff from java funciton rework
Efnilite Aug 2, 2025
11d29b2
add argument parser
Efnilite Aug 2, 2025
1e050f0
update parameter
Efnilite Aug 2, 2025
e0f213b
add getSignatures
Efnilite Aug 2, 2025
8fdead3
update tests
Efnilite Aug 2, 2025
20686a9
implement FunctionParser
Efnilite Aug 2, 2025
afd75f4
fix implementation for Param, SF, Sign
Efnilite Aug 2, 2025
380a85b
fix Function#execute
Efnilite Aug 2, 2025
a052853
update Documentation to use new Param
Efnilite Aug 2, 2025
bfe101d
fix EffFunctionCall
Efnilite Aug 2, 2025
1c1d184
add "" strings to parser
Efnilite Aug 2, 2025
5dd23d9
minor updates
Efnilite Aug 2, 2025
75a4ddb
move script param parsing to StructFunction, ScriptParameter
Efnilite Aug 2, 2025
6f60e66
make junit pass
Efnilite Aug 2, 2025
0f9673a
update Signature to be more private
Efnilite Aug 2, 2025
b4ae954
fix max({_list::*}) not selecting properly
Efnilite Aug 2, 2025
7731c2e
fix errors
Efnilite Aug 2, 2025
7c8f21f
add types to ExprFunctionCall
Efnilite Aug 2, 2025
af30556
fix clamp
Efnilite Aug 2, 2025
25fccca
fix keyed errors
Efnilite Aug 2, 2025
52faf48
attempt to fix keyed
Efnilite Aug 2, 2025
e5e9470
deprecate, add test
Efnilite Aug 3, 2025
b6c79cc
minor changes
Efnilite Aug 3, 2025
67eb2cc
add space
Efnilite Aug 3, 2025
e576dfe
fix errors
Efnilite Aug 3, 2025
382ec6f
fix local/local not being differentiable
Efnilite Aug 3, 2025
0fbfc6f
update already registered error
Efnilite Aug 3, 2025
2261cb3
disallow named/unnamed out of order arg mixing
Efnilite Aug 3, 2025
1f7a912
attempt to fix keyed again
Efnilite Aug 3, 2025
79f2617
Merge branch 'dev/feature' into feature/named-function-args
Efnilite Aug 3, 2025
d80c57a
fix exact matches not being matched before lists
Efnilite Aug 3, 2025
57de0b1
rename function -> reference
Efnilite Aug 3, 2025
a1f19aa
move FunctionParser to separate class
Efnilite Aug 3, 2025
86a9a97
move + improve error
Efnilite Aug 3, 2025
b7869cd
add Function#returnedType
Efnilite Aug 3, 2025
1812e57
add docs
Efnilite Aug 3, 2025
cc8dc52
IT'S ALIVE
Efnilite Aug 3, 2025
09b63c9
move argument parsing
Efnilite Aug 3, 2025
d18f55c
fix java 17 support
Efnilite Aug 3, 2025
38d960c
fix java 17 support again
Efnilite Aug 3, 2025
d412399
added more tests
Efnilite Aug 4, 2025
461bd10
added docs to FunctionArgumentParser
Efnilite Aug 4, 2025
c2d67f6
add errors to english.lang
Efnilite Aug 4, 2025
90cef5d
requested changes
Efnilite Aug 5, 2025
b2333a8
make parseFunctionReference public for multiline function arguments
Efnilite Aug 6, 2025
f145f2f
make FUNCTION_NAME_PATTERN a pattern
Efnilite Aug 6, 2025
eade6b0
add tests for unparsed literals
Efnilite Aug 7, 2025
5b152eb
use canInitSafely
Efnilite Aug 7, 2025
2fed5b8
move to common.function
Efnilite Aug 8, 2025
8176be9
fix tests
Efnilite Aug 9, 2025
82300ff
first changes
Efnilite Aug 9, 2025
f2119bb
further impl attempt
Efnilite Aug 9, 2025
f10b24b
Revert "further impl attempt"
Efnilite Aug 9, 2025
e2344c3
Revert "first changes"
Efnilite Aug 9, 2025
3b27ae0
rename FunctionParser
Efnilite Aug 11, 2025
5a450d7
update signature pattern
Efnilite Aug 11, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,12 @@ public Location[] execute(FunctionEvent<?> event, Object[][] params) {
return null;
}

World world = params[3].length == 1 ? (World) params[3][0] : Bukkit.getWorlds().get(0); // fallback to main world of server
World world;
if (params[3].length == 1 && params[3][0] != null) {
world = (World) params[3][0];
} else {
world = Bukkit.getWorlds().get(0);
}

return new Location[] {new Location(world,
((Number) params[0][0]).doubleValue(), ((Number) params[1][0]).doubleValue(), ((Number) params[2][0]).doubleValue(),
Expand Down
17 changes: 6 additions & 11 deletions src/main/java/ch/njol/skript/doc/Documentation.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import ch.njol.skript.lang.SyntaxElementInfo;
import ch.njol.skript.lang.function.Functions;
import ch.njol.skript.lang.function.JavaFunction;
import ch.njol.skript.lang.function.Parameter;
import ch.njol.skript.registrations.Classes;
import ch.njol.skript.util.Utils;
import ch.njol.util.NonNullPair;
Expand All @@ -17,13 +16,9 @@
import ch.njol.util.coll.iterator.IteratorIterable;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.jetbrains.annotations.Nullable;
import org.skriptlang.skript.common.function.Parameter;

import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
import java.io.*;
import java.util.ArrayList;
import java.util.function.Function;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -381,10 +376,10 @@ private static void insertClass(final PrintWriter pw, final ClassInfo<?> info) {
since);
}

private static void insertFunction(final PrintWriter pw, final JavaFunction<?> func) {
final StringBuilder params = new StringBuilder();
for (final Parameter<?> p : func.getParameters()) {
if (params.length() != 0)
private static void insertFunction(PrintWriter pw, JavaFunction<?> func) {
StringBuilder params = new StringBuilder();
for (Parameter<?> p : func.getSignature().parameters().values()) {
if (!params.isEmpty())
params.append(", ");
params.append(p.toString());
}
Expand Down
170 changes: 29 additions & 141 deletions src/main/java/ch/njol/skript/lang/SkriptParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import ch.njol.skript.expressions.ExprParse;
import ch.njol.skript.lang.DefaultExpressionUtils.DefaultExpressionError;
import ch.njol.skript.lang.function.ExprFunctionCall;
import ch.njol.skript.lang.function.FunctionReference;
import ch.njol.skript.lang.function.Functions;
import ch.njol.skript.lang.parser.DefaultValueData;
import ch.njol.skript.lang.parser.ParseStackOverflowException;
import ch.njol.skript.lang.parser.ParserInstance;
Expand Down Expand Up @@ -46,24 +44,17 @@
import org.skriptlang.skript.lang.converter.Converters;
import org.skriptlang.skript.lang.experiment.ExperimentSet;
import org.skriptlang.skript.lang.experiment.ExperimentalSyntax;
import org.skriptlang.skript.common.function.FunctionReferenceParser;
import org.skriptlang.skript.common.function.FunctionReference;
import org.skriptlang.skript.lang.script.Script;
import org.skriptlang.skript.lang.script.ScriptWarning;
import org.skriptlang.skript.registration.SyntaxInfo;
import org.skriptlang.skript.registration.SyntaxRegistry;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Deque;
import java.util.EnumMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.*;
import java.util.Map.Entry;
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 @@ -488,11 +479,10 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
log.printError();
return null;
}
FunctionReference<T> functionReference = parseFunction(types);
FunctionReference<T> functionReference = parseFunctionReference();
if (functionReference != null) {
log.printLog();
//noinspection rawtypes
return new ExprFunctionCall(functionReference);
return new ExprFunctionCall<>(functionReference, types);
} else if (log.hasError()) {
log.printError();
return null;
Expand Down Expand Up @@ -644,10 +634,9 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
}

// If it wasn't variable, do same for function call
FunctionReference<?> functionReference = parseFunction(types);
FunctionReference<?> functionReference = parseFunctionReference();
if (functionReference != null) {

if (onlySingular && !functionReference.isSingle()) {
if (onlySingular && !functionReference.single()) {
Skript.error("'" + expr + "' can only be a single "
+ Classes.toString(Stream.of(exprInfo.classes).map(classInfo -> classInfo.getName().toString()).toArray(), false)
+ ", not more.");
Expand All @@ -656,7 +645,7 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
}

log.printLog();
return new ExprFunctionCall<>(functionReference);
return new ExprFunctionCall<>(functionReference, types);
} else if (log.hasError()) {
log.printError();
return null;
Expand Down Expand Up @@ -876,16 +865,11 @@ private boolean checkAcceptedType(Class<?> clazz, Class<?> ... types) {
private final static String MULTIPLE_AND_OR = "List has multiple 'and' or 'or', will default to 'and'. Use brackets if you want to define multiple lists.";
private final static String MISSING_AND_OR = "List is missing 'and' or 'or', defaulting to 'and'";

private boolean suppressMissingAndOrWarnings = SkriptConfig.disableMissingAndOrWarnings.value();

private SkriptParser suppressMissingAndOrWarnings() {
suppressMissingAndOrWarnings = true;
return this;
}
private final boolean suppressMissingAndOrWarnings = SkriptConfig.disableMissingAndOrWarnings.value();

@SuppressWarnings("unchecked")
public <T> @Nullable Expression<? extends T> parseExpression(Class<? extends T>... types) {
if (expr.length() == 0)
if (expr.isEmpty())
return null;

assert types.length > 0;
Expand Down Expand Up @@ -1145,98 +1129,14 @@ private SkriptParser suppressMissingAndOrWarnings() {
}
}

private final static Pattern FUNCTION_CALL_PATTERN = Pattern.compile("(" + Functions.functionNamePattern + ")\\((.*)\\)");

/**
* @param types The required return type or null if it is not used (e.g. when calling a void function)
* @return The parsed function, or null if the given expression is not a function call or is an invalid function call (check for an error to differentiate these two)
* Attempts to parse {@link SkriptParser#expr} as a function reference.
*
* @param <T> The return type of the function.
* @return A {@link FunctionReference} if a function is found, or {@code null} if none is found.
*/
@SuppressWarnings("unchecked")
public <T> @Nullable FunctionReference<T> parseFunction(@Nullable Class<? extends T>... types) {
if (context != ParseContext.DEFAULT && context != ParseContext.EVENT)
return null;
AtomicBoolean unaryArgument = new AtomicBoolean(false);
try (ParseLogHandler log = SkriptLogger.startParseLogHandler()) {
Matcher matcher = FUNCTION_CALL_PATTERN.matcher(expr);
if (!matcher.matches()) {
log.printLog();
return null;
}

String functionName = "" + matcher.group(1);
String args = matcher.group(2);
Expression<?>[] params;

// Check for incorrect quotes, e.g. "myFunction() + otherFunction()" being parsed as one function
// See https://github.com/SkriptLang/Skript/issues/1532
for (int i = 0; i < args.length(); i = next(args, i, context)) {
if (i == -1) {
log.printLog();
return null;
}
}

if ((flags & PARSE_EXPRESSIONS) == 0) {
Skript.error("Functions cannot be used here (or there is a problem with your arguments).");
log.printError();
return null;
}
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);

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;
}
log.printLog();
return functionReference;
}
}

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;
public <T> FunctionReference<T> parseFunctionReference() {
return new FunctionReferenceParser(context, flags).parseFunctionReference(expr);
}

/**
Expand Down Expand Up @@ -1473,22 +1373,24 @@ public static int next(String expr, int startIndex, ParseContext context) {
return startIndex + 1;

int index;
switch (expr.charAt(startIndex)) {
case '"':
return switch (expr.charAt(startIndex)) {
case '"' -> {
index = nextQuote(expr, startIndex + 1);
return index < 0 ? -1 : index + 1;
case '{':
yield index < 0 ? -1 : index + 1;
}
case '{' -> {
index = VariableString.nextVariableBracket(expr, startIndex + 1);
return index < 0 ? -1 : index + 1;
case '(':
yield index < 0 ? -1 : index + 1;
}
case '(' -> {
for (index = startIndex + 1; index >= 0 && index < exprLength; index = next(expr, index, context)) {
if (expr.charAt(index) == ')')
return index + 1;
yield index + 1;
}
return -1;
default:
return startIndex + 1;
}
yield -1;
}
default -> startIndex + 1;
};
}

/**
Expand Down Expand Up @@ -1688,18 +1590,4 @@ private static ParserInstance getParser() {
ParserInstance.registerData(DefaultValueData.class, DefaultValueData::new);
}

/**
* @deprecated due to bad naming conventions,
* use {@link #LIST_SPLIT_PATTERN} instead.
*/
@Deprecated(since = "2.7.0", forRemoval = true)
public final static Pattern listSplitPattern = LIST_SPLIT_PATTERN;

/**
* @deprecated due to bad naming conventions,
* use {@link #WILDCARD} instead.
*/
@Deprecated(since = "2.8.0", forRemoval = true)
public final static String wildcard = WILDCARD;

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.UnknownNullability;
import org.skriptlang.skript.common.function.Parameter;
import org.skriptlang.skript.lang.script.Script;
import org.skriptlang.skript.util.Executable;
import org.skriptlang.skript.util.Validated;
Expand Down Expand Up @@ -44,7 +45,7 @@ public DynamicFunctionReference(Function<? extends Result> function) {
this.function = new WeakReference<>(function);
this.name = function.getName();
this.signature = function.getSignature();
@Nullable File file = ScriptLoader.getScriptFromName(signature.script);
@Nullable File file = ScriptLoader.getScriptFromName(signature.namespace());
this.source = file != null ? ScriptLoader.getScript(file) : null;
}

Expand All @@ -69,7 +70,7 @@ public DynamicFunctionReference(@NotNull String name, @Nullable Script source) {
this.function = new WeakReference<>(function);
if (resolved) {
this.signature = function.getSignature();
@Nullable File file = ScriptLoader.getScriptFromName(signature.script);
@Nullable File file = ScriptLoader.getScriptFromName(signature.namespace());
this.source = file != null ? ScriptLoader.getScript(file) : null;
} else {
this.signature = null;
Expand All @@ -90,17 +91,17 @@ public DynamicFunctionReference(@NotNull String name, @Nullable Script source) {
public boolean isSingle(Expression<?>... arguments) {
if (!resolved)
return true;
return signature.contract != null
? signature.contract.isSingle(arguments)
return signature.getContract() != null
? signature.getContract().isSingle(arguments)
: signature.isSingle();
}

@Override
public @Nullable Class<?> getReturnType(Expression<?>... arguments) {
if (!resolved)
return Object.class;
if (signature.contract != null)
return signature.contract.getReturnType(arguments);
if (signature.getContract() != null)
return signature.getContract().getReturnType(arguments);
Function<? extends Result> function = this.function.get();
if (function != null && function.getReturnType() != null)
return function.getReturnType().getC();
Expand Down Expand Up @@ -162,7 +163,7 @@ public String toString() {
this.checkedInputs.put(input, null); // failure case
if (signature == null)
return null;
boolean varArgs = signature.getMaxParameters() == 1 && !signature.getParameter(0).single;
boolean varArgs = signature.getMaxParameters() == 1 && !signature.parameters().entrySet().iterator().next().getValue().single();
Expression<?>[] parameters = input.parameters();
// Too many parameters
if (parameters.length > signature.getMaxParameters() && !varArgs)
Expand All @@ -174,12 +175,20 @@ else if (parameters.length < signature.getMinParameters())

// Check parameter types
for (int i = 0; i < parameters.length; i++) {
Parameter<?> parameter = signature.parameters[varArgs ? 0 : i];
Parameter<?> parameter = signature.parameters().values().toArray(new Parameter<?>[0])[varArgs ? 0 : i];

Class<?> target;
if (parameter.type().isArray()) {
target = parameter.type().componentType();
} else {
target = parameter.type();
}

//noinspection unchecked
Expression<?> expression = parameters[i].getConvertedExpression(parameter.type.getC());
Expression<?> expression = parameters[i].getConvertedExpression(target);
if (expression == null) {
return null;
} else if (parameter.single && !expression.isSingle()) {
} else if (parameter.single() && !expression.isSingle()) {
return null;
}
checked[i] = expression;
Expand Down
Loading