Skip to content

Commit

Permalink
Fix some issues with ExprParse (#5878)
Browse files Browse the repository at this point in the history 
* Fix bugs in ExprParse and add a `PARSE` ParseContext

* Make ExprParse use `ParseContext.PARSE` rather than `ParseContext.SCRIPT`

* Add tests and fix isSingle

* Improve #getReturnType

* Change every 'int' -> 'integer'

---------

Co-authored-by: Ayham Al Ali <[email protected]>
Co-authored-by: Moderocky <[email protected]>
  • Loading branch information
@UnderscoreTud @AyhamAl-Ali @Moderocky
3 people authored Oct 20, 2023
1 parent 65059e1 commit db1dad7
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 59 deletions.
12 changes: 6 additions & 6 deletions src/main/java/ch/njol/skript/classes/data/BukkitClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public Entity parse(final String s, final ParseContext context) {

@Override
public boolean canParse(final ParseContext context) {
return context == ParseContext.COMMAND;
return context == ParseContext.COMMAND || context == ParseContext.PARSE;
}

@Override
Expand Down Expand Up @@ -536,7 +536,7 @@ protected boolean canBeInstantiated() {
@Nullable
public World parse(final String s, final ParseContext context) {
// REMIND allow shortcuts '[over]world', 'nether' and '[the_]end' (server.properties: 'level-name=world') // inconsistent with 'world is "..."'
if (context == ParseContext.COMMAND || context == ParseContext.CONFIG)
if (context == ParseContext.COMMAND || context == ParseContext.PARSE || context == ParseContext.CONFIG)
return Bukkit.getWorld(s);
final Matcher m = parsePattern.matcher(s);
if (m.matches())
Expand Down Expand Up @@ -673,7 +673,7 @@ public String toVariableNameString(final Inventory i) {
@Override
@Nullable
public Player parse(String s, ParseContext context) {
if (context == ParseContext.COMMAND) {
if (context == ParseContext.COMMAND || context == ParseContext.PARSE) {
if (s.isEmpty())
return null;
if (UUID_PATTERN.matcher(s).matches())
Expand All @@ -693,7 +693,7 @@ public Player parse(String s, ParseContext context) {

@Override
public boolean canParse(final ParseContext context) {
return context == ParseContext.COMMAND;
return context == ParseContext.COMMAND || context == ParseContext.PARSE;
}

@Override
Expand Down Expand Up @@ -733,7 +733,7 @@ public String getDebugMessage(final Player p) {
@Nullable
@SuppressWarnings("deprecation")
public OfflinePlayer parse(final String s, final ParseContext context) {
if (context == ParseContext.COMMAND) {
if (context == ParseContext.COMMAND || context == ParseContext.PARSE) {
if (UUID_PATTERN.matcher(s).matches())
return Bukkit.getOfflinePlayer(UUID.fromString(s));
else if (!SkriptConfig.playerNameRegexPattern.value().matcher(s).matches())
Expand All @@ -746,7 +746,7 @@ else if (!SkriptConfig.playerNameRegexPattern.value().matcher(s).matches())

@Override
public boolean canParse(ParseContext context) {
return context == ParseContext.COMMAND;
return context == ParseContext.COMMAND || context == ParseContext.PARSE;
}

@Override
Expand Down
1 change: 1 addition & 0 deletions src/main/java/ch/njol/skript/classes/data/JavaClasses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ public String parse(String s, ParseContext context) {
return Utils.replaceChatStyles("" + s.substring(1, s.length() - 1).replace("\"\"", "\""));
return null;
case COMMAND:
case PARSE:
return s;
}
assert false;
Expand Down
24 changes: 18 additions & 6 deletions src/main/java/ch/njol/skript/effects/EffChange.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@
import java.util.Arrays;
import java.util.logging.Level;

import org.skriptlang.skript.lang.script.Script;
import ch.njol.skript.expressions.ExprParse;
import ch.njol.skript.lang.Effect;
import ch.njol.skript.lang.Expression;
import ch.njol.skript.lang.ExpressionList;
import ch.njol.skript.lang.SkriptParser;
import ch.njol.skript.lang.Variable;
import org.skriptlang.skript.lang.script.ScriptWarning;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -35,11 +40,7 @@
import ch.njol.skript.doc.Examples;
import ch.njol.skript.doc.Name;
import ch.njol.skript.doc.Since;
import ch.njol.skript.lang.Effect;
import ch.njol.skript.lang.Expression;
import ch.njol.skript.lang.SkriptParser;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.skript.lang.Variable;
import ch.njol.skript.log.CountingLogHandler;
import ch.njol.skript.log.ErrorQuality;
import ch.njol.skript.log.ParseLogHandler;
Expand Down Expand Up @@ -254,7 +255,18 @@ else if (mode == ChangeMode.SET)
Skript.error("only one " + Classes.getSuperClassInfo(x).getName() + " can be " + (mode == ChangeMode.ADD ? "added to" : "removed from") + " " + changed + ", not more", ErrorQuality.SEMANTIC_ERROR);
return false;
}


if (changed instanceof Variable && !changed.isSingle() && mode == ChangeMode.SET) {
if (ch instanceof ExprParse) {
((ExprParse) ch).flatten = false;
} else if (ch instanceof ExpressionList) {
for (Expression<?> expression : ((ExpressionList<?>) ch).getExpressions()) {
if (expression instanceof ExprParse)
((ExprParse) expression).flatten = false;
}
}
}

if (changed instanceof Variable && !((Variable<?>) changed).isLocal() && (mode == ChangeMode.SET || ((Variable<?>) changed).isList() && mode == ChangeMode.ADD)) {
final ClassInfo<?> ci = Classes.getSuperClassInfo(ch.getReturnType());
if (ci.getC() != Object.class && ci.getSerializer() == null && ci.getSerializeAs() == null && !SkriptConfig.disableObjectCannotBeSavedWarnings.value()) {
Expand Down
76 changes: 55 additions & 21 deletions src/main/java/ch/njol/skript/expressions/ExprParse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@
import org.eclipse.jdt.annotation.Nullable;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

@Name("Parse")
@Description({"Parses text as a given type, or as a given pattern.",
Expand Down Expand Up @@ -88,8 +91,9 @@ public class ExprParse extends SimpleExpression<Object> {
@Nullable
private SkriptPattern pattern;
@Nullable
private boolean[] patternExpressionPlurals;
private boolean patternHasSingleExpression = false;
private NonNullPair<ClassInfo<?>, Boolean>[] patternExpressions;
private boolean single = true;
public boolean flatten = true;

@Nullable
private ClassInfo<?> classInfo;
Expand All @@ -106,15 +110,23 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
return false;
}

NonNullPair<String, boolean[]> p = SkriptParser.validatePattern(pattern);
NonNullPair<String, NonNullPair<ClassInfo<?>, Boolean>[]> p = SkriptParser.validatePattern(pattern);
if (p == null) {
// Errored in validatePattern already
return false;
}

// Make all types in the pattern plural
pattern = p.getFirst();
patternExpressionPlurals = p.getSecond();
patternExpressions = p.getSecond();

// Check if all the types can actually parse
for (NonNullPair<ClassInfo<?>, Boolean> patternExpression : patternExpressions) {
if (!canParse(patternExpression.getFirst()))
return false;
if (patternExpression.getSecond())
single = false;
}

// Escape '¦' and ':' (used for parser tags/marks)
pattern = escapeParseTags(pattern);
Expand All @@ -128,8 +140,9 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
return false;
}

// If the pattern contains at most 1 type, this expression is single
this.patternHasSingleExpression = this.pattern.countNonNullTypes() <= 1;
// If the pattern contains at most 1 type, and this type is single, this expression is single
if (single)
this.single = this.pattern.countNonNullTypes() <= 1;
} else {
classInfo = ((Literal<ClassInfo<?>>) exprs[1]).getSingle();

Expand All @@ -139,11 +152,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
}

// Make sure the ClassInfo has a parser
Parser<?> parser = classInfo.getParser();
if (parser == null || !parser.canParse(ParseContext.COMMAND)) { // TODO special parse context?
Skript.error("Text cannot be parsed as " + classInfo.getName().withIndefiniteArticle());
return false;
}
return canParse(classInfo);
}
return true;
}
Expand All @@ -165,21 +174,36 @@ protected Object[] get(Event event) {
assert parser != null; // checked in init()

// Parse and return value
Object value = parser.parse(text, ParseContext.COMMAND);
Object value = parser.parse(text, ParseContext.PARSE);
if (value != null) {
Object[] valueArray = (Object[]) Array.newInstance(classInfo.getC(), 1);
valueArray[0] = value;
return valueArray;
}
} else {
assert pattern != null && patternExpressionPlurals != null;
assert pattern != null && patternExpressions != null;

MatchResult matchResult = pattern.match(text, SkriptParser.PARSE_LITERALS, ParseContext.COMMAND);
MatchResult matchResult = pattern.match(text, SkriptParser.PARSE_LITERALS, ParseContext.PARSE);

if (matchResult != null) {
Expression<?>[] exprs = matchResult.getExpressions();

assert patternExpressionPlurals.length == exprs.length;
assert patternExpressions.length == exprs.length;

if (flatten) {
List<Object> values = new ArrayList<>();
for (int i = 0; i < exprs.length; i++) {
if (exprs[i] != null) {
if (patternExpressions[i].getSecond()) {
values.addAll(Arrays.asList(exprs[i].getArray(null)));
continue;
}
values.add(exprs[i].getSingle(null));
}
}
return values.toArray();
}

int nonNullExprCount = 0;
for (Expression<?> expr : exprs) {
if (expr != null) // Ignore missing optional parts
Expand All @@ -192,8 +216,7 @@ protected Object[] get(Event event) {
for (int i = 0; i < exprs.length; i++) {
if (exprs[i] != null) {
//noinspection DataFlowIssue
values[valueIndex] = patternExpressionPlurals[i] ? exprs[i].getArray(null) : exprs[i].getSingle(null);

values[valueIndex] = patternExpressions[i].getSecond() ? exprs[i].getArray(null) : exprs[i].getSingle(null);
valueIndex++;
}
}
Expand Down Expand Up @@ -221,17 +244,28 @@ protected Object[] get(Event event) {

@Override
public boolean isSingle() {
return pattern == null || patternHasSingleExpression;
return single;
}

@Override
public Class<?> getReturnType() {
return classInfo != null ? classInfo.getC() : Object.class;
if (classInfo != null)
return classInfo.getC();
return patternExpressions.length == 1 ? patternExpressions[0].getFirst().getC() : Object.class;
}

@Override
public String toString(@Nullable Event e, boolean debug) {
return text.toString(e, debug) + " parsed as " + (classInfo != null ? classInfo.toString(Language.F_INDEFINITE_ARTICLE) : pattern);
public String toString(@Nullable Event event, boolean debug) {
return text.toString(event, debug) + " parsed as " + (classInfo != null ? classInfo.toString(Language.F_INDEFINITE_ARTICLE) : pattern);
}

private static boolean canParse(ClassInfo<?> classInfo) {
Parser<?> parser = classInfo.getParser();
if (parser == null || !parser.canParse(ParseContext.PARSE)) {
Skript.error("Text cannot be parsed as " + classInfo.getName().withIndefiniteArticle());
return false;
}
return true;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/main/java/ch/njol/skript/hooks/regions/classes/Region.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public Region parse(String s, final ParseContext context) {
quoted = true;
break;
case COMMAND:
case PARSE:
case CONFIG:
quoted = false;
break;
Expand Down
4 changes: 4 additions & 0 deletions src/main/java/ch/njol/skript/lang/ParseContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ public enum ParseContext {
* Only used for parsing arguments of commands
*/
COMMAND,
/**
* Used for parsing text in {@link ch.njol.skript.expressions.ExprParse}
*/
PARSE,
/**
* Used for parsing values from a config
*/
Expand Down
Loading

0 comments on commit db1dad7

Please sign in to comment.