Add variable disambiguation to prevent bdd confusion

Author: mtdowling

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/Template.java

@@ -22,7 +22,6 @@
 import software.amazon.smithy.rulesengine.language.evaluation.TypeCheck;
 import software.amazon.smithy.rulesengine.language.evaluation.type.Type;
 import software.amazon.smithy.rulesengine.language.syntax.ToExpression;
-import software.amazon.smithy.utils.SmithyBuilder;
 import software.amazon.smithy.utils.SmithyUnstableApi;
 
 /**
@@ -41,7 +40,7 @@ public final class Template implements FromSourceLocation, ToNode {
     private final String value;
 
     public Template(StringNode template) {
-        sourceLocation = SmithyBuilder.requiredState("source", template.getSourceLocation());
+        sourceLocation = template.getSourceLocation();
         value = template.getValue();
         parts = context("when parsing template", template, () -> parseTemplate(template.getValue(), template));
     }

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/GetAttr.java

@@ -9,7 +9,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
-import java.util.Set;
 import software.amazon.smithy.model.FromSourceLocation;
 import software.amazon.smithy.model.node.ArrayNode;
 import software.amazon.smithy.model.node.Node;
@@ -155,11 +154,6 @@ public List<Part> getPath() {
         return path;
     }
 
-    @Override
-    public Set<String> getReferences() {
-        return getTarget().getReferences();
-    }
-
     @Override
     public <R> R accept(ExpressionVisitor<R> visitor) {
         return visitor.visitGetAttr(this);

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/functions/LibraryFunction.java

@@ -5,7 +5,6 @@
 package software.amazon.smithy.rulesengine.language.syntax.expressions.functions;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
@@ -54,7 +53,7 @@ public Set<String> getReferences() {
         for (Expression arg : getArguments()) {
             references.addAll(arg.getReferences());
         }
-        return references.isEmpty() ? Collections.emptySet() : references;
+        return references;
     }
 
     /**

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/literal/RecordLiteral.java

@@ -81,6 +81,6 @@ public Set<String> getReferences() {
         for (Literal value : members().values()) {
             references.addAll(value.getReferences());
         }
-        return references.isEmpty() ? Collections.emptySet() : references;
+        return references;
     }
 }

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/literal/StringLiteral.java

@@ -83,6 +83,7 @@ public Set<String> getReferences() {
                 references.addAll(((Template.Dynamic) part).toExpression().getReferences());
             }
         }
-        return references.isEmpty() ? Collections.emptySet() : references;
+
+        return references;
     }
 }

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expressions/literal/TupleLiteral.java

@@ -5,7 +5,6 @@
 package software.amazon.smithy.rulesengine.language.syntax.expressions.literal;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
@@ -88,6 +87,6 @@ public Set<String> getReferences() {
         for (Literal member : members()) {
             references.addAll(member.getReferences());
         }
-        return references.isEmpty() ? Collections.emptySet() : references;
+        return references;
     }
 }

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/rule/Rule.java

@@ -242,7 +242,7 @@ public Builder conditions(ToCondition... conditions) {
             return this;
         }
 
-        public Builder conditions(List<ToCondition> conditions) {
+        public Builder conditions(List<? extends ToCondition> conditions) {
             this.conditions.addAll(conditions);
             return this;
         }
@@ -257,11 +257,15 @@ public EndpointRule endpoint(Endpoint endpoint) {
         }
 
         public ErrorRule error(Node error) {
-            return (ErrorRule) this.onBuild.apply(new ErrorRule(this, Expression.fromNode(error)));
+            return error(Expression.fromNode(error));
         }
 
         public ErrorRule error(String error) {
-            return (ErrorRule) this.onBuild.apply(new ErrorRule(this, Literal.of(error)));
+            return error(Literal.of(error));
+        }
+
+        public ErrorRule error(Expression error) {
+            return (ErrorRule) this.onBuild.apply(new ErrorRule(this, error));
         }
 
         public TreeRule treeRule(Rule... rules) {

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/logic/bdd/Bdd.java

@@ -31,12 +31,12 @@
  *   <li>{@code -1}: FALSE terminal; represents boolean false, treated as "no match" in endpoint resolution</li>
  *   <li>{@code 2, 3, ...}: Node references (points to nodes array at index ref-1)</li>
  *   <li>{@code -2, -3, ...}: Complement node references (logical NOT of the referenced node)</li>
- *   <li>{@code 2000000+}: Result terminals (2000000 + resultIndex)</li>
+ *   <li>{@code 100_000_000+}: Result terminals (100_000_000 + resultIndex)</li>
  * </ul>
  *
- * <p>Result terminals are encoded as special references starting at 2000000 (RESULT_OFFSET).
- * When evaluating the BDD, any reference >= 2000000 represents a result terminal that
- * indexes into the results array (resultIndex = ref - 2000000). These are not stored
+ * <p>Result terminals are encoded as special references starting at 100_000_000 (RESULT_OFFSET).
+ * When evaluating the BDD, any reference >= 100_000_000 represents a result terminal that
+ * indexes into the results array (resultIndex = ref - 100_000_000). These are not stored
  * as nodes in the nodes array.
  *
  * <p><b>Node Format:</b> {@code [variable, high, low]}
@@ -50,9 +50,9 @@ public final class Bdd implements ToNode {
     /**
      * Result reference encoding.
      *
-     * <p>Results start at 2M to avoid collision with node references.
+     * <p>Results start at 100M to avoid collision with node references.
      */
-    public static final int RESULT_OFFSET = 2000000;
+    public static final int RESULT_OFFSET = 100_000_000;
 
     private final Parameters parameters;
     private final List<Condition> conditions;

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/logic/bdd/BddBuilder.java

@@ -24,7 +24,7 @@
  *   <li>-1: FALSE terminal</li>
  *   <li>2, 3, 4, ...: BDD nodes (use index + 1)</li>
  *   <li>-2, -3, -4, ...: Complement of BDD nodes</li>
- *   <li>Bdd.RESULT_OFFSET+: Result terminals (2000000 + resultIndex)</li>
+ *   <li>Bdd.RESULT_OFFSET+: Result terminals (100_000_000 + resultIndex)</li>
  * </ul>
  *
  * <p>Node storage format: [variableIndex, highRef, lowRef]

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/logic/bdd/BddNodeHelpers.java

@@ -17,6 +17,7 @@
 import software.amazon.smithy.model.node.ObjectNode;
 import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameters;
 import software.amazon.smithy.rulesengine.language.syntax.rule.Condition;
+import software.amazon.smithy.rulesengine.language.syntax.rule.NoMatchRule;
 import software.amazon.smithy.rulesengine.language.syntax.rule.Rule;
 import software.amazon.smithy.utils.SetUtils;
 
@@ -41,8 +42,15 @@ static Node toNode(Bdd bdd) {
         }
 
         List<Node> results = new ArrayList<>();
-        for (Rule r : bdd.getResults()) {
-            results.add(r.toNode());
+        if (!(bdd.getResults().get(0) instanceof NoMatchRule)) {
+            throw new IllegalArgumentException("BDD must always have a NoMatchRule as the first result");
+        }
+        for (int i = 1; i < bdd.getResults().size(); i++) {
+            Rule result = bdd.getResults().get(i);
+            if (result instanceof NoMatchRule) {
+                throw new IllegalArgumentException("NoMatch rules can only appear at rule index 0. Found at index" + i);
+            }
+            results.add(bdd.getResults().get(i).toNode());
         }
 
         return builder
@@ -60,7 +68,13 @@ static Bdd fromNode(Node node) {
         obj.warnIfAdditionalProperties(ALLOWED_PROPERTIES);
         Parameters params = Parameters.fromNode(obj.expectObjectMember("parameters"));
         List<Condition> conditions = obj.expectArrayMember("conditions").getElementsAs(Condition::fromNode);
-        List<Rule> results = obj.expectArrayMember("results").getElementsAs(Rule::fromNode);
+
+        // Read the results and prepend NoMatchRule at index 0
+        List<Rule> serializedResults = obj.expectArrayMember("results").getElementsAs(Rule::fromNode);
+        List<Rule> results = new ArrayList<>();
+        results.add(NoMatchRule.INSTANCE); // Always add no-match at index 0
+        results.addAll(serializedResults);
+
         String nodesBase64 = obj.expectStringMember("nodes").getValue();
         int nodeCount = obj.expectNumberMember("nodeCount").getValue().intValue();
         int[][] nodes = decodeNodes(nodesBase64, nodeCount);