Fix return type of substring method (#1504)

* Fix return type of substring method * Add test & refactor tests that load resources * Use try-with-resources
smithy-lang · Dec 1, 2022 · 10fd0d1 · 10fd0d1
1 parent 9f393b6
commit 10fd0d1
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 23 deletions.
diff --git a/...es-engine/src/main/java/software/amazon/smithy/rulesengine/language/stdlib/Substring.java b/...es-engine/src/main/java/software/amazon/smithy/rulesengine/language/stdlib/Substring.java
@@ -44,7 +44,7 @@ public List<Type> getArguments() {
 
     @Override
     public Type getReturnType() {
-        return Type.string();
+        return Type.optional(Type.string());
     }
 
     @Override

diff --git a/...ine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expr/Expression.java b/...ine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/expr/Expression.java
@@ -208,7 +208,7 @@ public Type typeCheck(Scope<Type> scope) {
      */
     public Type type() {
         if (cachedType == null) {
-            throw new RuntimeException("you must call typeCheck first");
+            throw new RuntimeException("typechecking was never invoked on this expression.");
         }
         return cachedType;
     }

diff --git a/smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/RulesetTestUtil.java b/smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/RulesetTestUtil.java
@@ -0,0 +1,23 @@
+package software.amazon.smithy.rulesengine;
+
+import java.io.IOException;
+import java.io.InputStream;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
+
+public class RulesetTestUtil {
+    public static EndpointRuleSet loadRuleSet(String resourceId) {
+        try(InputStream is = RulesetTestUtil.class.getClassLoader().getResourceAsStream(resourceId)) {
+            Node node = ObjectNode.parse(is);
+            return EndpointRuleSet.fromNode(node);
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    public static EndpointRuleSet minimalRuleSet() {
+        return loadRuleSet("software/amazon/smithy/rulesengine/testutil/valid-rules/minimal-ruleset.json");
+    }
+
+}
diff --git a/...engine/src/test/java/software/amazon/smithy/rulesengine/language/EndpointRuleSetTest.java b/...engine/src/test/java/software/amazon/smithy/rulesengine/language/EndpointRuleSetTest.java
@@ -23,6 +23,7 @@
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.rulesengine.RulesetTestUtil;
 import software.amazon.smithy.rulesengine.language.eval.RuleEvaluator;
 import software.amazon.smithy.rulesengine.language.eval.Scope;
 import software.amazon.smithy.rulesengine.language.eval.Value;
@@ -35,17 +36,9 @@
 import software.amazon.smithy.utils.MapUtils;
 
 class EndpointRuleSetTest {
-    private EndpointRuleSet parse(String resource) {
-        InputStream is = getClass().getClassLoader().getResourceAsStream(resource);
-        assert is != null;
-        Node node = ObjectNode.parse(is);
-        return EndpointRuleSet.fromNode(node);
-    }
-
     @Test
     void testRuleEval() {
-        EndpointRuleSet actual = parse(
-                "software/amazon/smithy/rulesengine/testutil/valid-rules/minimal-ruleset.json");
+        EndpointRuleSet actual = RulesetTestUtil.minimalRuleSet();
         Value result = RuleEvaluator.evaluate(actual, MapUtils.of(Identifier.of("Region"),
                 Value.string("us-east-1")));
         Value.Endpoint expected = new Value.Endpoint.Builder(SourceLocation.none())
@@ -63,9 +56,7 @@ void testRuleEval() {
 
     @Test
     void testMinimalRuleset() {
-        EndpointRuleSet actual = parse(
-                "software/amazon/smithy/rulesengine/testutil/valid-rules/minimal-ruleset.json");
-        actual.typeCheck(new Scope<>());
+        EndpointRuleSet actual = RulesetTestUtil.minimalRuleSet();
         assertEquals(EndpointRuleSet.builder()
                 .version("1.3")
                 .parameters(Parameters

diff --git a/...gine/src/test/java/software/amazon/smithy/rulesengine/language/TypeIntrospectionTest.java b/...gine/src/test/java/software/amazon/smithy/rulesengine/language/TypeIntrospectionTest.java
@@ -0,0 +1,35 @@
+package software.amazon.smithy.rulesengine.language;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.InputStream;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.model.node.Node;
+import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.rulesengine.RulesetTestUtil;
+import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
+import software.amazon.smithy.rulesengine.language.eval.Type;
+import software.amazon.smithy.rulesengine.language.syntax.rule.Condition;
+
+public class TypeIntrospectionTest {
+    @Test
+    void introspectCorrectTypesForFunctions() {
+        EndpointRuleSet actual = RulesetTestUtil.loadRuleSet(
+                "software/amazon/smithy/rulesengine/testutil/valid-rules/substring.json");
+        List<Condition> conditions = actual.getRules().get(0).getConditions();
+        // stringEquals({TestCaseId}, 1)
+        assertEquals(conditions.get(0).getFn().type(), Type.bool());
+
+        // output = substring({Input}, ...);
+        assertEquals(conditions.get(1).getFn().type(), Type.optional(Type.string()));
+    }
+
+    @Test
+    void introspectCorrectTypesForGetAttr() {
+        EndpointRuleSet actual = RulesetTestUtil.loadRuleSet(
+                "software/amazon/smithy/rulesengine/testutil/valid-rules/get-attr-type-inference.json");
+        // bucketArn.resourceId[2]
+        assertEquals(actual.getRules().get(0).getConditions().get(2).getFn().type(), Type.optional(Type.string()));
+    }
+}
diff --git a/...engine/src/test/java/software/amazon/smithy/rulesengine/language/eval/RuleEngineTest.java b/...engine/src/test/java/software/amazon/smithy/rulesengine/language/eval/RuleEngineTest.java
@@ -23,23 +23,16 @@
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.rulesengine.RulesetTestUtil;
 import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
 import software.amazon.smithy.rulesengine.language.syntax.Identifier;
 import software.amazon.smithy.utils.MapUtils;
 
 class RuleEngineTest {
 
-    private EndpointRuleSet parse(String resource) {
-        InputStream is = getClass().getClassLoader().getResourceAsStream(resource);
-        assert is != null;
-        Node node = ObjectNode.parse(is);
-        return EndpointRuleSet.fromNode(node);
-    }
-
     @Test
     void testRuleEval() {
-        EndpointRuleSet actual = parse(
-                "software/amazon/smithy/rulesengine/testutil/valid-rules/minimal-ruleset.json");
+        EndpointRuleSet actual = RulesetTestUtil.minimalRuleSet();
         Value result = RuleEvaluator.evaluate(actual, MapUtils.of(Identifier.of("Region"),
                 Value.string("us-east-1")));
         Value.Endpoint expected = new Value.Endpoint.Builder(SourceLocation.none())