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 12 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 @@ -174,6 +174,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
68 changes: 46 additions & 22 deletions src/main/java/ch/njol/skript/effects/EffContinue.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,15 @@
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.Literal;
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.util.Kleenean;
import ch.njol.util.StringUtils;
import org.bukkit.event.Event;
import org.eclipse.jdt.annotation.Nullable;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.UnknownNullability;

import java.util.ArrayList;
import java.util.List;

@Name("Continue")
Expand Down Expand Up @@ -65,35 +63,56 @@ public class EffContinue extends Effect {
);
}

@SuppressWarnings("NotNullFieldNotInitialized")
private LoopSection loop;
@SuppressWarnings("NotNullFieldNotInitialized")
private List<LoopSection> innerLoops;
private @UnknownNullability LoopSection loop;
private @UnknownNullability List<LoopSection> innerLoops;
private int breakLevels;

@Override
@SuppressWarnings("unchecked")
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
List<LoopSection> currentLoops = getParser().getCurrentSections(LoopSection.class);
List<TriggerSection> sections = getParser().getCurrentSections();
innerLoops = new ArrayList<>();
int loopLevels = 0;
LoopSection lastLoop = null;

int size = currentLoops.size();
if (size == 0) {
Skript.error("The 'continue' effect may only be used in loops");
int level = matchedPattern == 0 ? -1 : ((Literal<Integer>) exprs[0]).getSingle();
if (matchedPattern != 0 && level < 1) {
Skript.error("Can't continue the " + StringUtils.fancyOrderNumber(level) + " loop");
return false;
}

int level = matchedPattern == 0 ? size : ((Literal<Integer>) exprs[0]).getSingle();
if (level < 1) {
Skript.error("Can't continue the " + StringUtils.fancyOrderNumber(level) + " loop");
for (TriggerSection section : sections) {
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
if (loop != null)
breakLevels++;
if (!(section instanceof LoopSection loopSection))
continue;
loopLevels++;
if (level == -1) {
lastLoop = loopSection;
} else if (loopLevels == level) {
loop = loopSection;
breakLevels++;
} else if (loopLevels > level) {
innerLoops.add(loopSection);
}
}

if (loopLevels == 0) {
Skript.error("The 'continue' effect may only be used in loops");
return false;
}
if (level > size) {

if (level > loopLevels) {
Skript.error("Can't continue the " + StringUtils.fancyOrderNumber(level) + " loop as there " +
(size == 1 ? "is only 1 loop" : "are only " + size + " loops") + " present");
(loopLevels == 1 ? "is only 1 loop" : "are only " + loopLevels + " loops") + " present");
return false;
}

loop = currentLoops.get(level - 1);
innerLoops = currentLoops.subList(level, size);
if (level == -1) {
loop = lastLoop;
breakLevels++;
}

return true;
}

Expand All @@ -110,9 +129,14 @@ protected TriggerItem walk(Event event) {
return loop;
}

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

@Override
public String toString(@Nullable Event event, boolean debug) {
return "continue";
return "continue" + (loop == null ? "" : " the " + StringUtils.fancyOrderNumber(innerLoops.size() + 1) + " loop");
}

}
36 changes: 18 additions & 18 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 @@ -123,23 +118,28 @@ protected TriggerItem walk(Event event) {
assert false : this;
return null;
}
if (node instanceof SectionExitHandler)
((SectionExitHandler) node).exit(event);
if (node instanceof SectionExitHandler exitHandler)
exitHandler.exit(event);

if (type == EVERYTHING || type == CONDITIONALS && node instanceof SecConditional || type == LOOPS && (node instanceof LoopSection))
i--;
}
return node instanceof LoopSection ? ((LoopSection) node).getActualNext() : node.getNext();
return node instanceof 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];
}

}
20 changes: 10 additions & 10 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 @@ -114,14 +109,14 @@ protected TriggerItem walk(Event event) {

TriggerSection parent = getParent();
while (parent != null && parent != handler) {
if (parent instanceof SectionExitHandler)
((SectionExitHandler) parent).exit(event);
if (parent instanceof SectionExitHandler exitHandler)
exitHandler.exit(event);

parent = parent.getParent();
}

if (handler instanceof SectionExitHandler)
((SectionExitHandler) handler).exit(event);
if (handler instanceof SectionExitHandler exitHandler)
exitHandler.exit(event);

return null;
}
Expand All @@ -131,6 +126,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
Loading
Loading