Move validation into core

This commit moves model validation and suppressions into the core. EmitEachSelector and EmitNoneSelector are now part of smithy-model and were removed from smithy-linters. The `@suppress` trait was added to suppress validaation events on specific shapes. The `validators` metadata property was updated so that it now only take `id`, `namespace`, and `reason`, where `namespace` can be set to `*` to suppress a validation event for all namespaces or validation events that aren't specific to a single shape. The ability to add custom suppressions to the ModeAssembler has been removed.
backend-souls · Apr 24, 2020 · ba00ac2 · ba00ac2
1 parent 7d4c862
commit ba00ac2
Show file tree

Hide file tree

Showing 35 changed files with 326 additions and 579 deletions.
diff --git a/smithy-aws-protocol-tests/model/awsJson1_1/main.json b/smithy-aws-protocol-tests/model/awsJson1_1/main.json
@@ -1,5 +1,10 @@
 {
     "smithy": "1.0.0",
+    "metadata": {
+        "suppressions": [
+            {"id": "UnreferencedShape", "namespace": "aws.protocoltests.json"}
+        ]
+    },
     "shapes": {
         "aws.protocoltests.json#JsonProtocol": {
             "type": "service",

diff --git a/smithy-aws-protocol-tests/model/shared-types.smithy b/smithy-aws-protocol-tests/model/shared-types.smithy
@@ -8,7 +8,8 @@
 $version: "1.0.0"
 
 metadata suppressions = [{
-    ids: ["DeprecatedTrait"],
+    id: "DeprecatedTrait",
+    namespace: "*",
     reason: """
         Some of the AWS protocols make use of deprecated traits, and some are
         themselves deprecated traits. As this package is intended to test those

diff --git a/...main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService b/...main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService
@@ -1,7 +1,5 @@
 software.amazon.smithy.linters.AbbreviationNameValidator$Provider
 software.amazon.smithy.linters.CamelCaseValidator$Provider
-software.amazon.smithy.linters.EmitEachSelectorValidator$Provider
-software.amazon.smithy.linters.EmitNoneSelectorValidator$Provider
 software.amazon.smithy.linters.InputOutputStructureReuseValidator$Provider
 software.amazon.smithy.linters.MissingPaginatedTraitValidator$Provider
 software.amazon.smithy.linters.ReservedWordsValidator$Provider

diff --git a/...rc/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors b/...rc/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors
diff --git a/.../src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json b/.../src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json
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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
@@ -41,7 +41,6 @@
 import software.amazon.smithy.model.shapes.AbstractShapeBuilder;
 import software.amazon.smithy.model.shapes.Shape;
 import software.amazon.smithy.model.traits.TraitFactory;
-import software.amazon.smithy.model.validation.Suppression;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.Validator;
@@ -110,7 +109,6 @@ public final class ModelAssembler {
     });
 
     private final List<Validator> validators = new ArrayList<>();
-    private final List<Suppression> suppressions = new ArrayList<>();
     private final List<Node> documentNodes = new ArrayList<>();
     private final List<Model> mergeModels = new ArrayList<>();
     private final List<AbstractShapeBuilder<?, ?>> shapes = new ArrayList<>();
@@ -140,7 +138,6 @@ public ModelAssembler copy() {
         assembler.validatorFactory = validatorFactory;
         assembler.inputStreamModels.putAll(inputStreamModels);
         assembler.validators.addAll(validators);
-        assembler.suppressions.addAll(suppressions);
         assembler.documentNodes.addAll(documentNodes);
         assembler.mergeModels.addAll(mergeModels);
         assembler.shapes.addAll(shapes);
@@ -158,7 +155,6 @@ public ModelAssembler copy() {
      *
      * <ul>
      *     <li>Validators registered via {@link #addValidator}</li>
-     *     <li>Suppressions registered via {@link #addSuppression}</li>
      *     <li>Models registered via {@link #addImport}</li>
      *     <li>Models registered via {@link #addDocumentNode}</li>
      *     <li>Models registered via {@link #addUnparsedModel}</li>
@@ -178,7 +174,6 @@ public ModelAssembler reset() {
         mergeModels.clear();
         inputStreamModels.clear();
         validators.clear();
-        suppressions.clear();
         documentNodes.clear();
         disablePrelude = false;
         return this;
@@ -221,17 +216,6 @@ public ModelAssembler addValidator(Validator validator) {
         return this;
     }
 
-    /**
-     * Registers a suppression to be used when validating the model.
-     *
-     * @param suppression Suppression to register.
-     * @return Returns the assembler.
-     */
-    public ModelAssembler addSuppression(Suppression suppression) {
-        suppressions.add(Objects.requireNonNull(suppression));
-        return this;
-    }
-
     /**
      * Adds a string containing an unparsed model to the assembler.
      *
@@ -531,8 +515,7 @@ private ValidatedResult<Model> validate(Model model, List<ValidationEvent> model
         }
 
         // Validate the model based on the explicit validators and model metadata.
-        List<ValidationEvent> events = ModelValidator.validate(model, validatorFactory,
-                assembleValidators(), suppressions);
+        List<ValidationEvent> events = ModelValidator.validate(model, validatorFactory, assembleValidators());
         events.addAll(modelResultEvents);
         return new ValidatedResult<>(model, events);
     }

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
@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
@@ -16,13 +16,19 @@
 package software.amazon.smithy.model.loader;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
+import software.amazon.smithy.model.node.ObjectNode;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.traits.SuppressTrait;
 import software.amazon.smithy.model.validation.Severity;
-import software.amazon.smithy.model.validation.Suppression;
 import software.amazon.smithy.model.validation.ValidatedResult;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.Validator;
@@ -31,33 +37,40 @@
 
 /**
  * Validates a model, including validators and suppressions loaded from
- * metadata.
+ * traits.
  *
  * <p>ModelValidator is used to apply validation to a loaded Model. This
  * class is used in tandem with {@link ModelAssembler}.
  *
- * <p>Validators and suppressions found in the metadata of the validated
- * model are automatically created and applied to the model. Explicitly
- * provided suppressions and validators are merged together with the
- * validators and suppressions loaded from metadata.
+ * <p>Validators found in metadata and suppressions found in traits are
+ * automatically created and applied to the model. Explicitly provided
+ * validators are merged together with the validators and suppressions
+ * loaded from metadata.
  */
 final class ModelValidator {
+
+    private static final String SUPPRESSIONS = "suppressions";
+    private static final String ID = "id";
+    private static final String NAMESPACE = "namespace";
+    private static final String REASON = "reason";
+    private static final String STAR = "*";
+    private static final String EMPTY_REASON = "";
+    private static final Collection<String> SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON);
+
     private final List<Validator> validators;
-    private final List<Suppression> suppressions;
     private final ArrayList<ValidationEvent> events = new ArrayList<>();
     private final ValidatorFactory validatorFactory;
     private final Model model;
+    private final Map<String, Map<String, String>> namespaceSuppressions = new HashMap<>();
 
     private ModelValidator(
             Model model,
             ValidatorFactory validatorFactory,
-            List<Validator> validators,
-            List<Suppression> suppressions
+            List<Validator> validators
     ) {
         this.model = model;
         this.validatorFactory = validatorFactory;
         this.validators = new ArrayList<>(validators);
-        this.suppressions = new ArrayList<>(suppressions);
     }
 
     /**
@@ -67,40 +80,28 @@ private ModelValidator(
      * @param model Model to validate.
      * @param validatorFactory Factory used to find ValidatorService providers.
      * @param validators Additional validators to use.
-     * @param suppressions Additional suppressions to use.
      * @return Returns the encountered validation events.
      */
     static List<ValidationEvent> validate(
             Model model,
             ValidatorFactory validatorFactory,
-            List<Validator> validators,
-            List<Suppression> suppressions
+            List<Validator> validators
     ) {
-        return new ModelValidator(model, validatorFactory, validators, suppressions).doValidate();
-    }
-
-    /**
-     * Validates the given Model using validators configured explicitly and
-     * detected through metadata.
-     *
-     * @param model Model to validate.
-     * @param validatorFactory Factory used to find ValidatorService providers.
-     * @return Returns the encountered validation events.
-     */
-    static List<ValidationEvent> validate(Model model, ValidatorFactory validatorFactory) {
-        return validate(model, validatorFactory, ListUtils.of(), ListUtils.of());
+        return new ModelValidator(model, validatorFactory, validators).doValidate();
     }
 
     private List<ValidationEvent> doValidate() {
+        assembleNamespaceSuppressions();
         List<ValidatorDefinition> assembledValidatorDefinitions = assembleValidatorDefinitions();
         assembleValidators(assembledValidatorDefinitions);
-        assembleSuppressions();
+
         events.addAll(validators
                 .parallelStream()
                 .flatMap(validator -> validator.validate(model).stream())
-                .map(event -> Suppression.suppressEvent(event, suppressions))
+                .map(this::suppressEvent)
                 .filter(ModelValidator::filterPrelude)
                 .collect(Collectors.toList()));
+
         return events;
     }
 
@@ -125,16 +126,6 @@ private List<ValidatorDefinition> assembleValidatorDefinitions() {
         return result.getResult().orElseGet(Collections::emptyList);
     }
 
-    /**
-     * Loads suppressions based on the model metadata, aggregating any
-     * errors along the way.
-     */
-    private void assembleSuppressions() {
-        ValidatedResult<List<Suppression>> result = ValidationLoader.loadSuppressions(model.getMetadata());
-        events.addAll(result.getValidationEvents());
-        result.getResult().ifPresent(suppressions::addAll);
-    }
-
     /**
      * Loads validators from model metadata, combines with explicit
      * validators, and aggregates errors.
@@ -151,18 +142,97 @@ private void assembleValidators(List<ValidatorDefinition> definitions) {
             result.getResult().ifPresent(validators::add);
             events.addAll(result.getValidationEvents());
             if (result.getValidationEvents().isEmpty() && !result.getResult().isPresent()) {
-                events.add(unknownValidatorError(val.name, val.sourceLocation));
+                events.add(suppressEvent(unknownValidatorError(val.name, val.sourceLocation)));
             }
         }
     }
 
+    // Unknown validators don't fail the build!
     private ValidationEvent unknownValidatorError(String name, SourceLocation location) {
         return ValidationEvent.builder()
-                // Per the spec, the eventID is "UnknownValidator.<validatorName>".
-                .eventId("UnknownValidator." + name)
+                // Per the spec, the eventID is "UnknownValidator_<validatorName>".
+                .eventId("UnknownValidator_" + name)
                 .severity(Severity.WARNING)
                 .sourceLocation(location)
                 .message("Unable to locate a validator named `" + name + "`")
                 .build();
     }
+
+    // Find all namespace suppressions.
+    private void assembleNamespaceSuppressions() {
+        model.getMetadataProperty(SUPPRESSIONS).ifPresent(value -> {
+            List<ObjectNode> values = value.expectArrayNode().getElementsAs(ObjectNode.class);
+            for (ObjectNode rule : values) {
+                rule.warnIfAdditionalProperties(SUPPRESSION_KEYS);
+                String id = rule.expectStringMember(ID).getValue();
+                String namespace = rule.expectStringMember(NAMESPACE).getValue();
+                String reason = rule.getStringMemberOrDefault(REASON, EMPTY_REASON);
+                namespaceSuppressions.computeIfAbsent(id, i -> new HashMap<>()).put(namespace, reason);
+            }
+        });
+    }
+
+    private ValidationEvent suppressEvent(ValidationEvent event) {
+        // ERROR and SUPPRESSED events cannot be suppressed.
+        if (!event.getSeverity().canSuppress()) {
+            return event;
+        }
+
+        String reason = resolveReason(event);
+
+        // The event is not suppressed, return as-is.
+        if (reason == null) {
+            return event;
+        }
+
+        // The event was suppressed so change the severity and reason.
+        ValidationEvent.Builder builder = event.toBuilder();
+        builder.severity(Severity.SUPPRESSED);
+        if (!reason.equals(EMPTY_REASON)) {
+            builder.suppressionReason(reason);
+        }
+
+        return builder.build();
+    }
+
+    // Get the reason as a String if it is suppressed, or null otherwise.
+    private String resolveReason(ValidationEvent event) {
+        return event.getShapeId()
+                .flatMap(model::getShape)
+                .flatMap(shape -> matchSuppression(shape, event.getEventId()))
+                // This is always evaluated if a reason hasn't been found.
+                .orElseGet(() -> matchWildcardNamespaceSuppressions(event.getEventId()));
+    }
+
+    private Optional<String> matchSuppression(Shape shape, String eventId) {
+        // Check namespace-wide suppressions.
+        if (namespaceSuppressions.containsKey(eventId)) {
+            Map<String, String> namespaces = namespaceSuppressions.get(eventId);
+            if (namespaces.containsKey(shape.getId().getNamespace())) {
+                return Optional.of(namespaces.get(shape.getId().getNamespace()));
+            }
+        }
+
+        // Traits take precedent over service suppressions.
+        if (shape.getTrait(SuppressTrait.class).isPresent()) {
+            if (shape.expectTrait(SuppressTrait.class).getValues().contains(eventId)) {
+                // The "" is filtered out before being passed to the
+                // updated ValidationEvent.
+                return Optional.of(EMPTY_REASON);
+            }
+        }
+
+        return Optional.empty();
+    }
+
+    private String matchWildcardNamespaceSuppressions(String eventId) {
+        if (namespaceSuppressions.containsKey(eventId)) {
+            Map<String, String> namespaces = namespaceSuppressions.get(eventId);
+            if (namespaces.containsKey(STAR)) {
+                return namespaces.get(STAR);
+            }
+        }
+
+        return null;
+    }
 }