Stop validation on critical loading errors

We previously would continue to perform more granular semantic model
validation even after an ERROR was encountered while loading the model.
If an ERROR is encountered while loading a model, then it is very likely
that it can cause a flurry of unrelated validation events to emit errors
that would only obscure the root cause of the issue.

This commit updates model validation to stop if an ERROR occurred while
loading models, if a model has broken shape references, if a shape
target targets an invalid shape, or if a resource hierarchy is
recursive. This update required various "kitchen-sink" style tests to be
updated to account for the new validation behavior.

Closes #743

smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/clientendpointdiscovery/service-not-configured.json

@@ -95,7 +95,7 @@
                 "Id": {
                     "target": "smithy.api#String",
                     "traits": {
-                        "aws.api#endpointDiscoveryId": {},
+                        "aws.api#clientEndpointDiscoveryId": {},
                         "smithy.api#required": {}
                     }
                 }

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/standard-operation-verb-invalid-config.errors

@@ -0,0 +1 @@
+[ERROR] -: Error creating `StandardOperationVerb` validator: Either verbs or suggestAlternatives must be set when configuring StandardOperationVerb | Model

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/standard-operation-verb-invalid-config.json

@@ -0,0 +1,11 @@
+{
+    "smithy": "1.0",
+    "metadata": {
+        "validators": [
+            {
+                "name": "StandardOperationVerb",
+                "id": "Pointless"
+            }
+        ]
+    }
+}

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/standard-operation-verb-validator-test.errors

@@ -12,4 +12,3 @@
 [DANGER] ns.foo#DestroyFoo: Operation shape `DestroyFoo` uses a non-standard verb, `Destroy`. Expected one of the following verbs: `Create`, `Delete`, `Update` | WithVerbsAndPrefixes
 [DANGER] ns.foo#MakeFoo: Operation shape `MakeFoo` uses a non-standard verb, `Make`. Consider using one of the following verbs instead: `Create`, `Generate` | SuggestAlternatives
 [DANGER] ns.foo#MakeFoo: Operation shape `MakeFoo` uses a non-standard verb, `Make`. Expected one of the following verbs: `Create`, `Delete`, `Update` | WithVerbsAndPrefixes
-[ERROR] -: Error creating `StandardOperationVerb` validator: Either verbs or suggestAlternatives must be set when configuring StandardOperationVerb | Model

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/standard-operation-verb-validator-test.json

@@ -43,10 +43,6 @@
     },
     "metadata": {
         "validators": [
-            {
-                "name": "StandardOperationVerb",
-                "id": "Pointless"
-            },
             {
                 "name": "StandardOperationVerb",
                 "id": "WithVerbsAndPrefixes",

smithy-model/src/main/java/software/amazon/smithy/model/loader/LoaderUtils.java

@@ -95,4 +95,19 @@ static boolean isSameLocation(FromSourceLocation a, FromSourceLocation b) {
         SourceLocation sb = b.getSourceLocation();
         return sa != SourceLocation.NONE && sa.equals(sb);
     }
+
+    /**
+     * Checks if a list of validation events contains an ERROR severity.
+     *
+     * @param events Events to check.
+     * @return Returns true if an ERROR event is present.
+     */
+    static boolean containsErrorEvents(List<ValidationEvent> events) {
+        for (ValidationEvent event : events) {
+            if (event.getSeverity() == Severity.ERROR) {
+                return true;
+            }
+        }
+        return false;
+    }
 }

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java

@@ -600,7 +600,9 @@ private List<ModelFile> createModelFiles() {
     private ValidatedResult<Model> validate(Model model, TraitContainer traits, List<ValidationEvent> events) {
         validateTraits(model.getShapeIds(), traits, events);
 
-        if (disableValidation) {
+        // If ERROR validation events occur while loading, then performing more
+        // granular semantic validation will only obscure the root cause of errors.
+        if (disableValidation || LoaderUtils.containsErrorEvents(events)) {
             return new ValidatedResult<>(model, events);
         }
 

smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
@@ -33,7 +34,10 @@
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.Validator;
 import software.amazon.smithy.model.validation.ValidatorFactory;
+import software.amazon.smithy.model.validation.validators.ResourceCycleValidator;
+import software.amazon.smithy.model.validation.validators.TargetValidator;
 import software.amazon.smithy.utils.ListUtils;
+import software.amazon.smithy.utils.SetUtils;
 
 /**
  * Validates a model, including validators and suppressions loaded from
@@ -57,6 +61,12 @@ final class ModelValidator {
     private static final String EMPTY_REASON = "";
     private static final Collection<String> SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON);
 
+    /** If these validators fail, then many others will too. Validate these first. */
+    private static final Set<Class<? extends Validator>> CORE_VALIDATORS = SetUtils.of(
+            TargetValidator.class,
+            ResourceCycleValidator.class
+    );
+
     private final List<Validator> validators;
     private final ArrayList<ValidationEvent> events = new ArrayList<>();
     private final ValidatorFactory validatorFactory;
@@ -71,6 +81,7 @@ private ModelValidator(
         this.model = model;
         this.validatorFactory = validatorFactory;
         this.validators = new ArrayList<>(validators);
+        this.validators.removeIf(v -> CORE_VALIDATORS.contains(v.getClass()));
     }
 
     /**
@@ -95,6 +106,15 @@ private List<ValidationEvent> doValidate() {
         List<ValidatorDefinition> assembledValidatorDefinitions = assembleValidatorDefinitions();
         assembleValidators(assembledValidatorDefinitions);
 
+        // Perform critical validation before other more granular semantic validators.
+        // If these validators fail, then many other validators will fail as well,
+        // which will only obscure the root cause.
+        events.addAll(new TargetValidator().validate(model));
+        events.addAll(new ResourceCycleValidator().validate(model));
+        if (LoaderUtils.containsErrorEvents(events)) {
+            return events;
+        }
+
         List<ValidationEvent> result = validators
                 .parallelStream()
                 .flatMap(validator -> validator.validate(model).stream())

smithy-model/src/test/java/software/amazon/smithy/model/knowledge/PaginatedIndexTest.java

@@ -33,7 +33,7 @@ public class PaginatedIndexTest {
     public void findDirectChildren() {
         ValidatedResult<Model> result = Model.assembler()
                 .addImport(getClass().getResource(
-                        "/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json"))
+                        "/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-valid.json"))
                 .assemble();
         Model model = result.getResult().get();
         PaginatedIndex index = PaginatedIndex.of(model);
@@ -63,7 +63,7 @@ public void findDirectChildren() {
     public void findIndirectChildren() {
         ValidatedResult<Model> result = Model.assembler()
                 .addImport(getClass().getResource(
-                        "/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json"))
+                        "/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-valid.json"))
                 .assemble();
         Model model = result.getResult().get();
         PaginatedIndex index = PaginatedIndex.of(model);

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/auth/auth-trait-invalid-shape-id.errors

@@ -0,0 +1 @@
+[ERROR] ns.foo#InvalidShapeId: Error creating trait `auth`: Invalid shape ID: not a shape ID | Model

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/auth/auth-trait-invalid-shape-id.json

@@ -0,0 +1,11 @@
+{
+    "smithy": "1.0",
+    "shapes": {
+        "ns.foo#InvalidShapeId": {
+            "type": "operation",
+            "traits": {
+                "smithy.api#auth": ["not a shape ID"]
+            }
+        }
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/auth/auth-trait-must-target-auth-definitions.errors

@@ -0,0 +1 @@
+[ERROR] ns.foo#Invalid1: Error validating trait `auth`.0: Shape ID `smithy.api#String` does not match selector `[trait|authDefinition]` | TraitValue

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/auth-trait-must-target-auth-definitions.json → smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/auth/auth-trait-must-target-auth-definitions.json

@@ -14,12 +14,6 @@
             }
         },
         "ns.foo#Invalid1": {
-            "type": "operation",
-            "traits": {
-                "smithy.api#auth": ["not a shape ID"]
-            }
-        },
-        "ns.foo#Invalid2": {
             "type": "operation",
             "traits": {
                 "smithy.api#auth": ["smithy.api#String"]

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation/external-documentation-bad-url.errors

@@ -0,0 +1 @@
+[ERROR] ns.foo#Invalid: Error creating trait `externalDocumentation`: Each externalDocumentation value must be a valid URL. Found "invalid!" for name "Homepage" | Model

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation/external-documentation-bad-url.json

@@ -0,0 +1,14 @@
+{
+    "smithy": "1.0",
+    "shapes": {
+        "ns.foo#Invalid": {
+            "type": "service",
+            "version": "2017-01-17",
+            "traits": {
+                "smithy.api#externalDocumentation": {
+                    "Homepage": "invalid!"
+                }
+            }
+        }
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation.errors → smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation/external-documentation-no-entries.errors

@@ -1,2 +1 @@
-[ERROR] ns.foo#Invalid: Error creating trait `externalDocumentation`: Each externalDocumentation value must be a valid URL. Found "invalid!" for name "Homepage" | Model
 [ERROR] ns.foo#Invalid2: Error validating trait `externalDocumentation`: Value provided for `smithy.api#externalDocumentation` must have at least 1 entries, but the provided value only has 0 entries | TraitValue

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation/external-documentation-no-entries.json

@@ -0,0 +1,12 @@
+{
+    "smithy": "1.0",
+    "shapes": {
+        "ns.foo#Invalid2": {
+            "type": "service",
+            "version": "2017-01-17",
+            "traits": {
+                "smithy.api#externalDocumentation": {}
+            }
+        }
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/external-documentation/external-documentation-valid.json

@@ -0,0 +1,14 @@
+{
+    "smithy": "1.0",
+    "shapes": {
+        "ns.foo#Valid": {
+            "type": "service",
+            "version": "2017-01-17",
+            "traits": {
+                "smithy.api#externalDocumentation": {
+                    "Homepage": "https://www.example.com"
+                }
+            }
+        }
+    }
+}

smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-each-invalid-selector.errors

@@ -0,0 +1,30 @@
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near ``: Unexpected selector EOF; expression:  | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near ``: Unexpected selector EOF; expression:  | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "!": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `!`: Unexpected selector character: !; expression: ! | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "'foo'": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `'foo'`: Unexpected selector character: '; expression: 'foo' | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "\"foo\"": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `"foo"`: Unexpected selector character: "; expression: "foo" | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "invalid": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Unknown shape type: invalid; expression: invalid | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 2, near `]`: Expected a valid identifier character, but found ']'; expression: [] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo|]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `]`: Expected a valid identifier character, but found ']'; expression: [foo|] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[|]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 2, near `|]`: Expected a valid identifier character, but found '|'; expression: [|] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[a=]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near `]`: Expected a valid identifier character, but found ']'; expression: [a=] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[a=b": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Expected: ']', but found '[EOF]'; expression: [a=b | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "string=b": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 7, near `=b`: Unexpected selector character: =; expression: string=b | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo=']": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Expected ' to close ]; expression: [foo='] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo=\"]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Expected " to close ]; expression: [foo="] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo==value]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `=value]`: Expected a valid identifier character, but found '='; expression: [foo==value] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo^foo]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `foo]`: Expected: '=', but found 'f'; expression: [foo^foo] | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(:not(string) > list": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 24, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :is(:not(string) > list | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[]->": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ` -[]->`: Unknown shape type: foo; expression: foo -[]-> | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[input]->": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ` -[input]->`: Unknown shape type: foo; expression: foo -[input]-> | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Expected: '(', but found '[EOF]'; expression: :not | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not(": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near ``: Unexpected selector EOF; expression: :not( | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not()": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `)`: Unexpected selector character: ); expression: :not() | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not(string": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 12, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :not(string | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ``: Expected: '(', but found '[EOF]'; expression: :is | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Unexpected selector EOF; expression: :is( | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":nay()": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `)`: Unexpected selector character: ); expression: :nay() | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 11, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :is(string | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, ": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 13, near ``: Unexpected selector EOF; expression: :is(string,  | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, )": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 13, near `)`: Unexpected selector character: ); expression: :is(string, ) | Model
+[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, :not())": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 18, near `))`: Unexpected selector character: ); expression: :is(string, :not()) | Model