Skip to content

Re-add local variable type hints #5457

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 20 commits into
base: dev/feature
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
175 changes: 87 additions & 88 deletions src/main/java/ch/njol/skript/ScriptLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -947,108 +947,107 @@ public static String replaceOptions(String string) {
/**
* Loads a section by converting it to {@link TriggerItem}s.
*/
public static ArrayList<TriggerItem> loadItems(SectionNode node) {
public static List<TriggerItem> loadItems(SectionNode node) {
ParserInstance parser = getParser();

if (Skript.debug())
parser.setIndentation(parser.getIndentation() + " ");

ArrayList<TriggerItem> items = new ArrayList<>();
List<TriggerItem> items = new ArrayList<>();

boolean executionStops = false;
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;

TriggerItem item;
if (subNode instanceof SimpleNode) {
long start = System.currentTimeMillis();
item = Statement.parse(expr, items, "Can't understand this condition/effect: " + expr);
if (item == 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().getAs(Timespan.TimePeriod.MILLISECOND);
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() + item.toString(null, true)));

items.add(item);
} else if (subNode instanceof SectionNode) {
TypeHints.enterScope(); // Begin conditional type hints

RetainingLogHandler handler = SkriptLogger.startRetainingLog();
find_section:
try {
item = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items);
if (item != null)
break find_section;

// back up the failure log
RetainingLogHandler backup = handler.backup();
handler.clear();

item = Statement.parse(expr, "Can't understand this effect: " + expr, (SectionNode) subNode, items);

if (item != null)
break find_section;
Collection<LogEntry> errors = handler.getErrors();

// restore the failure log
if (errors.isEmpty()) {
handler.restore(backup);
} else { // We specifically want these two errors in preference to the section error!
String firstError = errors.iterator().next().getMessage();
if (!firstError.contains("is a valid statement but cannot function as a section (:)")
&& !firstError.contains("You cannot have two section-starters in the same line"))

TriggerItem item;
if (subNode instanceof SimpleNode) {
long start = System.currentTimeMillis();
item = Statement.parse(expr, items, "Can't understand this condition/effect: " + expr);
if (item == null)
continue;
long requiredTime = SkriptConfig.longParseTimeWarningThreshold.value().getAs(Timespan.TimePeriod.MILLISECOND);
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() + item.toString(null, true)));

items.add(item);
} else if (subNode instanceof SectionNode) {
RetainingLogHandler handler = SkriptLogger.startRetainingLog();
find_section:
try {
item = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items);
if (item != null)
break find_section;

// back up the failure log
RetainingLogHandler backup = handler.backup();
handler.clear();

item = Statement.parse(expr, "Can't understand this effect: " + expr, (SectionNode) subNode, items);

if (item != null)
break find_section;
Collection<LogEntry> errors = handler.getErrors();

// restore the failure log
if (errors.isEmpty()) {
handler.restore(backup);
} else { // We specifically want these two errors in preference to the section error!
String firstError = errors.iterator().next().getMessage();
if (!firstError.contains("is a valid statement but cannot function as a section (:)")
&& !firstError.contains("You cannot have two section-starters in the same line"))
handler.restore(backup);
}
continue;
} finally {
handler.printLog();
handler.close();
}

if (Skript.debug() || subNode.debug())
Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + item.toString(null, true)));

items.add(item);
} else {
continue;
} finally {
handler.printLog();
handler.close();
}

if (Skript.debug() || subNode.debug())
Skript.debug(SkriptColor.replaceColorChar(parser.getIndentation() + item.toString(null, true)));

items.add(item);

// Destroy these conditional type hints
TypeHints.exitScope();
} else {
continue;
}

if (executionStops
&& !SkriptConfig.disableUnreachableCodeWarnings.value()
&& parser.isActive()
&& !parser.getCurrentScript().suppressesWarning(ScriptWarning.UNREACHABLE_CODE)) {
Skript.warning("Unreachable code. The previous statement stops further execution.");

if (executionStops
&& !SkriptConfig.disableUnreachableCodeWarnings.value()
&& parser.isActive()
&& !parser.getCurrentScript().suppressesWarning(ScriptWarning.UNREACHABLE_CODE)) {
Skript.warning("Unreachable code. The previous statement stops further execution.");
}
executionStops = item.executionIntent() != null;
}
executionStops = item.executionIntent() != null;

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));

return items;
}

Expand Down
37 changes: 23 additions & 14 deletions src/main/java/ch/njol/skript/effects/EffChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
import ch.njol.skript.doc.Name;
import ch.njol.skript.doc.Since;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.skript.lang.VariableString;
import ch.njol.skript.log.CountingLogHandler;
import ch.njol.skript.log.ErrorQuality;
import ch.njol.skript.log.ParseLogHandler;
import ch.njol.skript.log.SkriptLogger;
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;

/**
Expand Down Expand Up @@ -234,22 +236,29 @@ else if (mode == ChangeMode.SET)
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) {
if (!changed.isSingle() && mode == ChangeMode.SET) {
if (ch instanceof ExprParse exprParse) {
exprParse.flatten = false;
} else if (ch instanceof ExpressionList expressionList) {
for (Expression<?> expression : expressionList.getExpressions()) {
if (expression instanceof ExprParse exprParse)
exprParse.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()) {
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.");
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.");
}
}
}
}
}
Expand Down
28 changes: 24 additions & 4 deletions src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import ch.njol.util.Kleenean;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.UnknownNullability;
import org.skriptlang.skript.lang.script.ScriptWarning;

@Name("Locally Suppress Warning")
Expand All @@ -34,7 +33,27 @@ public class EffSuppressWarnings extends Effect {
Skript.registerEffect(EffSuppressWarnings.class, "[local[ly]] suppress [the] (" + warnings + ") warning[s]");
}

private @UnknownNullability ScriptWarning warning;
private enum Pattern {
INSTANCE(ScriptWarning.VARIABLE_SAVE),
CONJUNCTION(ScriptWarning.MISSING_CONJUNCTION),
START_EXPR(ScriptWarning.VARIABLE_STARTS_WITH_EXPRESSION),
DEPRECATED(ScriptWarning.DEPRECATED_SYNTAX),
UNREACHABLE(ScriptWarning.UNREACHABLE_CODE),
LOCAL_TYPES(ScriptWarning.LOCAL_VARIABLE_TYPE);

private final ScriptWarning warning;

Pattern(ScriptWarning warning) {
this.warning = warning;
}

public ScriptWarning getWarning() {
return warning;
}

}

private Pattern pattern;

@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, SkriptParser.ParseResult parseResult) {
Expand All @@ -43,7 +62,8 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
return false;
}

warning = ScriptWarning.values()[parseResult.mark];
pattern = Pattern.values()[matchedPattern];
ScriptWarning warning = pattern.getWarning();
if (warning.isDeprecated()) {
Skript.warning(warning.getDeprecationMessage());
} else {
Expand All @@ -57,7 +77,7 @@ protected void execute(Event event) { }

@Override
public String toString(@Nullable Event event, boolean debug) {
return "suppress " + warning.getWarningName() + " warnings";
return "suppress " + pattern.getWarning().getWarningName() + " warnings";
}

}
15 changes: 11 additions & 4 deletions src/main/java/ch/njol/skript/lang/Variable.java
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,9 @@ else if (character == '%')
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));
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
}
}
Expand Down Expand Up @@ -449,15 +450,21 @@ private T[] getConvertedArray(Event event) {
return Converters.convert((Object[]) get(event), types, superType);
}

private void set(String string, @Nullable Object value, Event event, boolean local) {
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) {
Variables.setVariable("" + name.toString(event), value, event, local);
set("" + name.toString(event), value, event, local);
}

private void setIndex(Event event, String index, @Nullable Object value) {
assert list;
String name = this.name.toString(event);
assert name.endsWith(SEPARATOR + "*") : name + "; " + this.name;
Variables.setVariable(name.substring(0, name.length() - 1) + index, value, event, local);
set(name.substring(0, name.length() - 1) + index, value, event, local);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@
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 ch.njol.skript.util.Contract;
import ch.njol.skript.util.LiteralUtils;
import ch.njol.skript.variables.TypeHints;
import ch.njol.util.StringUtils;
import org.bukkit.event.Event;
import org.skriptlang.skript.util.Executable;
import org.jetbrains.annotations.Nullable;
import org.skriptlang.skript.lang.converter.Converters;
import org.skriptlang.skript.lang.script.ScriptWarning;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -208,6 +212,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);
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;
}
}
}
//noinspection unchecked
Expression<?> e = parameters[i].getConvertedExpression(p.type.getC());
if (e == null) {
Expand Down
Loading
Loading