Rework/expressions (#117)

* 💡 Expression (utility methods, changer rework, stricter changes)

* 💡 VariableMap (better index ordering)

* 💡 SyntaxParser (restrict lists to only allow the same type)

* 💡 ExpressionList (revert restriction but improve lists instead)

* 💡 CondExprContains/CondExprDateCompare

* 💡 TaggedExpression (improved tag parsing for expression lists)

* 💡 conditions (improved code & logic improvements)

* 💡 expressions (apply improvements + property changes)

* 🔥 ExprNumberConvertBase/ExprDateNow (merge conflicts)

* 🔥 CondExprIsEmpty (fix CCE)

* 🔥 CondExprContains (make use of stored comparator)

* 💡 Expression (last #acceptsChange() changes and fixes)

* 💡 PatternInfos (#toChoiceGroup() so properties can use it too)

* 💡 PropertyConditional (removing TriggerContext parameter & better error messages)

Co-authored-by: Mwexim <[email protected]>

Author: Mwexim

src/main/java/io/github/syst3ms/skriptparser/effects/EffChange.java

@@ -5,18 +5,12 @@
 import io.github.syst3ms.skriptparser.lang.Expression;
 import io.github.syst3ms.skriptparser.lang.TriggerContext;
 import io.github.syst3ms.skriptparser.log.ErrorType;
-import io.github.syst3ms.skriptparser.log.SkriptLogger;
 import io.github.syst3ms.skriptparser.parsing.ParseContext;
 import io.github.syst3ms.skriptparser.registration.PatternInfos;
-import io.github.syst3ms.skriptparser.types.Type;
 import io.github.syst3ms.skriptparser.types.TypeManager;
 import io.github.syst3ms.skriptparser.types.changers.ChangeMode;
-import io.github.syst3ms.skriptparser.util.ClassUtils;
-import io.github.syst3ms.skriptparser.util.StringUtils;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.Optional;
-
 /**
  *  A very general effect that can change many expressions. Many expressions can only be set and/or deleted, while some can have things added to or removed from them.
  *
@@ -54,31 +48,30 @@ public class EffChange extends Effect {
     @Nullable
     private Expression<?> changeWith;
     private ChangeMode mode;
-    private boolean assignment; // A simple flag for identifying which syntax was precisely used
 
     @Override
     public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContext parseContext) {
         ChangeMode mode = PATTERNS.getInfo(matchedPattern);
         if (mode == ChangeMode.RESET || mode == ChangeMode.DELETE) {
             changed = expressions[0];
-        } else if ((matchedPattern & 1) == 1 || mode == ChangeMode.SET) { // Notice the difference in the order of expressions
+        } else if ((matchedPattern & 1) == 1 || mode == ChangeMode.SET) {
+            // Notice the difference in the order of expressions
             changed = expressions[0];
             changeWith = expressions[1];
-            assignment = (matchedPattern & 1) == 1;
         } else {
             changed = expressions[1];
             changeWith = expressions[0];
         }
         this.mode = mode;
-        SkriptLogger logger = parseContext.getLogger();
+
+        var logger = parseContext.getLogger();
         String changedString = changed.toString(TriggerContext.DUMMY, logger.isDebug());
+
         if (changeWith == null) {
             assert mode == ChangeMode.DELETE || mode == ChangeMode.RESET;
             return changed.acceptsChange(mode).isPresent();
         } else {
-            Class<?> changeType = changeWith.getReturnType();
-            Optional<Class<?>[]> acceptance = changed.acceptsChange(mode);
-            if (acceptance.isEmpty()) {
+            if (changed.acceptsChange(mode).isEmpty()) {
                 switch (mode) {
                     case SET:
                         logger.error(changedString + " cannot be set to anything", ErrorType.SEMANTIC_ERROR);
@@ -90,19 +83,14 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
                     case REMOVE:
                         logger.error("Nothing can be removed from " + changedString, ErrorType.SEMANTIC_ERROR);
                         break;
-                    case DELETE:
-                    case RESET:
+                    default:
                     	throw new IllegalStateException();
                 }
                 return false;
-            } else if (!ClassUtils.containsSuperclass(acceptance.get(), changeType)) {
-                boolean array = changeType.isArray();
-                Optional<? extends Type<?>> type = TypeManager.getByClassExact(changeType);
+            } else if (!changed.acceptsChange(mode, changeWith)) {
+                var type = TypeManager.getByClassExact(changeWith.getReturnType());
                 assert type.isPresent();
-                String changeTypeName = StringUtils.withIndefiniteArticle(
-                    type.get().getPluralForms()[array ? 1 : 0],
-                    array
-                );
+                String changeTypeName = type.get().withIndefiniteArticle(!changeWith.isSingle());
                 switch (mode) {
                     case SET:
                         logger.error(changedString + " cannot be set to " + changeTypeName, ErrorType.SEMANTIC_ERROR);
@@ -127,12 +115,12 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
     @Override
     public void execute(TriggerContext ctx) {
         if (changeWith == null) {
-            changed.change(ctx, new Object[0], mode);
+            changed.change(ctx, mode, new Object[0]);
         } else {
-            var values= changeWith.getValues(ctx);
+            var values = changeWith.getValues(ctx);
             if (values.length == 0)
                 return;
-            changed.change(ctx, values, mode);
+            changed.change(ctx, mode, values);
         }
     }
 
@@ -142,23 +130,11 @@ public String toString(TriggerContext ctx, boolean debug) {
         String changedWithString = changeWith != null ? changeWith.toString(ctx, debug) : "";
         switch (mode) {
             case SET:
-                if (assignment) {
-                    return String.format("%s = %s", changedString, changedWithString);
-                } else {
-                    return String.format("set %s to %s", changedString, changedWithString);
-                }
+                return String.format("set %s to %s", changedString, changedWithString);
             case ADD:
-                if (assignment) {
-                    return String.format("%s += %s", changedString, changedWithString);
-                } else {
-                    return String.format("add %s to %s", changedWithString, changedString);
-                }
+                return String.format("add %s to %s", changedWithString, changedString);
             case REMOVE:
-                if (assignment) {
-                    return String.format("%s -= %s", changedString, changedWithString);
-                } else {
-                    return String.format("remove %s from %s", changedWithString, changedString);
-                }
+                return String.format("remove %s from %s", changedWithString, changedString);
             case DELETE:
             case RESET:
                 return String.format("%s %s", mode.name().toLowerCase(), changedString);

src/main/java/io/github/syst3ms/skriptparser/effects/EffPrint.java

@@ -4,8 +4,8 @@
 import io.github.syst3ms.skriptparser.lang.Effect;
 import io.github.syst3ms.skriptparser.lang.Expression;
 import io.github.syst3ms.skriptparser.lang.TriggerContext;
+import io.github.syst3ms.skriptparser.lang.base.TaggedExpression;
 import io.github.syst3ms.skriptparser.parsing.ParseContext;
-import io.github.syst3ms.skriptparser.util.StringUtils;
 
 /**
  * Prints some text to the console
@@ -23,25 +23,24 @@ public class EffPrint extends Effect {
         );
     }
 
-    private Expression<String> string;
+    private Expression<String> expression;
 
     @SuppressWarnings("unchecked")
     @Override
     public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContext parseContext) {
-        string = (Expression<String>) expressions[0];
+        expression = (Expression<String>) expressions[0];
         return true;
     }
 
     @Override
     public void execute(TriggerContext ctx) {
-        String[] strs = StringUtils.applyTags(string, ctx, "console");
-        for (String str : strs) {
-            System.out.println(str);
+        for (String val : TaggedExpression.apply(expression, ctx, "console")) {
+            System.out.println(val);
         }
     }
 
     @Override
     public String toString(TriggerContext ctx, boolean debug) {
-        return "print " + string.toString(ctx, debug);
+        return "print " + expression.toString(ctx, debug);
     }
 }

src/main/java/io/github/syst3ms/skriptparser/expressions/CondExprCompare.java

@@ -215,16 +215,6 @@ private boolean initComparator() {
 
         if (f == Object.class || s == Object.class) {
             return true;
-        } else if (f != s) {
-            // Tries to convert the instances to each other.
-            // Basically takes expression conversions into account.
-            var converted = Expression.convertPair(first, second);
-            if (!first.equals(converted.getFirst()) || !second.equals(converted.getSecond())) {
-                first = converted.getFirst();
-                second = converted.getSecond();
-                comparator = (Comparator<Object, Object>) Comparators.getComparator(first.getReturnType(), second.getReturnType()).orElseThrow(AssertionError::new);
-                return true;
-            }
         }
         return (comparator = (Comparator<Object, Object>) Comparators.getComparator(f, s).orElse(null)) != null;
     }
@@ -265,9 +255,9 @@ private boolean initComparator() {
      */
     @Override
     public boolean check(TriggerContext ctx) {
-        Object[] firstValues = first.getValues(ctx);
-        Object[] secondValues = second.getValues(ctx);
-        Object[] thirdValues = third != null ? third.getValues(ctx) : null;
+        Object[] firstValues = first.getArray(ctx);
+        Object[] secondValues = second.getArray(ctx);
+        Object[] thirdValues = third != null ? third.getArray(ctx) : null;
         if (thirdValues == null) {
             if (firstEach && secondEach) {
                 if (firstValues.length != secondValues.length)

src/main/java/io/github/syst3ms/skriptparser/expressions/CondExprContains.java

@@ -4,10 +4,13 @@
 import io.github.syst3ms.skriptparser.lang.Expression;
 import io.github.syst3ms.skriptparser.lang.TriggerContext;
 import io.github.syst3ms.skriptparser.lang.base.ConditionalExpression;
+import io.github.syst3ms.skriptparser.log.ErrorType;
 import io.github.syst3ms.skriptparser.parsing.ParseContext;
-import io.github.syst3ms.skriptparser.util.CollectionUtils;
-
-import java.util.Optional;
+import io.github.syst3ms.skriptparser.types.comparisons.Comparator;
+import io.github.syst3ms.skriptparser.types.comparisons.Comparators;
+import io.github.syst3ms.skriptparser.types.comparisons.Relation;
+import io.github.syst3ms.skriptparser.util.DoubleOptional;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * See if a given list of objects contain a given element.
@@ -16,7 +19,7 @@
  * @name Contain
  * @type CONDITION
  * @pattern %string% [does(n't| not)] contain[s] %string%
- * @pattern %objects% [do[es](n't| not)] contain[s] %object%
+ * @pattern %objects% [do[es](n't| not)] contain[s] %objects%
  * @since ALPHA
  * @author Mwexim
  */
@@ -28,33 +31,68 @@ public class CondExprContains extends ConditionalExpression {
                 true,
                 2,
                 "%string% [1:does(n't| not)] contain[s] %string%",
-                "%objects% [1:do[es](n't| not)] contain[s] %object%"
+                "%objects% [1:do[es](n't| not)] contain[s] %objects%"
         );
     }
 
-    private Expression<?> first, second;
+    private Expression<Object> first, second;
+    @Nullable
+    private Comparator<Object, Object> comparator;
     private boolean onlyString;
 
+    @SuppressWarnings("unchecked")
     @Override
     public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContext parseContext) {
-        first = expressions[0];
-        second = expressions[1];
+        first = (Expression<Object>) expressions[0];
+        second = (Expression<Object>) expressions[1];
+        comparator = (Comparator<Object, Object>) Comparators.getComparator(first.getReturnType(), second.getReturnType()).orElse(null);
+        // If the expressions are variables, their return type is unknown at parse time
+        if (first.getReturnType() != Object.class
+                && second.getReturnType() != Object.class
+                && comparator == null) {
+            var logger = parseContext.getLogger();
+            logger.error(
+                    "'" +
+                    first.toString(TriggerContext.DUMMY, logger.isDebug()) +
+                    "' can never contain '" +
+                    second.toString(TriggerContext.DUMMY, logger.isDebug()) +
+                    "' because their values cannot be compared",
+                    ErrorType.SEMANTIC_ERROR
+            );
+            return false;
+        }
+
         onlyString = matchedPattern == 0;
         setNegated(parseContext.getNumericMark() == 1);
+        if (!onlyString && !first.isAndList()) {
+            parseContext.getLogger().error(
+                    "An or-list cannot contain any values",
+                    ErrorType.SEMANTIC_ERROR,
+                    "If you want to check if an or-list 'contains' a value, you should do an equality check instead."
+            );
+            return false;
+        }
         return true;
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public boolean check(TriggerContext ctx) {
         if (onlyString) {
-            Optional<? extends String> f = ((Expression<String>) first).getSingle(ctx);
-            Optional<? extends String> s = ((Expression<String>) second).getSingle(ctx);
-            return isNegated() != f.filter(o1 -> s.map(o1::contains).isPresent()).isPresent();
+            return isNegated() != DoubleOptional.ofOptional(first.getSingle(ctx), second.getSingle(ctx))
+                    .map(toCheck -> (String) toCheck, toMatch -> (String) toMatch)
+                    .mapToOptional(String::contains)
+                    .orElse(false);
         } else {
-            Object[] f = ((Expression<Object>) first).getValues(ctx);
-            Object[] s = ((Expression<Object>) second).getValues(ctx);
-            return isNegated() != CollectionUtils.contains(f, s);
+            return second.check(
+                    ctx,
+                    toMatch -> Expression.check(
+                            first.getValues(ctx),
+                            toCheck -> (comparator == null ? Comparators.compare(toCheck, toMatch) : comparator.apply(toCheck, toMatch)).is(Relation.EQUAL),
+                            false,
+                            !first.isAndList()
+                    ),
+                    isNegated()
+            );
         }
     }
 

src/main/java/io/github/syst3ms/skriptparser/expressions/CondExprDateCompare.java

@@ -5,7 +5,6 @@
 import io.github.syst3ms.skriptparser.lang.TriggerContext;
 import io.github.syst3ms.skriptparser.lang.base.ConditionalExpression;
 import io.github.syst3ms.skriptparser.parsing.ParseContext;
-import io.github.syst3ms.skriptparser.util.DoubleOptional;
 import io.github.syst3ms.skriptparser.util.SkriptDate;
 
 import java.time.Duration;
@@ -26,8 +25,8 @@ public class CondExprDateCompare extends ConditionalExpression {
                 CondExprDateCompare.class,
                 Boolean.class,
                 true,
-                "%date% (was|were)( more|(n't| not) less) than %duration% [ago]",
-                "%date% (was|were)((n't| not) more| less) than %duration% [ago]"
+                "%dates% (was|were)( more|(n't| not) less) than %duration% [ago]",
+                "%dates% (was|were)((n't| not) more| less) than %duration% [ago]"
         );
     }
 
@@ -45,11 +44,13 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
 
     @Override
     public boolean check(TriggerContext ctx) {
-        return DoubleOptional.ofOptional(date.getSingle(ctx), duration.getSingle(ctx))
-                .filter(
-                        (dat, dur) -> isNegated() != (dat.getTimestamp() < SkriptDate.now().getTimestamp() - dur.toMillis())
-                )
-                .isPresent();
+        return date.check(
+                ctx,
+                toCheck -> duration.getSingle(ctx)
+                        .filter(toCompare -> toCheck.compareTo(SkriptDate.now().minus(toCompare)) < 0)
+                        .isPresent(),
+                isNegated()
+        );
     }
 
     @Override

src/main/java/io/github/syst3ms/skriptparser/expressions/CondExprIsDivisible.java

@@ -9,7 +9,6 @@
 import io.github.syst3ms.skriptparser.util.math.BigDecimalMath;
 
 import java.math.BigInteger;
-import java.util.Arrays;
 
 /**
  * Check if a given number is divisible by another number.
@@ -42,15 +41,15 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, ParseContex
     }
 
     @Override
-    public boolean check(TriggerContext ctx, Number[] performers) {
-        if (performers.length == 0)
-            return isNegated();
-        return isNegated() != Arrays.stream(performers)
-                .allMatch(val -> divider.getSingle(ctx)
-                        .filter(__ -> BigDecimalMath.isIntValue(BigDecimalMath.getBigDecimal(val)))
-                        .filter(d -> BigDecimalMath.getBigInteger(val).mod(d).equals(BigInteger.ZERO))
-                        .isPresent()
-                );
+    public boolean check(TriggerContext ctx) {
+        return getPerformer().check(
+                ctx,
+                performer -> divider.getSingle(ctx)
+                        .filter(__ -> BigDecimalMath.isIntValue(BigDecimalMath.getBigDecimal(performer)))
+                        .filter(val -> BigDecimalMath.getBigInteger(performer).mod(val).equals(BigInteger.ZERO))
+                        .isPresent(),
+                isNegated()
+        );
     }
 
     @Override