From 6ec4d034362e09f7763461068709b82282a2ccaa Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 04:50:24 -0700 Subject: [PATCH 01/13] Re-add type hints --- .../java/ch/njol/skript/ScriptLoader.java | 20 ++++++--------- .../ch/njol/skript/effects/EffChange.java | 25 +++++++++++++------ .../java/ch/njol/skript/lang/Variable.java | 4 +-- src/test/skript/tests/misc/type-hints.sk | 11 ++++++++ 4 files changed, 39 insertions(+), 21 deletions(-) create mode 100644 src/test/skript/tests/misc/type-hints.sk diff --git a/src/main/java/ch/njol/skript/ScriptLoader.java b/src/main/java/ch/njol/skript/ScriptLoader.java index 7c48fd5a104..fd2ec4be382 100644 --- a/src/main/java/ch/njol/skript/ScriptLoader.java +++ b/src/main/java/ch/njol/skript/ScriptLoader.java @@ -900,12 +900,12 @@ public static String replaceOptions(String string) { */ public static ArrayList loadItems(SectionNode node) { ParserInstance parser = getParser(); - if (Skript.debug()) parser.setIndentation(parser.getIndentation() + " "); - + ArrayList items = new ArrayList<>(); + TypeHints.enterScope(); for (Node subNode : node) { parser.setNode(subNode); @@ -926,8 +926,8 @@ public static ArrayList loadItems(SectionNode node) { long timeTaken = System.currentTimeMillis() - start; if (timeTaken > requiredTime) Skript.warning( - "The current line took a long time to parse (" + new Timespan(timeTaken) + ")." - + " Avoid using long lines and use parentheses to create clearer instructions." + "The current line took a long time to parse (" + new Timespan(timeTaken) + "). " + + "Avoid using long lines and use parentheses to create clearer instructions." ); } @@ -936,8 +936,6 @@ public static ArrayList loadItems(SectionNode node) { items.add(stmt); } else if (subNode instanceof SectionNode) { - TypeHints.enterScope(); // Begin conditional type hints - Section section = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items); if (section == null) continue; @@ -946,20 +944,18 @@ public static ArrayList loadItems(SectionNode node) { Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + section.toString(null, true))); items.add(section); - - // Destroy these conditional type hints - TypeHints.exitScope(); } } - + for (int i = 0; i < items.size() - 1; i++) items.get(i).setNext(items.get(i + 1)); parser.setNode(node); - + if (Skript.debug()) parser.setIndentation(parser.getIndentation().substring(0, parser.getIndentation().length() - 4)); - + + TypeHints.exitScope(); return items; } diff --git a/src/main/java/ch/njol/skript/effects/EffChange.java b/src/main/java/ch/njol/skript/effects/EffChange.java index 80ddd172646..6b03a641a9a 100644 --- a/src/main/java/ch/njol/skript/effects/EffChange.java +++ b/src/main/java/ch/njol/skript/effects/EffChange.java @@ -40,6 +40,7 @@ import ch.njol.skript.lang.SkriptParser; import ch.njol.skript.lang.SkriptParser.ParseResult; import ch.njol.skript.lang.Variable; +import ch.njol.skript.lang.VariableString; import ch.njol.skript.log.CountingLogHandler; import ch.njol.skript.log.ErrorQuality; import ch.njol.skript.log.ParseLogHandler; @@ -47,6 +48,7 @@ import ch.njol.skript.registrations.Classes; import ch.njol.skript.util.Patterns; import ch.njol.skript.util.Utils; +import ch.njol.skript.variables.TypeHints; import ch.njol.util.Kleenean; /** @@ -254,19 +256,28 @@ 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 && !((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()) { - if (getParser().isActive() && !getParser().getCurrentScript().suppressesWarning(ScriptWarning.VARIABLE_SAVE)) { - Skript.warning(ci.getName().withIndefiniteArticle() + " cannot be saved, i.e. the contents of the variable " + changed + " will be lost when the server stops."); + + if (changed instanceof Variable) { + Variable variable = (Variable) changed; + VariableString name = variable.getName(); + if (mode == ChangeMode.SET || (variable.isList() && mode == ChangeMode.ADD)) { + if (variable.isLocal()) { + if (name.isSimple()) // Emit a type hint if possible + TypeHints.add(name.toString(), ch.getReturnType()); + } else { + ClassInfo ci = Classes.getSuperClassInfo(ch.getReturnType()); + if (ci.getC() != Object.class && ci.getSerializer() == null && ci.getSerializeAs() == null && !SkriptConfig.disableObjectCannotBeSavedWarnings.value()) { + if (getParser().isActive() && !getParser().getCurrentScript().suppressesWarning(ScriptWarning.VARIABLE_SAVE)) { + Skript.warning(ci.getName().withIndefiniteArticle() + " cannot be saved, i.e. the contents of the variable " + changed + " will be lost when the server stops."); + } + } } } } } return true; } - + @Override protected void execute(Event e) { Object[] delta = changer == null ? null : changer.getArray(e); diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index ed40f8b15ea..89f12fac26f 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -239,8 +239,8 @@ public static Variable newInstance(String name, Class[] type for (int i = 0; i < types.length; i++) { infos[i] = Classes.getExactClassInfo(types[i]); } - Skript.warning("Variable '{_" + name + "}' is " + Classes.toString(Classes.getExactClassInfo(hint)) - + ", not " + Classes.toString(infos, false)); + Skript.warning("Variable '{" + name + "}' is of type '" + Classes.toString(Classes.getExactClassInfo(hint)) + + "', not a '" + Classes.toString(infos, false) + "'"); // Fall back to not having any type hints } } diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk new file mode 100644 index 00000000000..e0876bb19a2 --- /dev/null +++ b/src/test/skript/tests/misc/type-hints.sk @@ -0,0 +1,11 @@ +test "type hints": + set {_string} to "empty" + set helmet of last spawned zombie to 1 of {_string} + # {_string} still a string + add stone to {_string} # Fails and does nothing + # {_string} still a string + set helmet of last spawned zombie to 1 of {_string} + # {_string} still a string + + set {_timespan} to a second + set helmet of last spawned zombie to 1 of {_timespan} From f672c9859fdfb1d9141a3f4b64483d5e4b0c910c Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 05:14:24 -0700 Subject: [PATCH 02/13] Add suppression warning --- .../ch/njol/skript/effects/EffSuppressWarnings.java | 7 +++++-- src/main/java/ch/njol/skript/lang/Variable.java | 6 ++++-- .../skriptlang/skript/lang/script/ScriptWarning.java | 4 +++- src/test/skript/tests/misc/type-hints.sk | 10 ++++++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java b/src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java index 9d422f36137..bb7e99c8b1b 100644 --- a/src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java +++ b/src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java @@ -42,11 +42,11 @@ public class EffSuppressWarnings extends Effect { static { Skript.registerEffect(EffSuppressWarnings.class, - "[local[ly]] suppress [the] (1:conflict|2:variable save|3:[missing] conjunction[s]|4:starting [with] expression[s]) warning[s]" + "[local[ly]] suppress [the] (1:conflict|2:variable save|3:[missing] conjunction[s]|4:starting [with] expression[s]|5:local variable type[s]) warning[s]" ); } - private static final int CONFLICT = 1, INSTANCE = 2, CONJUNCTION = 3, START_EXPR = 4; + private static final int CONFLICT = 1, INSTANCE = 2, CONJUNCTION = 3, START_EXPR = 4, LOCAL_TYPES = 5; private int mark = 0; @Override @@ -84,6 +84,9 @@ public String toString(@Nullable Event event, boolean debug) { case START_EXPR: word = "starting expression"; break; + case LOCAL_TYPES: + word = "local variable types"; + break; default: throw new IllegalStateException(); } diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 89f12fac26f..0b1f37ca4b3 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -239,8 +239,10 @@ public static Variable newInstance(String name, Class[] type for (int i = 0; i < types.length; i++) { infos[i] = Classes.getExactClassInfo(types[i]); } - Skript.warning("Variable '{" + name + "}' is of type '" + Classes.toString(Classes.getExactClassInfo(hint)) + - "', not a '" + Classes.toString(infos, false) + "'"); + if (parser.isActive() && !parser.getCurrentScript().suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { + Skript.warning("Variable '{" + name + "}' is of type '" + Classes.toString(Classes.getExactClassInfo(hint)) + + "', not a '" + Classes.toString(infos, false) + "'"); + } // Fall back to not having any type hints } } diff --git a/src/main/java/org/skriptlang/skript/lang/script/ScriptWarning.java b/src/main/java/org/skriptlang/skript/lang/script/ScriptWarning.java index f56971b7940..278abd09495 100644 --- a/src/main/java/org/skriptlang/skript/lang/script/ScriptWarning.java +++ b/src/main/java/org/skriptlang/skript/lang/script/ScriptWarning.java @@ -27,6 +27,8 @@ public enum ScriptWarning { MISSING_CONJUNCTION, // Missing "and" or "or" - VARIABLE_STARTS_WITH_EXPRESSION // Variable starts with an Expression + VARIABLE_STARTS_WITH_EXPRESSION, // Variable starts with an Expression + + LOCAL_VARIABLE_TYPE // Variable '{_example}' is of type 'string', not a 'player, entity type or entity data' } diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk index e0876bb19a2..8ef757e5fe8 100644 --- a/src/test/skript/tests/misc/type-hints.sk +++ b/src/test/skript/tests/misc/type-hints.sk @@ -9,3 +9,13 @@ test "type hints": set {_timespan} to a second set helmet of last spawned zombie to 1 of {_timespan} + +# TODO LimeGlass - Move to regression test #3753 when adding warning test runner checking. +command /test: + trigger: + set {_test} to a chicken + if {_test} is an entity: + message "%{_test}% is an entity" + else: + message "%{_test}% is not an entity" + spawn {_test} at player's location From c2294ee4c1d4e128492ae41d34a8eb97c89cf05b Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 06:57:30 -0700 Subject: [PATCH 03/13] Fix function parameters with local variables --- .../java/ch/njol/skript/lang/Variable.java | 3 +-- .../lang/function/FunctionReference.java | 18 ++++++++++++++++++ src/test/skript/tests/misc/type-hints.sk | 12 ++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 0b1f37ca4b3..9c2b799f099 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -240,8 +240,7 @@ public static Variable newInstance(String name, Class[] type infos[i] = Classes.getExactClassInfo(types[i]); } if (parser.isActive() && !parser.getCurrentScript().suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { - Skript.warning("Variable '{" + name + "}' is of type '" + Classes.toString(Classes.getExactClassInfo(hint)) + - "', not a '" + Classes.toString(infos, false) + "'"); + Skript.warning("Variable '{" + name + "}' is of type " + Classes.toString(Classes.getExactClassInfo(hint)) + ", and is " + SkriptParser.notOfType(infos)); } // Fall back to not having any type hints } diff --git a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java index a12bd8f735f..d30785f9c49 100644 --- a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java +++ b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java @@ -25,11 +25,16 @@ import ch.njol.skript.config.Node; import ch.njol.skript.lang.Expression; import ch.njol.skript.lang.SkriptParser; +import ch.njol.skript.lang.Variable; +import ch.njol.skript.lang.parser.ParserInstance; import ch.njol.skript.log.RetainingLogHandler; import ch.njol.skript.log.SkriptLogger; import ch.njol.skript.registrations.Classes; import org.skriptlang.skript.lang.converter.Converters; +import org.skriptlang.skript.lang.script.ScriptWarning; + import ch.njol.skript.util.LiteralUtils; +import ch.njol.skript.variables.TypeHints; import ch.njol.util.StringUtils; import org.bukkit.event.Event; import org.eclipse.jdt.annotation.Nullable; @@ -212,6 +217,19 @@ public boolean validateFunction(boolean first) { Parameter p = sign.parameters[singleListParam ? 0 : i]; RetainingLogHandler log = SkriptLogger.startRetainingLog(); try { + // Check type hints + if (parameters[i] instanceof Variable) { + Variable variable = (Variable) parameters[i]; + Class hint = TypeHints.get(variable.getName().toString()); + if (hint != null && !p.type.getC().isAssignableFrom(hint)) { + ParserInstance parser = ParserInstance.get(); + if (parser.isActive() && !parser.getCurrentScript().suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { + // This section of code is expecting an error, we cannot change to a warning, otherwise 'cannot understand condition/effect' errors also print for this line. + Skript.error("Variable '" + variable.toString() + "' is of type " + Classes.toString(Classes.getExactClassInfo(hint)) + ", and is " + SkriptParser.notOfType(p.getType())); + return false; + } + } + } Expression e = parameters[i].getConvertedExpression(p.type.getC()); if (e == null) { if (first) { diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk index 8ef757e5fe8..32242192846 100644 --- a/src/test/skript/tests/misc/type-hints.sk +++ b/src/test/skript/tests/misc/type-hints.sk @@ -1,3 +1,7 @@ +# TODO Remove and properly catch warnings. +#on script load: + #suppress local variable type warnings + test "type hints": set {_string} to "empty" set helmet of last spawned zombie to 1 of {_string} @@ -19,3 +23,11 @@ command /test: else: message "%{_test}% is not an entity" spawn {_test} at player's location + +# TODO LimeGlass - Move to regression test #1229 when adding warning test runner checking. +function test(player: player): + broadcast "%{_player}% is {_player}" + +test "function type hints": + set {_function} to "a string" + assert test({_function}) to fail with "Testing function failed" From c903b7a8da558de87683628159f7d4d6dc2d9802 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 07:04:07 -0700 Subject: [PATCH 04/13] Clean test --- src/test/skript/tests/misc/type-hints.sk | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk index 32242192846..1551b913432 100644 --- a/src/test/skript/tests/misc/type-hints.sk +++ b/src/test/skript/tests/misc/type-hints.sk @@ -1,7 +1,3 @@ -# TODO Remove and properly catch warnings. -#on script load: - #suppress local variable type warnings - test "type hints": set {_string} to "empty" set helmet of last spawned zombie to 1 of {_string} From 1b544d9bcd4221a4614e9b08d0e949245c2dd823 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 07:12:42 -0700 Subject: [PATCH 05/13] Add API util method in TypeHints --- .../ch/njol/skript/variables/TypeHints.java | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/java/ch/njol/skript/variables/TypeHints.java b/src/main/java/ch/njol/skript/variables/TypeHints.java index 73c810264f4..34b47c30d3e 100644 --- a/src/main/java/ch/njol/skript/variables/TypeHints.java +++ b/src/main/java/ch/njol/skript/variables/TypeHints.java @@ -25,6 +25,9 @@ import org.eclipse.jdt.annotation.Nullable; +import ch.njol.skript.SkriptAPIException; +import ch.njol.skript.lang.Variable; + /** * This is used to manage local variable type hints. * @@ -36,13 +39,13 @@ * */ public class TypeHints { - + private static final Deque>> typeHints = new ArrayDeque<>(); - + static { clear(); // Initialize type hints } - + public static void add(String variable, Class hint) { if (hint.equals(Object.class)) // Ignore useless type hint return; @@ -51,7 +54,20 @@ public static void add(String variable, Class hint) { Map> hints = typeHints.getFirst(); hints.put(variable, hint); } - + + /** + * Return any known type hints of a local variable. + * + * @param variable The local variable expression to check against. + * @return The return type that the local variable has been set to otherwise null if unset. + */ + @Nullable + public static Class get(Variable variable) { + if (!variable.isLocal()) + throw new SkriptAPIException("Must only get TypeHints of local variables."); + return get(variable.getName().toString()); + } + @Nullable public static Class get(String variable) { // Go through stack of hints for different scopes @@ -63,17 +79,18 @@ public static Class get(String variable) { return null; // No type hint available } - + public static void enterScope() { typeHints.push(new HashMap<>()); } - + public static void exitScope() { typeHints.pop(); } - + public static void clear() { typeHints.clear(); typeHints.push(new HashMap<>()); } -} \ No newline at end of file + +} From 3876331f5ba0a64361685359af089d0d29458cd6 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 07:13:36 -0700 Subject: [PATCH 06/13] Clean test --- .../java/ch/njol/skript/lang/function/FunctionReference.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java index d30785f9c49..1244c8d0de8 100644 --- a/src/main/java/ch/njol/skript/lang/function/FunctionReference.java +++ b/src/main/java/ch/njol/skript/lang/function/FunctionReference.java @@ -220,7 +220,7 @@ public boolean validateFunction(boolean first) { // Check type hints if (parameters[i] instanceof Variable) { Variable variable = (Variable) parameters[i]; - Class hint = TypeHints.get(variable.getName().toString()); + Class hint = TypeHints.get(variable); if (hint != null && !p.type.getC().isAssignableFrom(hint)) { ParserInstance parser = ParserInstance.get(); if (parser.isActive() && !parser.getCurrentScript().suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { From 503c72d424e916fb01e95dc2593ec059020a267b Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 18:27:06 -0700 Subject: [PATCH 07/13] Apply changes --- .../java/ch/njol/skript/ScriptLoader.java | 99 ++++++++++--------- .../java/ch/njol/skript/lang/Variable.java | 2 +- .../ch/njol/skript/variables/TypeHints.java | 20 ++-- 3 files changed, 60 insertions(+), 61 deletions(-) diff --git a/src/main/java/ch/njol/skript/ScriptLoader.java b/src/main/java/ch/njol/skript/ScriptLoader.java index fd2ec4be382..4ec1da3c917 100644 --- a/src/main/java/ch/njol/skript/ScriptLoader.java +++ b/src/main/java/ch/njol/skript/ScriptLoader.java @@ -898,64 +898,65 @@ public static String replaceOptions(String string) { /** * Loads a section by converting it to {@link TriggerItem}s. */ - public static ArrayList loadItems(SectionNode node) { + public static List loadItems(SectionNode node) { ParserInstance parser = getParser(); if (Skript.debug()) parser.setIndentation(parser.getIndentation() + " "); ArrayList items = new ArrayList<>(); - - TypeHints.enterScope(); - for (Node subNode : node) { - parser.setNode(subNode); - - String subNodeKey = subNode.getKey(); - if (subNodeKey == null) - throw new IllegalArgumentException("Encountered node with null key: '" + subNode + "'"); - String expr = replaceOptions(subNodeKey); - if (!SkriptParser.validateLine(expr)) - continue; - - if (subNode instanceof SimpleNode) { - long start = System.currentTimeMillis(); - Statement stmt = Statement.parse(expr, "Can't understand this condition/effect: " + expr); - if (stmt == null) + try { + TypeHints.enterScope(); + for (Node subNode : node) { + parser.setNode(subNode); + + String subNodeKey = subNode.getKey(); + if (subNodeKey == null) + throw new IllegalArgumentException("Encountered node with null key: '" + subNode + "'"); + String expr = replaceOptions(subNodeKey); + if (!SkriptParser.validateLine(expr)) continue; - long requiredTime = SkriptConfig.longParseTimeWarningThreshold.value().getMilliSeconds(); - if (requiredTime > 0) { - long timeTaken = System.currentTimeMillis() - start; - if (timeTaken > requiredTime) - Skript.warning( - "The current line took a long time to parse (" + new Timespan(timeTaken) + "). " + - "Avoid using long lines and use parentheses to create clearer instructions." - ); + + if (subNode instanceof SimpleNode) { + long start = System.currentTimeMillis(); + Statement stmt = Statement.parse(expr, "Can't understand this condition/effect: " + expr); + if (stmt == null) + continue; + long requiredTime = SkriptConfig.longParseTimeWarningThreshold.value().getMilliSeconds(); + if (requiredTime > 0) { + long timeTaken = System.currentTimeMillis() - start; + if (timeTaken > requiredTime) + Skript.warning( + "The current line took a long time to parse (" + new Timespan(timeTaken) + "). " + + "Avoid using long lines and use parentheses to create clearer instructions." + ); + } + + if (Skript.debug() || subNode.debug()) + Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + stmt.toString(null, true))); + + items.add(stmt); + } else if (subNode instanceof SectionNode) { + Section section = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items); + if (section == null) + continue; + + if (Skript.debug() || subNode.debug()) + Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + section.toString(null, true))); + + items.add(section); } - - if (Skript.debug() || subNode.debug()) - Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + stmt.toString(null, true))); - - items.add(stmt); - } else if (subNode instanceof SectionNode) { - Section section = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items); - if (section == null) - continue; - - if (Skript.debug() || subNode.debug()) - Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + section.toString(null, true))); - - items.add(section); } + + for (int i = 0; i < items.size() - 1; i++) + items.get(i).setNext(items.get(i + 1)); + + parser.setNode(node); + + if (Skript.debug()) + parser.setIndentation(parser.getIndentation().substring(0, parser.getIndentation().length() - 4)); + } finally { + TypeHints.exitScope(); } - - for (int i = 0; i < items.size() - 1; i++) - items.get(i).setNext(items.get(i + 1)); - - parser.setNode(node); - - if (Skript.debug()) - parser.setIndentation(parser.getIndentation().substring(0, parser.getIndentation().length() - 4)); - - TypeHints.exitScope(); return items; } diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 9c2b799f099..6f79686e602 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -239,7 +239,7 @@ public static Variable newInstance(String name, Class[] type for (int i = 0; i < types.length; i++) { infos[i] = Classes.getExactClassInfo(types[i]); } - if (parser.isActive() && !parser.getCurrentScript().suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { + if (parser.isActive() && !currentScript.suppressesWarning(ScriptWarning.LOCAL_VARIABLE_TYPE)) { Skript.warning("Variable '{" + name + "}' is of type " + Classes.toString(Classes.getExactClassInfo(hint)) + ", and is " + SkriptParser.notOfType(infos)); } // Fall back to not having any type hints diff --git a/src/main/java/ch/njol/skript/variables/TypeHints.java b/src/main/java/ch/njol/skript/variables/TypeHints.java index 34b47c30d3e..e464dd02768 100644 --- a/src/main/java/ch/njol/skript/variables/TypeHints.java +++ b/src/main/java/ch/njol/skript/variables/TypeHints.java @@ -18,10 +18,10 @@ */ package ch.njol.skript.variables; -import java.util.ArrayDeque; -import java.util.Deque; import java.util.HashMap; import java.util.Map; +import java.util.Queue; +import java.util.concurrent.LinkedBlockingQueue; import org.eclipse.jdt.annotation.Nullable; @@ -40,7 +40,7 @@ */ public class TypeHints { - private static final Deque>> typeHints = new ArrayDeque<>(); + private static final Queue>> TYPE_HINTS = new LinkedBlockingQueue<>(); static { clear(); // Initialize type hints @@ -49,9 +49,7 @@ public class TypeHints { public static void add(String variable, Class hint) { if (hint.equals(Object.class)) // Ignore useless type hint return; - - // Take top of stack, without removing it - Map> hints = typeHints.getFirst(); + Map> hints = TYPE_HINTS.peek(); hints.put(variable, hint); } @@ -71,7 +69,7 @@ public static Class get(Variable variable) { @Nullable public static Class get(String variable) { // Go through stack of hints for different scopes - for (Map> hints : typeHints) { + for (Map> hints : TYPE_HINTS) { Class hint = hints.get(variable); if (hint != null) // Found in this scope return hint; @@ -81,16 +79,16 @@ public static Class get(String variable) { } public static void enterScope() { - typeHints.push(new HashMap<>()); + TYPE_HINTS.add(new HashMap<>()); } public static void exitScope() { - typeHints.pop(); + TYPE_HINTS.poll(); } public static void clear() { - typeHints.clear(); - typeHints.push(new HashMap<>()); + TYPE_HINTS.clear(); + TYPE_HINTS.add(new HashMap<>()); } } From ea47e2cc627e5e6dfbdbe3cf8d92d5a0ce81b163 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Fri, 17 Feb 2023 19:56:35 -0700 Subject: [PATCH 08/13] Apply update when changing --- .../java/ch/njol/skript/lang/Variable.java | 21 ++++++++++++------- .../ch/njol/skript/variables/TypeHints.java | 3 +++ .../ch/njol/skript/variables/Variables.java | 8 +++---- src/test/skript/tests/misc/type-hints.sk | 4 ++++ 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/Variable.java b/src/main/java/ch/njol/skript/lang/Variable.java index 6f79686e602..10583ed2c5b 100644 --- a/src/main/java/ch/njol/skript/lang/Variable.java +++ b/src/main/java/ch/njol/skript/lang/Variable.java @@ -245,7 +245,6 @@ public static Variable newInstance(String name, Class[] type // Fall back to not having any type hints } } - return new Variable<>(vs, types, isLocal, isPlural, null); } @@ -498,15 +497,21 @@ private T[] getConvertedArray(Event e) { return Converters.convert((Object[]) get(e), types, superType); } - private void set(Event e, @Nullable Object value) { - Variables.setVariable("" + name.toString(e), value, e, local); + private void set(String string, @Nullable Object value, Event event) { + if (local && value != null && name.isSimple()) + TypeHints.add(name.toString(), value.getClass()); + Variables.setVariable(string, value, event, local); + } + + private void set(Event event, @Nullable Object value) { + set(name.toString(event), value, event); } - private void setIndex(Event e, String index, @Nullable Object value) { + private void setIndex(Event event, String index, @Nullable Object value) { assert list; - String s = name.toString(e); - assert s.endsWith("::*") : s + "; " + name; - Variables.setVariable(s.substring(0, s.length() - 1) + index, value, e, local); + String string = name.toString(event); + assert string.endsWith("::*") : string + "; " + name; + set(string.substring(0, string.length() - 1) + index, value, event); } @Override @@ -516,8 +521,8 @@ public Class[] acceptChange(ChangeMode mode) { return CollectionUtils.array(Object[].class); } - @SuppressWarnings({"unchecked", "rawtypes"}) @Override + @SuppressWarnings({"unchecked", "rawtypes"}) public void change(Event e, @Nullable Object[] delta, ChangeMode mode) throws UnsupportedOperationException { switch (mode) { case DELETE: diff --git a/src/main/java/ch/njol/skript/variables/TypeHints.java b/src/main/java/ch/njol/skript/variables/TypeHints.java index e464dd02768..9e86cd2da12 100644 --- a/src/main/java/ch/njol/skript/variables/TypeHints.java +++ b/src/main/java/ch/njol/skript/variables/TypeHints.java @@ -50,6 +50,9 @@ public static void add(String variable, Class hint) { if (hint.equals(Object.class)) // Ignore useless type hint return; Map> hints = TYPE_HINTS.peek(); + Class existing = hints.get(variable); + if (existing != null && existing.equals(hint)) + return; hints.put(variable, hint); } diff --git a/src/main/java/ch/njol/skript/variables/Variables.java b/src/main/java/ch/njol/skript/variables/Variables.java index 6d9706d93f4..944b40b35ca 100644 --- a/src/main/java/ch/njol/skript/variables/Variables.java +++ b/src/main/java/ch/njol/skript/variables/Variables.java @@ -295,20 +295,18 @@ public static Object copyLocalVariables(Event event) { copy.treeMap.putAll(from.treeMap); return copy; } - + /** * Remember to lock with {@link #getReadLock()}! */ - @SuppressWarnings("null") static Map getVariablesHashMap() { return Collections.unmodifiableMap(variables.hashMap); } - - @SuppressWarnings("null") + static Lock getReadLock() { return variablesLock.readLock(); } - + /** * Returns the internal value of the requested variable. *

diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk index 1551b913432..755b29bed2c 100644 --- a/src/test/skript/tests/misc/type-hints.sk +++ b/src/test/skript/tests/misc/type-hints.sk @@ -10,6 +10,10 @@ test "type hints": set {_timespan} to a second set helmet of last spawned zombie to 1 of {_timespan} + spawn a cow at spawn of world "world": + set {_entity} to event-entity + set helmet of last spawned zombie to {_entity} + # TODO LimeGlass - Move to regression test #3753 when adding warning test runner checking. command /test: trigger: From 41418024c94aeef2616295358ab46b4e341cf4ef Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Tue, 17 Sep 2024 22:40:11 -0600 Subject: [PATCH 09/13] Fix broken tests --- .../skript/expressions/ExprItemAmount.java | 1 + src/test/skript/tests/misc/type-hints.sk | 45 ++++++------------- .../2384-invalid function parameter type.sk | 9 ++-- .../551-item amount on itemstacks.sk | 11 ++--- .../tests/regressions/5804-is-burning.sk | 12 +++-- .../regressions/6385-x component of block.sk | 4 +- 6 files changed, 33 insertions(+), 49 deletions(-) diff --git a/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java b/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java index 046d4ec41c9..bd210c29603 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java +++ b/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java @@ -43,6 +43,7 @@ public class ExprItemAmount extends SimplePropertyExpression { @Override public Long convert(final Object item) { + System.out.println("item: " + item); if (item instanceof ItemType) { return (long) ((ItemType) item).getAmount(); } else if (item instanceof Slot) { diff --git a/src/test/skript/tests/misc/type-hints.sk b/src/test/skript/tests/misc/type-hints.sk index 755b29bed2c..c82d05f0737 100644 --- a/src/test/skript/tests/misc/type-hints.sk +++ b/src/test/skript/tests/misc/type-hints.sk @@ -1,33 +1,14 @@ test "type hints": - set {_string} to "empty" - set helmet of last spawned zombie to 1 of {_string} - # {_string} still a string - add stone to {_string} # Fails and does nothing - # {_string} still a string - set helmet of last spawned zombie to 1 of {_string} - # {_string} still a string - - set {_timespan} to a second - set helmet of last spawned zombie to 1 of {_timespan} - - spawn a cow at spawn of world "world": - set {_entity} to event-entity - set helmet of last spawned zombie to {_entity} - -# TODO LimeGlass - Move to regression test #3753 when adding warning test runner checking. -command /test: - trigger: - set {_test} to a chicken - if {_test} is an entity: - message "%{_test}% is an entity" - else: - message "%{_test}% is not an entity" - spawn {_test} at player's location - -# TODO LimeGlass - Move to regression test #1229 when adding warning test runner checking. -function test(player: player): - broadcast "%{_player}% is {_player}" - -test "function type hints": - set {_function} to "a string" - assert test({_function}) to fail with "Testing function failed" + set {_entity} to a zombie + spawn {_entity} at event-location: + parse: + set {_string} to "empty" + set helmet of event-entity to 1 of {_string} + assert last parse logs is set with "Variable '{_string}' is of type text, and is neither an item stack, an item type nor an entity type" + + parse: + set {_timespan} to a second + set helmet of last spawned {_entity} to 1 of {_timespan} + assert last parse logs is set with "Variable '{_timespan}' is of type time span, and is neither an item stack, an item type nor an entity type" + + clear event-entity diff --git a/src/test/skript/tests/regressions/2384-invalid function parameter type.sk b/src/test/skript/tests/regressions/2384-invalid function parameter type.sk index 76f040dd889..9c064756a2f 100644 --- a/src/test/skript/tests/regressions/2384-invalid function parameter type.sk +++ b/src/test/skript/tests/regressions/2384-invalid function parameter type.sk @@ -1,4 +1,7 @@ +# Also issue #1229 + test "invalid function parameter type": - set {_list::*} to "hello" and "bye" - set {_test} to max({_list::*}) # This code should not throw an ArrayIndexOutOfBoundException - assert {_test} is not set with "invalid function parameter type failed" # Shouldn't return anything... + parse: + set {_list::*} to "hello" and "bye" + set {_test} to max({_list::*}) + assert last parse logs is set with "Variable '{_list::*}' is of type text, and is not a number" diff --git a/src/test/skript/tests/regressions/551-item amount on itemstacks.sk b/src/test/skript/tests/regressions/551-item amount on itemstacks.sk index 809f5497f94..d2dd2ac3a4f 100644 --- a/src/test/skript/tests/regressions/551-item amount on itemstacks.sk +++ b/src/test/skript/tests/regressions/551-item amount on itemstacks.sk @@ -4,9 +4,9 @@ test "item amount of itemstack": set block at {_loc} to chest add 10 stone to inventory of block at {_loc} set {_item} to slot 0 of inventory of block at {_loc} - assert item amount of {_item} is 10 with "Amount of itemstack is not 10: %{_amount}%" + assert item amount of {_item} is 10 with "Amount of itemstack is not 10" remove 1 from item amount of {_item} - assert item amount of {_item} is 9 with "Amount of itemstack is not 9: %{_amount}%" + assert item amount of {_item} is 9 with "Amount of itemstack is not 9" set block at {_loc} to {_old-block} test "item amount of itemtype": @@ -20,8 +20,9 @@ test "item amount of slot": set {_loc} to spawn of world "world" set {_old-block} to type of block at {_loc} set block at {_loc} to chest + clear inventory of block at {_loc} add 10 stone to inventory of block at {_loc} - assert item amount of (slot 0 of inventory of block at {_loc}) is 10 with "Amount of slot is not 10: %{_amount}%" + assert item amount of (slot 0 of inventory of block at {_loc}) is 10 with "Amount of slot is not 10" remove 1 from item amount of (slot 0 of inventory of block at {_loc}) - assert item amount of (slot 0 of inventory of block at {_loc}) is 9 with "Amount of slot is not 9: %{_amount}%" - set block at {_loc} to {_old-block} + assert item amount of (slot 0 of inventory of block at {_loc}) is 9 with "Amount of slot is not 9" + set block at {_loc} to {_old-block} \ No newline at end of file diff --git a/src/test/skript/tests/regressions/5804-is-burning.sk b/src/test/skript/tests/regressions/5804-is-burning.sk index d3e01711dc6..8ac4e132296 100644 --- a/src/test/skript/tests/regressions/5804-is-burning.sk +++ b/src/test/skript/tests/regressions/5804-is-burning.sk @@ -1,9 +1,7 @@ test "burning": - spawn a pig at event-location - set {_pig} to the last spawned pig assert "burning" parsed as damage cause is burning with "burning damage cause compare" - set burning time of {_pig} to 9000 ticks - wait a tick - assert entity within {_pig} is burning with "is burning failed" - assert {_pig} is burning with "is burning failed ##2" - clear entity within {_pig} + spawn a pig at event-location: + set burning time of event-entity to 9000 ticks + assert entity within event-entity is burning with "is burning failed" + assert event-entity is burning with "is burning failed ##2" + clear entity within event-entity diff --git a/src/test/skript/tests/regressions/6385-x component of block.sk b/src/test/skript/tests/regressions/6385-x component of block.sk index b3be4d85c52..b1e11d14db9 100644 --- a/src/test/skript/tests/regressions/6385-x component of block.sk +++ b/src/test/skript/tests/regressions/6385-x component of block.sk @@ -1,3 +1,3 @@ test "x component of block": - set {_a} to block at location(0, 0, 0) - add 1 to x of {_a} # should not throw exception + set {a} to block at location(0, 0, 0) + add 1 to x of {a} # should not throw exception \ No newline at end of file From f325b5a6baf6234fea438e6736dbc1ec14865bc1 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Tue, 17 Sep 2024 22:41:08 -0600 Subject: [PATCH 10/13] Remove debug --- src/main/java/ch/njol/skript/expressions/ExprItemAmount.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java b/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java index bd210c29603..046d4ec41c9 100644 --- a/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java +++ b/src/main/java/ch/njol/skript/expressions/ExprItemAmount.java @@ -43,7 +43,6 @@ public class ExprItemAmount extends SimplePropertyExpression { @Override public Long convert(final Object item) { - System.out.println("item: " + item); if (item instanceof ItemType) { return (long) ((ItemType) item).getAmount(); } else if (item instanceof Slot) { From 40c76e8427983709fa6eb5261c8bca486d5cffe5 Mon Sep 17 00:00:00 2001 From: TheLimeGlass Date: Thu, 2 Jan 2025 16:43:02 -0700 Subject: [PATCH 11/13] Git conflicts --- .../java/ch/njol/skript/ScriptLoader.java | 5 ---- .../skript/effects/EffSuppressWarnings.java | 27 +------------------ 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/src/main/java/ch/njol/skript/ScriptLoader.java b/src/main/java/ch/njol/skript/ScriptLoader.java index 295945a064d..bb0517611b8 100644 --- a/src/main/java/ch/njol/skript/ScriptLoader.java +++ b/src/main/java/ch/njol/skript/ScriptLoader.java @@ -206,12 +206,7 @@ public static Set