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 1 commit
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
32 changes: 20 additions & 12 deletions src/main/java/ch/njol/skript/ScriptLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -949,9 +949,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 +963,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 +980,40 @@ 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)
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
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
85 changes: 85 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,85 @@
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 abstract class ExecutionIntent implements Comparable<ExecutionIntent> {
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved

private ExecutionIntent() {}

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

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

public static StopSections stopSection() {
return new StopSections(1);
}

public abstract @Nullable ExecutionIntent use();

public static class StopTrigger extends ExecutionIntent {

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";
}

}

public static class StopSections extends ExecutionIntent {

private final int levels;

public StopSections(int levels) {
this.levels = levels;
}

public int levels() {
return levels;
}

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

@Override
public String toString() {
return "StopSections(" + 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
18 changes: 18 additions & 0 deletions src/main/java/ch/njol/skript/lang/TriggerItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,20 @@ public static boolean walk(TriggerItem start, Event event) {
return false;
}

/**
* Returns whether this item stops the execution of the current trigger or section(s).
* <br>
* If present, and there are statement(s) after this one, the parser will print a warning
* to the user.
* <p>
* <b>Note: This method is used purely to print warnings and doesn't affect parsing, execution or anything else.</b>
*
* @return whether this item stops the execution of the current trigger or section.
*/
public @Nullable ExecutionIntent executionIntent() {
return null;
}

/**
* how much to indent each level
*/
Expand Down Expand Up @@ -167,4 +181,8 @@ public TriggerItem setNext(@Nullable TriggerItem next) {
return next;
}

public @Nullable TriggerItem getActualNext() {
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
return next;
}

}
12 changes: 12 additions & 0 deletions src/main/java/ch/njol/skript/lang/TriggerSection.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,16 @@ protected final boolean run(Event event) {
}
}

protected @Nullable ExecutionIntent triggerExecutionIntent() {
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
TriggerItem current = first;
while (current != null && current != last) {
ExecutionIntent executionIntent = current.executionIntent();
if (executionIntent != null)
return executionIntent.use();
current = current.getActualNext();
}
ExecutionIntent executionIntent = current != null ? current.executionIntent() : null;
return executionIntent != null ? executionIntent.use() : null;
UnderscoreTud marked this conversation as resolved.
Show resolved Hide resolved
}

}
Loading
Loading