Repository: smithy-lang/smithy
Pull request #2098 (2024-01-13 05:21:39)

Add "critical" validation phase to validation
Runs "critical" validators before other validators so that they can be written without needing to account for things like whether traits are applied in valid places or not or if a trait's value matches its definition and constraints.

This can help to simplify validators as they can now call methods like .expectShape on a Model to throw if a shape is missing rather than having to defensively code around potentially incorrect models.

To ensure this works backward compatibly with existing errorfile based test runners, we will now detect whether to use a "legacy" validation mode that continues to validate after a critical validator emitted an event because an errorfiles mixes critical and non-critical events. This ensures backward compatibility and that any new addition of critical validators in the future will not break existing test cases.


Patch:
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java
index bff196c1d..93ce65a61 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java
@@ -574,10 +574,11 @@ public final class ModelAssembler {
 
         try {
             List<ValidationEvent> mergedEvents = ModelValidator.builder()
-                    .validators(validators)
+                    .addValidators(validators)
                     .validatorFactory(validatorFactory, decorator)
                     .eventListener(validationEventListener)
                     .includeEvents(events)
+                    .legacyValidationMode((boolean) properties.getOrDefault("LEGACY_VALIDATION_MODE", false))
                     .build()
                     .validate(transformed);
             return new ValidatedResult<>(transformed, mergedEvents);
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java
index 85edc2c20..9cc9d26be 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java
@@ -19,24 +19,26 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.validation.Severity;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationEventDecorator;
+import software.amazon.smithy.model.validation.ValidationUtils;
 import software.amazon.smithy.model.validation.Validator;
 import software.amazon.smithy.model.validation.ValidatorFactory;
 import software.amazon.smithy.model.validation.suppressions.ModelBasedEventDecorator;
-import software.amazon.smithy.model.validation.suppressions.SeverityOverride;
 import software.amazon.smithy.model.validation.validators.ResourceCycleValidator;
 import software.amazon.smithy.model.validation.validators.TargetValidator;
+import software.amazon.smithy.utils.BuilderRef;
 import software.amazon.smithy.utils.ListUtils;
-import software.amazon.smithy.utils.SetUtils;
+import software.amazon.smithy.utils.MapUtils;
 import software.amazon.smithy.utils.SmithyBuilder;
 
 /**
@@ -68,24 +70,27 @@ final class ModelValidator implements Validator {
     }
 
     /** 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 static final Map<Class<?>, Validator> CORRECTNESS_VALIDATORS = MapUtils.of(
+            TargetValidator.class, new TargetValidator(),
+            ResourceCycleValidator.class, new ResourceCycleValidator()
     );
 
     private final ValidatorFactory validatorFactory;
-    private final List<ValidationEvent> events = new ArrayList<>();
-    private final List<Validator> validators = new ArrayList<>();
-    private final List<SeverityOverride> severityOverrides = new ArrayList<>();
+    private final List<ValidationEvent> events;
+    private final List<Validator> validators;
+    private final List<Validator> criticalValidators;
     private final ValidationEventDecorator validationEventDecorator;
     private final Consumer<ValidationEvent> eventListener;
+    private final boolean legacyValidationMode;
 
     ModelValidator(Builder builder) {
         this.validatorFactory = builder.validatorFactory;
         this.eventListener = builder.eventListener;
         this.validationEventDecorator = builder.validationEventDecorator;
-        this.events.addAll(builder.includeEvents);
-        this.validators.addAll(builder.validators);
+        this.events = builder.includeEvents.copy();
+        this.validators = builder.validators.copy();
+        this.criticalValidators = builder.criticalValidators.copy();
+        this.legacyValidationMode = builder.legacyValidationMode;
     }
 
     @Override
@@ -103,34 +108,43 @@ final class ModelValidator implements Validator {
 
     static final class Builder implements SmithyBuilder<ModelValidator> {
 
-        private final List<Validator> validators = new ArrayList<>();
-        private final List<ValidationEvent> includeEvents = new ArrayList<>();
+        private final BuilderRef<List<Validator>> validators = BuilderRef.forList();
+        private final BuilderRef<List<Validator>> criticalValidators = BuilderRef.forList();
+        private final BuilderRef<List<ValidationEvent>> includeEvents = BuilderRef.forList();
         private ValidatorFactory validatorFactory = LazyValidatorFactoryHolder.INSTANCE;
         private Consumer<ValidationEvent> eventListener = event -> { };
         private ValidationEventDecorator validationEventDecorator;
+        private boolean legacyValidationMode = false;
 
         private Builder() {}
 
         /**
-         * Sets the custom {@link Validator}s to use when running the ModelValidator.
+         * Adds an array of {@link Validator}s to use when running the ModelValidator.
          *
-         * @param validators Validators to set.
-         * @return Returns the ModelValidator.
+         * @param validators Validators to add.
+         * @return Returns the builder.
          */
-        public Builder validators(Collection<? extends Validator> validators) {
-            this.validators.clear();
-            validators.forEach(this::addValidator);
+        public Builder addValidators(Collection<? extends Validator> validators) {
+            for (Validator validator : validators) {
+                addValidator(validator);
+            }
             return this;
         }
 
         /**
-         * Adds a custom {@link Validator}.
+         * Adds a {@link Validator}.
          *
          * @param validator Validator to add.
          * @return Returns the builder.
          */
         public Builder addValidator(Validator validator) {
-            validators.add(Objects.requireNonNull(validator));
+            if (!CORRECTNESS_VALIDATORS.containsKey(validator.getClass())) {
+                if (ValidationUtils.isCriticalValidator(validator.getClass())) {
+                    criticalValidators.get().add(validator);
+                } else {
+                    validators.get().add(validator);
+                }
+            }
             return this;
         }
 
@@ -165,21 +179,31 @@ final class ModelValidator implements Validator {
          * Includes a set of events that were already encountered in the result.
          *
          * <p>Suppressions and severity overrides will be applied to the given {@code events}. However, the validator
-         * assumes that the event has already been decroated and the event listener has already seen the event.
+         * assumes that the event has already been decorated and the event listener has already seen the event.
          *
          * @param events Events to include.
          * @return Returns the builder.
          */
         public Builder includeEvents(List<ValidationEvent> events) {
-            this.includeEvents.clear();
-            this.includeEvents.addAll(events);
+            this.includeEvents.get().addAll(events);
+            return this;
+        }
+
+        /**
+         * Enables legacy validation mode that does not fail if critical Validators emit an ERROR.
+         *
+         * @param legacyValidationMode Set to true to enable legacy validation mode.
+         * @return Returns the builder.
+         */
+        public Builder legacyValidationMode(boolean legacyValidationMode) {
+            this.legacyValidationMode = legacyValidationMode;
             return this;
         }
 
         @Override
         public ModelValidator build() {
-            validators.addAll(validatorFactory.loadBuiltinValidators());
-            validators.removeIf(v -> CORE_VALIDATORS.contains(v.getClass()));
+            // Adding built-in validators is deferred to allow for a custom factory to be set on the builder.
+            addValidators(validatorFactory.loadBuiltinValidators());
             return new ModelValidator(this);
         }
     }
@@ -187,22 +211,23 @@ final class ModelValidator implements Validator {
     private static final class LoadedModelValidator {
 
         private final Model model;
-        private final List<SeverityOverride> severityOverrides;
         private final List<Validator> validators;
+        private final List<Validator> criticalValidators;
         private final List<ValidationEvent> events = new ArrayList<>();
         private final ValidationEventDecorator validationEventDecorator;
         private final Consumer<ValidationEvent> eventListener;
+        private final boolean legacyValidationMode;
 
         private LoadedModelValidator(Model model, ModelValidator validator) {
             this.model = model;
             this.eventListener = validator.eventListener;
-            this.severityOverrides = new ArrayList<>(validator.severityOverrides);
             this.validators = new ArrayList<>(validator.validators);
+            this.criticalValidators = Collections.unmodifiableList(validator.criticalValidators);
+            this.legacyValidationMode = validator.legacyValidationMode;
 
             // Suppressing and elevating events is handled by composing a given decorator with a
             // ModelBasedEventDecorator.
             ModelBasedEventDecorator modelBasedEventDecorator = new ModelBasedEventDecorator();
-            modelBasedEventDecorator.severityOverrides(validator.severityOverrides);
             ValidatedResult<ValidationEventDecorator> result = modelBasedEventDecorator.createDecorator(model);
             this.validationEventDecorator = result.getResult()
                     .map(decorator -> ValidationEventDecorator.compose(
@@ -265,35 +290,42 @@ final class ModelValidator implements Validator {
         }
 
         private void pushEvent(ValidationEvent event) {
+            events.add(updateAndEmitEvent(event));
+        }
+
+        private ValidationEvent updateAndEmitEvent(ValidationEvent event) {
             if (validationEventDecorator.canDecorate(event)) {
                 event = validationEventDecorator.decorate(event);
             }
-            events.add(event);
             eventListener.accept(event);
+            return event;
         }
 
         private List<ValidationEvent> validate() {
-            // Perform critical validation before other more granular semantic validators.
-            pushEvents(new TargetValidator().validate(model));
-            pushEvents(new ResourceCycleValidator().validate(model));
-
-            // Fail early if errors were detected since further validation will just obscure the root cause.
+            // Perform critical correctness validation before other critical validators.
+            events.addAll(streamEvents(CORRECTNESS_VALIDATORS.values().stream()));
             if (LoaderUtils.containsErrorEvents(events)) {
                 return events;
             }
 
-            List<ValidationEvent> result = validators.parallelStream()
+            // Same thing, but for other critical validators.
+            events.addAll(streamEvents(criticalValidators.parallelStream()));
+
+            // Only fail early here if legacy validation mode is enabled.
+            if (!legacyValidationMode && LoaderUtils.containsErrorEvents(events)) {
+                return events;
+            }
+
+            events.addAll(streamEvents(validators.parallelStream()));
+            return events;
+        }
+
+        private List<ValidationEvent> streamEvents(Stream<Validator> validators) {
+            return validators
                     .flatMap(validator -> validator.validate(model).stream())
                     .filter(this::filterPrelude)
-                    .map(e -> validationEventDecorator.canDecorate(e) ? validationEventDecorator.decorate(e) : e)
-                    // Emit events as they occur during validation.
-                    .peek(eventListener)
+                    .map(this::updateAndEmitEvent)
                     .collect(Collectors.toList());
-
-            // Add in events encountered while building up validators and suppressions.
-            result.addAll(events);
-
-            return result;
         }
 
         private boolean filterPrelude(ValidationEvent event) {
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationUtils.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationUtils.java
index 7027aee32..eba4a744a 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationUtils.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/ValidationUtils.java
@@ -26,21 +26,96 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
 import java.util.StringJoiner;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.shapes.ToShapeId;
+import software.amazon.smithy.model.validation.validators.DefaultTraitValidator;
+import software.amazon.smithy.model.validation.validators.EnumShapeValidator;
+import software.amazon.smithy.model.validation.validators.ExclusiveStructureMemberTraitValidator;
+import software.amazon.smithy.model.validation.validators.ResourceCycleValidator;
+import software.amazon.smithy.model.validation.validators.ShapeIdConflictValidator;
+import software.amazon.smithy.model.validation.validators.SingleOperationBindingValidator;
+import software.amazon.smithy.model.validation.validators.SingleResourceBindingValidator;
+import software.amazon.smithy.model.validation.validators.TargetValidator;
+import software.amazon.smithy.model.validation.validators.TraitConflictValidator;
+import software.amazon.smithy.model.validation.validators.TraitTargetValidator;
+import software.amazon.smithy.model.validation.validators.TraitValueValidator;
+import software.amazon.smithy.utils.SetUtils;
+import software.amazon.smithy.utils.SmithyInternalApi;
 
 /**
  * Utility methods used when validating.
  */
 public final class ValidationUtils {
+
+    // Critical validators must emit validation events with names that match their class name (Name in NameValidator).
+    private static final Set<Class<? extends Validator>> CRITICAL_VALIDATORS = SetUtils.of(
+            // These validators should run before even any other validators. These validators are considered
+            // correctness validators that ensure relationships are valid and to shapes that can be found and that
+            // the model has no cycles.
+            TargetValidator.class,
+            ResourceCycleValidator.class,
+
+            // This validator is considered critical because it ensures an enum shape itself is valid.
+            EnumShapeValidator.class,
+
+            // Ensure that traits are valid, according to the model, before other validators are run.
+            ExclusiveStructureMemberTraitValidator.class,
+            TraitTargetValidator.class,
+            TraitValueValidator.class,
+            TraitConflictValidator.class,
+
+            // Defaults need to be valid.
+            DefaultTraitValidator.class,
+
+            // Ensure the semantic model adheres to the requirements of shape IDs and service shape uniqueness.
+            ShapeIdConflictValidator.class,
+            SingleOperationBindingValidator.class,
+            SingleResourceBindingValidator.class
+    );
+
     private static final Pattern CAMEL_WORD_SPLITTER = Pattern.compile("(?<!(^|[A-Z]))(?=[A-Z])|(?<!^)(?=[A-Z][a-z])");
 
     private ValidationUtils() {}
 
+    /**
+     * Checks if the given validator class is a critical validator that should be applied before other downstream
+     * validators are applied.
+     *
+     * @param validator Validator class to check.
+     * @return Returns true if the given validator is considered a critical validator.
+     */
+    @SmithyInternalApi
+    public static boolean isCriticalValidator(Class<? extends Validator> validator) {
+        return CRITICAL_VALIDATORS.contains(validator);
+    }
+
+    /**
+     * Checks if the given validation event was emitted by a critical validator.
+     *
+     * @param eventId Event ID to check if it's a critical validation event.
+     * @return Returns true if the given event was emitted from a critical validator.
+     */
+    @SmithyInternalApi
+    public static boolean isCriticalEvent(String eventId) {
+        int slice = eventId.indexOf(".");
+        if (slice > 0) {
+            eventId = eventId.substring(0, slice);
+        }
+
+        for (Class<?> c : CRITICAL_VALIDATORS) {
+            if (c.getSimpleName().replace("Validator", "").equals(eventId)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Splits a camelCase word into a list of words.
      *


Test patch:
diff --git a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.errors b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.errors
new file mode 100644
index 000000000..b39f0ed06
--- /dev/null
+++ b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.errors
@@ -0,0 +1,1 @@
+[ERROR] ns.foo#SomeService: Each `scheme` of the `aws.apigateway#authorizers` trait must target one of the auth schemes applied to the service (i.e., [`aws.auth#sigv4`]). The following mappings of authorizer names to schemes are invalid: ns.foo#invalidAuth -> ns.foo#invalidAuth | AuthorizersTrait
diff --git a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.json b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.json
new file mode 100644
index 000000000..acb6008db
--- /dev/null
+++ b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizer-target.json
@@ -0,0 +1,26 @@
+{
+    "smithy": "2.0",
+    "shapes": {
+        "ns.foo#SomeService": {
+            "type": "service",
+            "version": "2018-03-17",
+            "traits": {
+                "aws.auth#sigv4": {
+                    "name": "someservice"
+                },
+                "aws.apigateway#authorizers": {
+                    "ns.foo#invalidAuth": {
+                        "scheme": "ns.foo#invalidAuth"
+                    }
+                }
+            }
+        },
+        "ns.foo#invalidAuth": {
+            "type": "structure",
+            "traits": {
+                "smithy.api#trait": {},
+                "smithy.api#authDefinition": {}
+            }
+        }
+    }
+}
diff --git a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.errors b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.errors
index 05579a2d5..09b98aed9 100644
--- a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.errors
+++ b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.errors
@@ -1,2 +1,1 @@
-[ERROR] ns.foo#SomeService: Each `scheme` of the `aws.apigateway#authorizers` trait must target one of the auth schemes applied to the service (i.e., [`aws.auth#sigv4`]). The following mappings of authorizer names to schemes are invalid: another -> smithy.api#httpBasicAuth, invalid -> ns.foo#invalid | AuthorizersTrait
-[ERROR] ns.foo#SomeService: Error validating trait `aws.apigateway#authorizers`.invalid.scheme: The scheme of an authorizer definition must reference an auth trait | TraitValue
+[ERROR] ns.foo#SomeService: Each `scheme` of the `aws.apigateway#authorizers` trait must target one of the auth schemes applied to the service (i.e., [`aws.auth#sigv4`]). The following mappings of authorizer names to schemes are invalid: another -> smithy.api#httpBasicAuth | AuthorizersTrait
diff --git a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.json b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.json
index e3debf2e2..844630ffa 100644
--- a/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.json
+++ b/smithy-aws-apigateway-traits/src/test/resources/software/amazon/smithy/aws/apigateway/traits/errorfiles/invalid-authorizers.json
@@ -9,9 +9,6 @@
                     "name": "someservice"
                 },
                 "aws.apigateway#authorizers": {
-                    "invalid": {
-                        "scheme": "ns.foo#invalid"
-                    },
                     "another": {
                         "scheme": "smithy.api#httpBasicAuth",
                         "type": "token",
diff --git a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java
index efb9551a8..b3f2fcf32 100644
--- a/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java
+++ b/smithy-cli/src/test/java/software/amazon/smithy/cli/commands/ValidateCommandTest.java
@@ -87,7 +87,7 @@ public class ValidateCommandTest {
         assertThat(result, containsString("EmitNotes"));
         assertThat(result, containsString("EmitWarnings"));
         assertThat(result, containsString("EmitDangers"));
-        assertThat(result, containsString("TraitTarget"));
+        assertThat(result, containsString("HttpLabelTrait"));
     }
 
     @Test
@@ -99,7 +99,7 @@ public class ValidateCommandTest {
         assertThat(result, containsString("─ EmitNotes"));
         assertThat(result, containsString("─ EmitWarnings"));
         assertThat(result, containsString("─ EmitDangers"));
-        assertThat(result, containsString("─ TraitTarget"));
+        assertThat(result, containsString("─ HttpLabelTrait"));
     }
 
     @Test
@@ -111,7 +111,7 @@ public class ValidateCommandTest {
         assertThat(result, not(containsString("EmitNotes")));
         assertThat(result, containsString("EmitWarnings"));
         assertThat(result, containsString("EmitDangers"));
-        assertThat(result, containsString("TraitTarget"));
+        assertThat(result, containsString("HttpLabelTrait"));
     }
 
     @Test
@@ -123,7 +123,7 @@ public class ValidateCommandTest {
         assertThat(result, not(containsString("EmitNotes")));
         assertThat(result, not(containsString("EmitWarnings")));
         assertThat(result, containsString("EmitDangers"));
-        assertThat(result, containsString("TraitTarget"));
+        assertThat(result, containsString("HttpLabelTrait"));
     }
 
     @Test
@@ -135,7 +135,7 @@ public class ValidateCommandTest {
         assertThat(result, not(containsString("EmitNotes")));
         assertThat(result, not(containsString("EmitWarnings")));
         assertThat(result, not(containsString("EmitDangers")));
-        assertThat(result, containsString("TraitTarget"));
+        assertThat(result, containsString("HttpLabelTrait"));
     }
 
     private CliUtils.Result runValidationEventsTest(Severity severity) throws Exception {
diff --git a/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy
index 8060a6231..b159b690f 100644
--- a/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy
+++ b/smithy-cli/src/test/resources/software/amazon/smithy/cli/commands/validation-events.smithy
@@ -46,5 +46,7 @@ string Warning
 
 string Danger
 
-@required // this trait is invalid and causes an error.
-string Error
+// The uri will trigger an ERROR.
+@http(method: "GET", uri: "/hi/{missingLabel}")
+@readonly
+operation Error {}
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestSuite.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestSuite.java
index 943c4d4e1..70a8e4716 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestSuite.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestSuite.java
@@ -25,21 +25,30 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.Future;
 import java.util.function.Supplier;
+import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.loader.ModelAssembler;
+import software.amazon.smithy.model.validation.Severity;
+import software.amazon.smithy.model.validation.ValidationEvent;
+import software.amazon.smithy.model.validation.ValidationUtils;
+import software.amazon.smithy.model.validation.Validator;
 
 /**
  * Runs test cases against a directory of models and error files.
  */
 public final class SmithyTestSuite {
+
+    private static final Logger LOGGER = Logger.getLogger(SmithyTestSuite.class.getName());
     private static final String DEFAULT_TEST_CASE_LOCATION = "errorfiles";
 
     private final List<SmithyTestCase> cases = new ArrayList<>();
@@ -247,13 +256,60 @@ public final class SmithyTestSuite {
     }
 
     private Callable<SmithyTestCase.Result> createTestCaseCallable(SmithyTestCase testCase) {
+        boolean useLegacyValidationMode = isLegacyValidationRequired(testCase);
         return () -> {
             ModelAssembler assembler = modelAssemblerFactory.get();
             assembler.addImport(testCase.getModelLocation());
+            if (useLegacyValidationMode) {
+                assembler.putProperty("LEGACY_VALIDATION_MODE", true);
+            }
             return testCase.createResult(assembler.assemble());
         };
     }
 
+    // We introduced the concept of "critical" validation events after many errorfiles were created that relied
+    // on all validators being run, including validators now considered critical that prevent further validation.
+    // If we didn't account for that here, the addition of "critical" validators would have been a breaking change.
+    // To make it backward compatible, we preemptively detect if the errorfiles contains both critical and
+    // non-critical validation event assertions, and if so, we run validation using an internal-only validation
+    // mode that doesn't fail after critical validators report errors.
+    private boolean isLegacyValidationRequired(SmithyTestCase testCase) {
+        Set<String> criticalEvents = new TreeSet<>();
+        Set<String> nonCriticalEvents = new TreeSet<>();
+
+        for (ValidationEvent event : testCase.getExpectedEvents()) {
+            if (isCriticalValidationEvent(event)) {
+                criticalEvents.add(event.getId());
+            } else {
+                nonCriticalEvents.add(event.getId());
+            }
+        }
+
+        if (!criticalEvents.isEmpty() && !nonCriticalEvents.isEmpty()) {
+            LOGGER.warning(String.format("Test suite `%s` relies on the emission of non-critical validation events "
+                                         + "after critical validation events were emitted. This test case should be "
+                                         + "refactored so that critical validation events are tested using separate "
+                                         + "test cases from non-critical events. Critical events: %s. Non-critical "
+                                         + "events: %s",
+                                         testCase.getModelLocation(),
+                                         criticalEvents,
+                                         nonCriticalEvents));
+            return true;
+        }
+
+        return false;
+    }
+
+    private static boolean isCriticalValidationEvent(ValidationEvent event) {
+        if (ValidationUtils.isCriticalEvent(event.getId())) {
+            return true;
+        }
+
+        // In addition to the method to the check based on Validator classes, MODEL validation event IDs marked as
+        // ERROR that go along with non-critical events requires legacy validation.
+        return event.getId().equals(Validator.MODEL_ERROR) && event.getSeverity() == Severity.ERROR;
+    }
+
     /**
      * Executes the test runner.
      *