Skip to content
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

Unreachable Code #6960

Merged
merged 25 commits into from
Oct 13, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
816e29e
Add a warning for unreachable code
UnderscoreTud Aug 4, 2024
ff6c45b
Clean up EffSuppressWarnings and add the ability to disable the unrea…
UnderscoreTud Aug 7, 2024
a091a6c
Add comments
UnderscoreTud Aug 11, 2024
21bca9b
Requested Changes
UnderscoreTud Aug 14, 2024
e38462f
Merge branch 'dev/feature' into feature/unreachable-code-warning
sovdeeth Aug 14, 2024
849cbd7
Generify compareTo method
UnderscoreTud Aug 15, 2024
1562983
Add docs to ExecutionIntent
UnderscoreTud Aug 31, 2024
18591ad
Fix lines using spaces instead of tabs for indentation (unsure how th…
UnderscoreTud Aug 31, 2024
fc14b86
Add ExecutionIntent to EffContinue, and fix its toString
UnderscoreTud Sep 3, 2024
065c5c8
Improve SecLoop's intent detection
UnderscoreTud Sep 3, 2024
c1bf355
Merge branch 'dev/feature' into feature/unreachable-code-warning
UnderscoreTud Sep 3, 2024
8a51732
Use instanceof pattern matching
UnderscoreTud Sep 3, 2024
cd80a39
Use MoreObject.toStringHelper
UnderscoreTud Sep 4, 2024
0ab6a00
Replace all space indentations with tabs
UnderscoreTud Sep 4, 2024
ce79b11
Add simple helper methods for getting relative sections
UnderscoreTud Sep 4, 2024
501253d
Add comments to SecLoop#guaranteedToLoop
UnderscoreTud Sep 4, 2024
3afdbb6
Capitalize the 'C' in EffExit's errors
UnderscoreTud Sep 4, 2024
b6e7f01
Return a copy of the sections list rather than a view in the section …
UnderscoreTud Sep 5, 2024
245d1e5
Make EffContinue's and EffExit's patterns stricter
UnderscoreTud Sep 5, 2024
666f94c
Fix ArrayOutOfBoundsException when using Section.getSections of a typ…
UnderscoreTud Sep 5, 2024
79fcc25
Update javadocs
UnderscoreTud Sep 10, 2024
e6e6000
Merge branch 'dev/feature' into feature/unreachable-code-warning
UnderscoreTud Sep 17, 2024
3fea368
Requested Changes
UnderscoreTud Sep 17, 2024
79074af
Update src/main/java/ch/njol/skript/sections/SecConditional.java
UnderscoreTud Sep 22, 2024
fd3d93d
Merge branch 'dev/feature' into feature/unreachable-code-warning
sovdeeth Oct 13, 2024
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
37 changes: 25 additions & 12 deletions src/main/java/ch/njol/skript/ScriptLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;
import org.skriptlang.skript.lang.script.Script;
import org.skriptlang.skript.lang.script.ScriptWarning;
import org.skriptlang.skript.lang.structure.Structure;

import java.io.File;
Expand Down Expand Up @@ -949,9 +950,10 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {

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

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

boolean executionStops = false;
for (Node subNode : node) {
parser.setNode(subNode);

Expand All @@ -962,10 +964,11 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
if (!SkriptParser.validateLine(expr))
continue;

TriggerItem item;
if (subNode instanceof SimpleNode) {
long start = System.currentTimeMillis();
Statement stmt = Statement.parse(expr, items, "Can't understand this condition/effect: " + expr);
if (stmt == null)
item = Statement.parse(expr, items, "Can't understand this condition/effect: " + expr);
if (item == null)
continue;
long requiredTime = SkriptConfig.longParseTimeWarningThreshold.value().getMilliSeconds();
if (requiredTime > 0) {
Expand All @@ -978,34 +981,44 @@ public static ArrayList<TriggerItem> loadItems(SectionNode node) {
}

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

items.add(stmt);
items.add(item);
} 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)
item = Section.parse(expr, "Can't understand this section: " + expr, (SectionNode) subNode, items);
if (item == null)
continue;

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

items.add(section);
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.");
}
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));

return items;
}

Expand Down
1 change: 1 addition & 0 deletions src/main/java/ch/njol/skript/SkriptConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public static String formatDate(final long timestamp) {
public static final Option<Boolean> disableMissingAndOrWarnings = new Option<>("disable variable missing and/or warnings", false);
public static final Option<Boolean> disableVariableStartingWithExpressionWarnings =
new Option<>("disable starting a variable's name with an expression warnings", false);
public static final Option<Boolean> disableUnreachableCodeWarnings = new Option<>("disable unreachable code warnings", false);

@Deprecated
public static final Option<Boolean> enableScriptCaching = new Option<>("enable script caching", false)
Expand Down
30 changes: 15 additions & 15 deletions src/main/java/ch/njol/skript/effects/EffExit.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,8 @@
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.SectionExitHandler;
import ch.njol.skript.lang.LoopSection;
import ch.njol.skript.lang.*;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.skript.lang.TriggerItem;
import ch.njol.skript.lang.TriggerSection;
import ch.njol.skript.lang.parser.ParserInstance;
import ch.njol.skript.log.ErrorQuality;
import ch.njol.skript.sections.SecConditional;
Expand All @@ -51,7 +46,7 @@
"\tset loop-block to water"
})
@Since("<i>unknown</i> (before 2.1)")
public class EffExit extends Effect { // TODO [code style] warn user about code after a stop effect
public class EffExit extends Effect {

static {
Skript.registerEffect(EffExit.class,
Expand All @@ -60,15 +55,15 @@ public class EffExit extends Effect { // TODO [code style] warn user about code
"(exit|stop) <\\d+> (section|1:loop|2:conditional)s",
"(exit|stop) all (section|1:loop|2:conditional)s");
}

private int breakLevels;

private static final int EVERYTHING = 0;
private static final int LOOPS = 1;
private static final int CONDITIONALS = 2;
private static final String[] names = {"sections", "loops", "conditionals"};
private int type;

@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parser) {
switch (matchedPattern) {
Expand Down Expand Up @@ -99,7 +94,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
}
return true;
}

private static int numLevels(int type) {
List<TriggerSection> currentSections = ParserInstance.get().getCurrentSections();
if (type == EVERYTHING)
Expand All @@ -111,7 +106,7 @@ private static int numLevels(int type) {
}
return level;
}

@Override
@Nullable
protected TriggerItem walk(Event event) {
Expand All @@ -131,15 +126,20 @@ protected TriggerItem walk(Event event) {
}
return node instanceof LoopSection ? ((LoopSection) node).getActualNext() : node.getNext();
}

@Override
protected void execute(Event event) {
assert false;
}


@Override
public @Nullable ExecutionIntent executionIntent() {
return ExecutionIntent.stopSections(breakLevels);
}

@Override
public String toString(@Nullable Event event, boolean debug) {
return "stop " + breakLevels + " " + names[type];
}

}
12 changes: 6 additions & 6 deletions src/main/java/ch/njol/skript/effects/EffReturn.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,9 @@
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.ReturnHandler;
import ch.njol.skript.lang.*;
import ch.njol.skript.lang.ReturnHandler.ReturnHandlerStack;
import ch.njol.skript.lang.SectionExitHandler;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.skript.lang.TriggerItem;
import ch.njol.skript.lang.TriggerSection;
import ch.njol.skript.lang.parser.ParserInstance;
import ch.njol.skript.log.RetainingLogHandler;
import ch.njol.skript.log.SkriptLogger;
Expand Down Expand Up @@ -130,6 +125,11 @@ protected void execute(Event event) {
assert false;
}

@Override
public ExecutionIntent executionIntent() {
return ExecutionIntent.stopTrigger();
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public String toString(@Nullable Event event, boolean debug) {
return "return " + value.toString(event, debug);
Expand Down
45 changes: 15 additions & 30 deletions src/main/java/ch/njol/skript/effects/EffSuppressWarnings.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import ch.njol.util.Kleenean;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;
import org.jetbrains.annotations.UnknownNullability;
import org.skriptlang.skript.lang.script.ScriptWarning;

@Name("Locally Suppress Warning")
Expand All @@ -41,13 +42,17 @@
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]|5:deprecated syntax) warning[s]"
);
StringBuilder warnings = new StringBuilder();
ScriptWarning[] values = ScriptWarning.values();
for (int i = 0; i < values.length; i++) {
if (i != 0)
warnings.append('|');
warnings.append(values[i].ordinal()).append(':').append(values[i].getPattern());
}
Skript.registerEffect(EffSuppressWarnings.class, "[local[ly]] suppress [the] (" + warnings + ") warning[s]");
}

private static final int CONFLICT = 1, INSTANCE = 2, CONJUNCTION = 3, START_EXPR = 4, DEPRECATED = 5;
private int mark = 0;
private @UnknownNullability ScriptWarning warning;

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

mark = parseResult.mark;
if (mark == 1) {
Skript.warning("Variable conflict warnings no longer need suppression, as they have been removed altogether");
warning = ScriptWarning.values()[parseResult.mark];
if (warning.isDeprecated()) {
Skript.warning(warning.getDeprecationMessage());
} else {
getParser().getCurrentScript().suppressWarning(ScriptWarning.values()[mark - 2]);
getParser().getCurrentScript().suppressWarning(warning);
}
return true;
}
Expand All @@ -70,27 +75,7 @@ protected void execute(Event event) { }

@Override
public String toString(@Nullable Event event, boolean debug) {
String word;
switch (mark) {
case CONFLICT:
word = "conflict";
break;
case INSTANCE:
word = "variable save";
break;
case CONJUNCTION:
word = "missing conjunction";
break;
case START_EXPR:
word = "starting expression";
break;
case DEPRECATED:
word = "deprecated syntax";
break;
default:
throw new IllegalStateException();
}
return "suppress " + word + " warnings";
return "suppress " + warning.getWarningName() + " warnings";
}

}
69 changes: 69 additions & 0 deletions src/main/java/ch/njol/skript/lang/ExecutionIntent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package ch.njol.skript.lang;

import com.google.common.base.Preconditions;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/**
* Used to describe the intention of a {@link TriggerItem}.
* Currently only used to tell whether the item halts the execution or not and print the appropriate warnings.
*
* @see TriggerItem#executionIntent()
*/
public sealed interface ExecutionIntent extends Comparable<ExecutionIntent>
permits ExecutionIntent.StopTrigger, ExecutionIntent.StopSections {

static StopTrigger stopTrigger() {
return new StopTrigger();
}

static StopSections stopSections(int levels) {
Preconditions.checkArgument(levels > 0, "Depth must be at least 1");
return new StopSections(levels);
}

static StopSections stopSection() {
return new StopSections(1);
}
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved

@Nullable ExecutionIntent use();

final class StopTrigger implements ExecutionIntent {
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved

private StopTrigger() {}

@Override
public StopTrigger use() {
return new StopTrigger();
}

@Override
@SuppressWarnings("ComparatorMethodParameterNotUsed")
public int compareTo(@NotNull ExecutionIntent other) {
return other instanceof StopTrigger ? 0 : 1;
}

@Override
public String toString() {
return "StopTrigger";
}

}

record StopSections(int levels) implements ExecutionIntent {

public @Nullable ExecutionIntent.StopSections use() {
return levels > 1 ? new StopSections(levels - 1) : null;
}

@Override
public int compareTo(@NotNull ExecutionIntent other) {
if (other instanceof StopTrigger)
return -1;
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
int levels = ((StopSections) other).levels;
return Integer.compare(this.levels, levels);
}

}

}
24 changes: 19 additions & 5 deletions src/main/java/ch/njol/skript/lang/ExpressionList.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,10 @@
import ch.njol.util.coll.CollectionUtils;
import org.bukkit.event.Event;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.Unmodifiable;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.*;

/**
* A list of expressions.
Expand Down Expand Up @@ -302,6 +299,23 @@ public Expression<? extends T>[] getExpressions() {
return expressions;
}

/**
* Retrieves all expressions, including those nested within any {@code ExpressionList}s.
*
* @return A list of all expressions.
*/
public List<Expression<? extends T>> getAllExpressions() {
APickledWalrus marked this conversation as resolved.
Show resolved Hide resolved
List<Expression<? extends T>> expressions = new ArrayList<>();
for (Expression<? extends T> expression : this.expressions) {
if (expression instanceof ExpressionList<?>) {
expressions.addAll(((ExpressionList<? extends T>) expression).getAllExpressions());
continue;
}
expressions.add(expression);
}
return expressions;
}

@Override
@SuppressWarnings("unchecked")
public Expression<T> simplify() {
Expand Down
Loading
Loading