Skip to content

Commit 3186fc6

Browse files
authored
Add warning for ambiguous arg subtraction (#7808)
* Add warning for ambiguous arg subtraction * optimize condition and check if the second literal is specifically 1. * Update src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java
1 parent 116c766 commit 3186fc6

File tree

2 files changed

+42
-12
lines changed

2 files changed

+42
-12
lines changed

src/main/java/ch/njol/skript/expressions/ExprArgument.java

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,5 @@
11
package ch.njol.skript.expressions;
22

3-
import java.util.List;
4-
import java.util.regex.MatchResult;
5-
6-
import ch.njol.skript.lang.EventRestrictedSyntax;
7-
import ch.njol.util.coll.CollectionUtils;
8-
import org.bukkit.event.Event;
9-
import org.bukkit.event.player.PlayerCommandPreprocessEvent;
10-
import org.bukkit.event.server.ServerCommandEvent;
11-
import org.jetbrains.annotations.Nullable;
12-
133
import ch.njol.skript.Skript;
144
import ch.njol.skript.classes.ClassInfo;
155
import ch.njol.skript.command.Argument;
@@ -19,6 +9,7 @@
199
import ch.njol.skript.doc.Examples;
2010
import ch.njol.skript.doc.Name;
2111
import ch.njol.skript.doc.Since;
12+
import ch.njol.skript.lang.EventRestrictedSyntax;
2213
import ch.njol.skript.lang.Expression;
2314
import ch.njol.skript.lang.ExpressionType;
2415
import ch.njol.skript.lang.Literal;
@@ -29,6 +20,14 @@
2920
import ch.njol.skript.util.Utils;
3021
import ch.njol.util.Kleenean;
3122
import ch.njol.util.StringUtils;
23+
import ch.njol.util.coll.CollectionUtils;
24+
import org.bukkit.event.Event;
25+
import org.bukkit.event.player.PlayerCommandPreprocessEvent;
26+
import org.bukkit.event.server.ServerCommandEvent;
27+
import org.jetbrains.annotations.Nullable;
28+
29+
import java.util.List;
30+
import java.util.regex.MatchResult;
3231

3332
@Name("Argument")
3433
@Description({
@@ -65,7 +64,9 @@ public class ExprArgument extends SimpleExpression<Object> implements EventRestr
6564
private Argument<?> argument;
6665

6766
private int ordinal = -1; // Available in ORDINAL and sometimes CLASSINFO
68-
67+
68+
private boolean couldCauseArithmeticConfusion = false;
69+
6970
@Override
7071
@SuppressWarnings("unchecked")
7172
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
@@ -81,6 +82,8 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
8182
break;
8283
case 3:
8384
what = parseResult.mark == 1 ? ALL : SINGLE;
85+
if (what == SINGLE && parseResult.expr.matches("(the )?arg(ument)?"))
86+
couldCauseArithmeticConfusion = true; // 'arg-1' could be parsed as 'argument - 1'
8487
break;
8588
case 4:
8689
case 5:
@@ -246,6 +249,13 @@ protected Object[] get(final Event e) {
246249
public boolean isSingle() {
247250
return argument != null ? argument.isSingle() : what != ALL;
248251
}
252+
253+
/**
254+
* @return whether the expression is 'arg', a single argument that could cause confusion with 'arg-1' being parsed as 'argument - 1'.
255+
*/
256+
public boolean couldCauseArithmeticConfusion() {
257+
return couldCauseArithmeticConfusion;
258+
}
249259

250260
@Override
251261
public Class<?> getReturnType() {

src/main/java/ch/njol/skript/expressions/arithmetic/ExprArithmetic.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import ch.njol.skript.doc.Examples;
77
import ch.njol.skript.doc.Name;
88
import ch.njol.skript.doc.Since;
9+
import ch.njol.skript.expressions.ExprArgument;
910
import ch.njol.skript.lang.Expression;
1011
import ch.njol.skript.lang.ExpressionType;
1112
import ch.njol.skript.lang.Literal;
@@ -26,8 +27,8 @@
2627

2728
import java.lang.reflect.Array;
2829
import java.util.ArrayList;
29-
import java.util.List;
3030
import java.util.Collection;
31+
import java.util.List;
3132

3233
@Name("Arithmetic")
3334
@Description("Arithmetic expressions, e.g. 1 + 2, (health of player - 2) / 3, etc.")
@@ -113,6 +114,9 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
113114
rightGrouped = patternInfo.rightGrouped;
114115
operator = patternInfo.operator;
115116

117+
// print warning for arg-1 confusion scenario
118+
printArgWarning(first, second, operator);
119+
116120
/*
117121
* Step 1: UnparsedLiteral Resolving
118122
*
@@ -294,6 +298,22 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
294298
return arithmeticGettable != null || error(firstClass, secondClass);
295299
}
296300

301+
private void printArgWarning(Expression<L> first, Expression<R> second, Operator operator) {
302+
if (operator == Operator.SUBTRACTION && !rightGrouped && !leftGrouped // if the operator is '-' and the user didn't use ()
303+
&& first instanceof ExprArgument argument && argument.couldCauseArithmeticConfusion() // if the first expression is 'arg'
304+
&& second instanceof ExprArithmetic<?, ?, ?> secondArith && secondArith.first instanceof Literal<?> literal // this ambiguity only occurs when the code is parsed as `arg - (1 * 2)` or a similar PEMDAS priority.
305+
&& literal.canReturn(Number.class)) {
306+
// ensure that the second literal is a 1
307+
Literal<?> secondLiteral = (Literal<?>) LiteralUtils.defendExpression(literal);
308+
if (LiteralUtils.canInitSafely(secondLiteral)) {
309+
double number = ((Number) secondLiteral.getSingle()).doubleValue();
310+
if (number == 1)
311+
Skript.warning("This subtraction is ambiguous and could be interpreted as either the 'first argument' expression ('argument-1') or as subtraction from the argument value ('(argument) - 1'). " +
312+
"If you meant to use 'argument-1', omit the hyphen ('arg 1') or use parentheses to clarify your intent.");
313+
}
314+
}
315+
}
316+
297317
@Override
298318
@SuppressWarnings("unchecked")
299319
protected T[] get(Event event) {

0 commit comments

Comments
 (0)