Skip to content

Limit literal specification regex pattern to improve matching behavior. #8068

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

Merged
merged 6 commits into from
Aug 1, 2025
Merged

Limit literal specification regex pattern to improve matching behavior. #8068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
573b6a8
improve literal spec. parsing in functions
sovdeeth Jul 22, 2025
1923ff4
simplify pattern
sovdeeth Jul 22, 2025
b26f67c
Update SkriptParser.java
sovdeeth Jul 22, 2025
5fcb727
Update SkriptParser.java
sovdeeth Jul 22, 2025
25ea0cc
Merge branch 'dev/patch' into patch/litspecfuncparse
sovdeeth Jul 22, 2025
55a4560
Merge branch 'dev/patch' into patch/litspecfuncparse
APickledWalrus Aug 1, 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
145 changes: 76 additions & 69 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 @@ -467,31 +467,12 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
log.printError();
return null;
}
if (types[0] == Object.class) {
// Do check if a literal with this name actually exists before returning an UnparsedLiteral
if (!allowUnparsedLiteral || Classes.parseSimple(expr, Object.class, context) == null) {
log.printError();
return null;
}
log.clear();
LogEntry logError = log.getError();
return (Literal<? extends T>) new UnparsedLiteral(expr, logError != null && (error == null || logError.quality > error.quality) ? logError : error);
}
for (Class<? extends T> type : types) {
log.clear();
assert type != null;
T parsedObject = Classes.parse(expr, type, context);
if (parsedObject != null) {
log.printLog();
return new SimpleLiteral<>(parsedObject, false);
}
}
log.printError();
return null;
return parseAsLiteral(allowUnparsedLiteral, log, error, types);
}
}

private static final Pattern LITERAL_SPECIFICATION_PATTERN = Pattern.compile("(?<literal>[^(]+) \\((?<classinfo>[^)]+)\\)");
private static final String INVALID_LSPEC_CHARS = "[^,():/\"'\\[\\]}{]";
private static final Pattern LITERAL_SPECIFICATION_PATTERN = Pattern.compile("(?<literal>" + INVALID_LSPEC_CHARS + "+) \\((?<classinfo>[\\w\\p{L} ]+)\\)");

private @Nullable Expression<?> parseSingleExpr(boolean allowUnparsedLiteral, @Nullable LogEntry error, ExprInfo exprInfo) {
if (expr.isEmpty()) // Empty expressions return nothing, obviously
Expand Down Expand Up @@ -675,56 +656,80 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
log.printError();
return null;
}
if (expr.endsWith(")") && expr.indexOf("(") != -1) {
Matcher classInfoMatcher = LITERAL_SPECIFICATION_PATTERN.matcher(expr);
if (classInfoMatcher.matches()) {
String literalString = classInfoMatcher.group("literal");
String unparsedClassInfo = Noun.stripDefiniteArticle(classInfoMatcher.group("classinfo"));
Expression<?> result = parseSpecifiedLiteral(literalString, unparsedClassInfo, types);
if (result != null) {
log.printLog();
return result;
}
return parseAsLiteral(allowUnparsedLiteral, log, error, nonNullTypes);
}
}

/**
* Helper method for {@link #parseSingleExpr(boolean, LogEntry, Class[])} and {@link #parseSingleExpr(boolean, LogEntry, ExprInfo)}.
* Attempts to parse {@link #expr} as a literal. Prints errors.
*
* @param allowUnparsedLiteral If {@code true}, will allow unparsed literals to be returned.
* @param log The current {@link ParseLogHandler} to use for logging.
* @param error A {@link LogEntry} containing a default error to be printed if failed to parse.
* @param types The valid types to parse the literal as.
* @return {@link Expression} of type {@code T} if successful, otherwise {@code null}.<br>
* @param <T> The type of the literal to parse.<br>
*/
@SafeVarargs
private <T> @Nullable Expression<? extends T> parseAsLiteral(
boolean allowUnparsedLiteral,
ParseLogHandler log,
@Nullable LogEntry error,
Class<? extends T>... types
) {
if (expr.endsWith(")") && expr.contains("(")) {
Matcher classInfoMatcher = LITERAL_SPECIFICATION_PATTERN.matcher(expr);
if (classInfoMatcher.matches()) {
String literalString = classInfoMatcher.group("literal");
String unparsedClassInfo = Noun.stripDefiniteArticle(classInfoMatcher.group("classinfo"));
Expression<? extends T> result = parseSpecifiedLiteral(literalString, unparsedClassInfo, types);
if (result != null) {
log.printLog();
return result;
}
}
if (exprInfo.classes.length == 1 && exprInfo.classes[0].getC() == Object.class) {
if (!allowUnparsedLiteral) {
log.printError();
return null;
}
return getUnparsedLiteral(log, error);
}
if (types.length == 1 && types[0] == Object.class) {
if (!allowUnparsedLiteral) {
log.printError();
return null;
}
boolean containsObjectClass = false;
for (ClassInfo<?> classInfo : exprInfo.classes) {
log.clear();
assert classInfo.getC() != null;
if (classInfo.getC().equals(Object.class)) {
// If 'Object.class' is an option, needs to be treated as previous behavior
// But we also want to be sure every other 'ClassInfo' is attempted to be parsed beforehand
containsObjectClass = true;
continue;
}
Object parsedObject = Classes.parse(expr, classInfo.getC(), context);
if (parsedObject != null) {
log.printLog();
return new SimpleLiteral<>(parsedObject, false, new UnparsedLiteral(expr));
}
//noinspection unchecked
return (Expression<? extends T>) getUnparsedLiteral(log, error);
}
boolean containsObjectClass = false;
for (Class<?> type : types) {
log.clear();
if (type == Object.class) {
// If 'Object.class' is an option, needs to be treated as previous behavior
// But we also want to be sure every other 'ClassInfo' is attempted to be parsed beforehand
containsObjectClass = true;
continue;
}
if (allowUnparsedLiteral && containsObjectClass)
return getUnparsedLiteral(log, error);
if (expr.startsWith("\"") && expr.endsWith("\"") && expr.length() > 1) {
for (ClassInfo<?> aClass : exprInfo.classes) {
if (!aClass.getC().isAssignableFrom(String.class))
continue;
VariableString string = VariableString.newInstance(expr.substring(1, expr.length() - 1));
if (string instanceof LiteralString)
return string;
break;
}
//noinspection unchecked
T parsedObject = (T) Classes.parse(expr, type, context);
if (parsedObject != null) {
log.printLog();
return new SimpleLiteral<>(parsedObject, false, new UnparsedLiteral(expr));
}
log.printError();
return null;
}
if (allowUnparsedLiteral && containsObjectClass)
//noinspection unchecked
return (Expression<? extends T>) getUnparsedLiteral(log, error);
Comment on lines +717 to +719
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, does this need to be after the string literal check? My concern is that something like "%object/string%" wouldn't work right (it would always return an unparsed literal).

Maybe the string check needs to go in loop above instead?

if (expr.startsWith("\"") && expr.endsWith("\"") && expr.length() > 1) {
for (Class<?> type : types) {
if (!type.isAssignableFrom(String.class))
continue;
VariableString string = VariableString.newInstance(expr.substring(1, expr.length() - 1));
if (string instanceof LiteralString)
//noinspection unchecked
return (Expression<? extends T>) string;
break;
}
}
log.printError();
return null;
}

/**
Expand Down Expand Up @@ -759,10 +764,11 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
* @param types An {@link Array} of the acceptable {@link Class}es
* @return {@link SimpleLiteral} or {@code null} if any checks fail
*/
private @Nullable Expression<?> parseSpecifiedLiteral(
@SafeVarargs
private <T> @Nullable Expression<? extends T> parseSpecifiedLiteral(
String literalString,
String unparsedClassInfo,
Class<?> ... types
Class<? extends T> ... types
) {
ClassInfo<?> classInfo = Classes.parse(unparsedClassInfo, ClassInfo.class, context);
if (classInfo == null) {
Expand All @@ -778,7 +784,8 @@ private static <T extends SyntaxElement> boolean checkExperimentalSyntax(T eleme
Skript.error(expr + " " + Language.get("is") + " " + notOfType(types));
return null;
}
Object parsedObject = classInfoParser.parse(literalString, context);
//noinspection unchecked
T parsedObject = (T) classInfoParser.parse(literalString, context);
if (parsedObject != null)
return new SimpleLiteral<>(parsedObject, false, new UnparsedLiteral(literalString));
return null;
Expand Down
7 changes: 7 additions & 0 deletions src/test/skript/tests/misc/literal specification.sk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ test "literal specification":
assert firework (the item type) is an item type with "Literal specification should work with definite articles"
assert firework (an entity type) is an entity type with "Literal specification should work with indefinite article"

local function test(entity: entitytype, item: itemtype):
assert {_entity} is an entity type with "Literal specification should work with entitytype"
assert {_item} is an item type with "Literal specification should work with itemtype"

test "literal specification in functions":
test(firework (entity type), firework (item type))

test "literal specification error":
parse:
set {_block} to oak log (block)
Expand Down