Improve EndpointTests built-in validation

This commit adds new validation for built-in parameters when used
with the `endpointTests` trait.

1. Validates that built-in values with defaults are specified to the
   correct value if they are mismatched in `operationInput` cases.
2. Validates that built-in values without defaults are specified
   in `operationInput` cases.

Also fixes skipping DANGER/ERROR in endpoint test validation and
adds a convenience method to the `Parameter` class.

Author: kstich

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/language/syntax/parameters/Parameter.java

@@ -118,6 +118,15 @@ public Identifier getName() {
         return name;
     }
 
+    /**
+     * Gets the parameter name.
+     *
+     * @return returns the parameter name as a {@link String}.
+     */
+    public String getNameString() {
+        return name.toString();
+    }
+
     /**
      * Gets the parameter in template form.
      *

smithy-rules-engine/src/main/java/software/amazon/smithy/rulesengine/traits/EndpointTestsTraitValidator.java

@@ -10,13 +10,16 @@
 import java.util.Map;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.TopDownIndex;
+import software.amazon.smithy.model.node.Node;
 import software.amazon.smithy.model.shapes.OperationShape;
 import software.amazon.smithy.model.shapes.ServiceShape;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.NodeValidationVisitor;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidationEvent;
+import software.amazon.smithy.rulesengine.language.EndpointRuleSet;
+import software.amazon.smithy.rulesengine.language.syntax.parameters.Parameter;
 import software.amazon.smithy.utils.SmithyUnstableApi;
 
 /**
@@ -36,7 +39,28 @@ public List<ValidationEvent> validate(Model model) {
                 operationNameMap.put(operationShape.getId().getName(), operationShape);
             }
 
+            // Precompute the built-ins and their default states, as this will
+            // be used frequently in downstream validation.
+            List<Parameter> builtInParamsWithDefaults = new ArrayList<>();
+            List<Parameter> builtInParamsWithoutDefaults = new ArrayList<>();
+            EndpointRuleSet ruleSet = serviceShape.expectTrait(EndpointRuleSetTrait.class).getEndpointRuleSet();
+            for (Parameter parameter : ruleSet.getParameters()) {
+                if (parameter.isBuiltIn()) {
+                    if (parameter.getDefault().isPresent()) {
+                        builtInParamsWithDefaults.add(parameter);
+                    } else {
+                        builtInParamsWithoutDefaults.add(parameter);
+                    }
+                }
+            }
+
             for (EndpointTestCase testCase : serviceShape.expectTrait(EndpointTestsTrait.class).getTestCases()) {
+                // If values for built-in parameters don't match the default, they MUST
+                // be specified in the operation inputs. Precompute the ones that don't match
+                // and capture their value.
+                Map<Parameter, Node> mismatchedBuiltInParams =
+                        getMismatchedBuiltInParams(builtInParamsWithDefaults, testCase);
+
                 for (EndpointTestOperationInput testOperationInput : testCase.getOperationInputs()) {
                     String operationName = testOperationInput.getOperationName();
 
@@ -47,42 +71,102 @@ public List<ValidationEvent> validate(Model model) {
                                 String.format("Test case operation `%s` does not exist in service `%s`",
                                         operationName,
                                         serviceShape.getId())));
+                        continue;
                     }
 
                     // Still emit events if the operation exists, but was just not bound.
-                    if (operationNameMap.containsKey(operationName)) {
-                        StructureShape inputShape = model.expectShape(
-                                operationNameMap.get(operationName).getInputShape(),
-                                StructureShape.class);
-
-                        List<ValidationEvent> operationInputEvents = validateOperationInput(model,
-                                serviceShape,
-                                inputShape,
-                                testOperationInput);
-
-                        // Error test cases may use invalid inputs as the mechanism to trigger their error,
-                        // so lower the severity before emitting. All other events here should be raised to
-                        // DANGER level as well.
-                        for (ValidationEvent event : operationInputEvents) {
-                            if (event.getSeverity() == Severity.WARNING
-                                    || event.getSeverity() == Severity.NOTE
-                                    || testCase.getExpect().getError().isPresent()) {
-                                events.add(event.toBuilder().severity(Severity.DANGER).build());
-                            }
-                        }
-                    }
+                    validateBuiltInValuesAreValid(serviceShape,
+                            mismatchedBuiltInParams,
+                            testOperationInput,
+                            events);
+                    validateNoDefaultBuiltInParamsHaveValues(serviceShape,
+                            builtInParamsWithoutDefaults,
+                            testCase,
+                            testOperationInput,
+                            events);
+
+                    StructureShape inputShape = model.expectShape(
+                            operationNameMap.get(operationName).getInputShape(),
+                            StructureShape.class);
+                    validateOperationInput(model, serviceShape, inputShape, testCase, testOperationInput, events);
                 }
             }
         }
 
         return events;
     }
 
-    private List<ValidationEvent> validateOperationInput(
+    private Map<Parameter, Node> getMismatchedBuiltInParams(
+            List<Parameter> builtInParamsWithDefaults,
+            EndpointTestCase testCase
+    ) {
+        Map<Parameter, Node> mismatchedBuiltInParams = new HashMap<>();
+        for (Parameter parameter : builtInParamsWithDefaults) {
+            String parameterName = parameter.getName().toString();
+            Node defaultValue = parameter.getDefault().get().toNode();
+
+            // Consider a parameter mismatched if the built-in's default and its
+            // value in this test case aren't the same.
+            if (testCase.getParams().containsMember(parameterName)
+                    && !testCase.getParams().expectMember(parameterName).equals(defaultValue)) {
+                mismatchedBuiltInParams.put(parameter, testCase.getParams().expectMember(parameterName));
+            }
+        }
+        return mismatchedBuiltInParams;
+    }
+
+    private void validateBuiltInValuesAreValid(
+            ServiceShape serviceShape,
+            Map<Parameter, Node> mismatchedBuiltInParams,
+            EndpointTestOperationInput testOperationInput,
+            List<ValidationEvent> events
+    ) {
+        for (Map.Entry<Parameter, Node> mismatchedBuiltInParam : mismatchedBuiltInParams.entrySet()) {
+            String builtInName = mismatchedBuiltInParam.getKey().getBuiltIn().get();
+            // Emit if either the built-in with a mismatched value isn't
+            // specified or the value set for it doesn't match.
+            if (!testOperationInput.getBuiltInParams().containsMember(builtInName)
+                    || !testOperationInput.getBuiltInParams()
+                            .expectMember(builtInName)
+                            .equals(mismatchedBuiltInParam.getValue())) {
+                events.add(error(serviceShape,
+                        testOperationInput,
+                        String.format("Test case does not supply the `%s` value for the `%s` parameter's "
+                                + "`%s` built-in.",
+                                Node.printJson(mismatchedBuiltInParam.getValue()),
+                                mismatchedBuiltInParam.getKey().getNameString(),
+                                mismatchedBuiltInParam.getKey().getBuiltIn().get())));
+            }
+        }
+    }
+
+    private void validateNoDefaultBuiltInParamsHaveValues(
+            ServiceShape serviceShape,
+            List<Parameter> builtInParamsWithoutDefaults,
+            EndpointTestCase testCase,
+            EndpointTestOperationInput testOperationInput,
+            List<ValidationEvent> events
+    ) {
+        for (Parameter parameter : builtInParamsWithoutDefaults) {
+            if (testCase.getParams().containsMember(parameter.getNameString())
+                    && !testOperationInput.getBuiltInParams().containsMember(parameter.getBuiltIn().get())) {
+                events.add(error(serviceShape,
+                        testOperationInput,
+                        String.format("Operation input does not supply a value for the `%s` built-in parameter "
+                                + "and the `%s` parameter does not set a default.",
+                                parameter.getBuiltIn().get(),
+                                parameter.getName())));
+            }
+        }
+    }
+
+    private void validateOperationInput(
             Model model,
             ServiceShape serviceShape,
             StructureShape inputShape,
-            EndpointTestOperationInput testOperationInput
+            EndpointTestCase testCase,
+            EndpointTestOperationInput testOperationInput,
+            List<ValidationEvent> events
     ) {
         NodeValidationVisitor validator = NodeValidationVisitor.builder()
                 .model(model)
@@ -92,6 +176,18 @@ private List<ValidationEvent> validateOperationInput(
                 .startingContext("The operationInput value for an endpoint test "
                         + "does not match the operation's input shape")
                 .build();
-        return inputShape.accept(validator);
+
+        // Error test cases may use invalid inputs as the mechanism to trigger their error,
+        // so lower the severity before emitting. All other events here should be raised to
+        // DANGER level as well.
+        for (ValidationEvent event : inputShape.accept(validator)) {
+            if (event.getSeverity() == Severity.WARNING
+                    || event.getSeverity() == Severity.NOTE
+                    || testCase.getExpect().getError().isPresent()) {
+                events.add(event.toBuilder().severity(Severity.DANGER).build());
+            } else {
+                events.add(event);
+            }
+        }
     }
 }

smithy-rules-engine/src/test/java/software/amazon/smithy/rulesengine/traits/EndpointTestsTest.java

@@ -31,7 +31,7 @@ public void loadsFromTheModel() {
         EndpointTestsTrait ruleSetTrait = serviceShape.getTrait(EndpointTestsTrait.class).get();
         List<EndpointTestCase> testCases = ruleSetTrait.getTestCases();
 
-        assertThat(2, equalTo(testCases.size()));
+        assertThat(3, equalTo(testCases.size()));
         assertThat(EndpointTestCase.builder()
                 .documentation("a documentation string")
                 .params(ObjectNode.builder()

smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-test-missing-built-in-value.errors

@@ -0,0 +1,3 @@
+[WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait
+[WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait
+[ERROR] smithy.example#ExampleService: Operation input does not supply a value for the `SDK::Endpoint` built-in parameter and the `endpoint` parameter does not set a default. | EndpointTestsTrait

smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/invalid-test-missing-built-in-value.smithy

@@ -0,0 +1,57 @@
+$version: "2.0"
+
+namespace smithy.example
+
+use smithy.rules#endpointRuleSet
+use smithy.rules#endpointTests
+
+@endpointRuleSet({
+    version: "1.0"
+    parameters: {
+        endpoint: {
+            type: "string"
+            builtIn: "SDK::Endpoint"
+            documentation: "docs"
+        }
+    }
+    rules: [
+        {
+            documentation: "Passthrough"
+            conditions: []
+            endpoint: {
+                url: "https://example.com"
+            }
+            type: "endpoint"
+        }
+    ]
+})
+@endpointTests({
+    version: "1.0"
+    testCases: [
+        {
+            params: {
+                endpoint: "https://example.com"
+            }
+            operationInputs: [{
+                operationName: "GetThing"
+            }]
+            expect: {
+                endpoint: {
+                    url: "https://example.com"
+                }
+            }
+        }
+    ]
+})
+@suppress(["RuleSetParameter.TestCase.Unused"])
+service ExampleService {
+    version: "2020-07-02"
+    operations: [GetThing]
+}
+
+operation GetThing {
+    input := {
+        fizz: String
+    }
+}
+

smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/mismatched-builtin-values.errors

@@ -0,0 +1,4 @@
+[WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait
+[WARNING] smithy.example#ExampleService: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait
+[ERROR] smithy.example#ExampleService: Test case does not supply the `"https://another.example.com"` value for the `endpoint` parameter's `SDK::Endpoint` built-in. | EndpointTestsTrait
+[ERROR] smithy.example#ExampleService: Test case does not supply the `"https://some.example.com"` value for the `endpoint` parameter's `SDK::Endpoint` built-in. | EndpointTestsTrait

smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/mismatched-builtin-values.smithy

@@ -0,0 +1,92 @@
+$version: "2.0"
+
+namespace smithy.example
+
+use smithy.rules#endpointRuleSet
+use smithy.rules#endpointTests
+
+@endpointRuleSet({
+    version: "1.0"
+    parameters: {
+        endpoint: {
+            type: "string"
+            builtIn: "SDK::Endpoint"
+            documentation: "docs"
+            default: "https://example.com"
+            required: true
+        }
+    }
+    rules: [
+        {
+            documentation: "Passthrough"
+            conditions: []
+            endpoint: {
+                url: "https://example.com"
+            }
+            type: "endpoint"
+        }
+    ]
+})
+@endpointTests({
+    version: "1.0"
+    testCases: [
+        {
+            params: {
+                endpoint: "https://another.example.com"
+            }
+            operationInputs: [{
+                operationName: "GetThing"
+                builtInParams: {
+                    "SDK::Endpoint": "https://custom.example.com"
+                }
+            }]
+            expect: {
+                endpoint: {
+                    url: "https://example.com"
+                }
+            }
+        }
+        {
+            params: {
+                endpoint: "https://some.example.com"
+            }
+            operationInputs: [{
+                operationName: "GetThing"
+                builtInParams: {}
+            }]
+            expect: {
+                endpoint: {
+                    url: "https://example.com"
+                }
+            }
+        }
+        {
+            params: {
+                endpoint: "https://valid.example.com"
+            }
+            operationInputs: [{
+                operationName: "GetThing"
+                builtInParams: {
+                    "SDK::Endpoint": "https://valid.example.com"
+                }
+            }]
+            expect: {
+                endpoint: {
+                    url: "https://example.com"
+                }
+            }
+        }
+    ]
+})
+@suppress(["RuleSetParameter.TestCase.Unused"])
+service ExampleService {
+    version: "2020-07-02"
+    operations: [GetThing]
+}
+
+operation GetThing {
+    input := {
+        fizz: String
+    }
+}
+

smithy-rules-engine/src/test/resources/software/amazon/smithy/rulesengine/traits/errorfiles/missing-input-target.errors

@@ -1,2 +1,3 @@
+[WARNING] smithy.example#InvalidService: This shape applies a trait that is unstable: smithy.rules#endpointRuleSet | UnstableTrait
 [WARNING] smithy.example#InvalidService: This shape applies a trait that is unstable: smithy.rules#endpointTests | UnstableTrait
 [DANGER] smithy.example#InvalidService: The operationInput value for an endpoint test does not match the operation's input shape: Invalid structure member `fizz` found for `smithy.example#GetThingInput` | EndpointTestsTrait.smithy.example#GetThingInput.fizz