From 5b197cc97715413b74ae2cbff662c3a24f6089e1 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Thu, 23 Apr 2020 14:27:15 -0700 Subject: [PATCH 1/2] 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. --- .../model/awsJson1_1/main.json | 5 + .../model/shared-types.smithy | 3 +- ...n.smithy.model.validation.ValidatorService | 2 - .../errorfiles/validator-loading-test.errors | 5 - .../errorfiles/validator-loading-test.json | 42 --- .../smithy/model/loader/ModelAssembler.java | 21 +- .../smithy/model/loader/ModelValidator.java | 154 ++++++++--- .../amazon/smithy/model/loader/Prelude.java | 2 + .../smithy/model/loader/ValidationLoader.java | 52 +--- .../smithy/model/traits/SuppressTrait.java | 53 ++++ .../EmitEachSelectorValidator.java | 7 +- .../EmitNoneSelectorValidator.java | 8 +- .../LineValidationEventFormatter.java | 9 +- .../smithy/model/validation/Suppression.java | 243 ------------------ .../validation/testrunner/SmithyTestCase.java | 11 +- ...re.amazon.smithy.model.traits.TraitService | 1 + ...n.smithy.model.validation.ValidatorService | 2 + .../smithy/model/loader/prelude-traits.smithy | 8 + .../model/loader/ModelAssemblerTest.java | 17 -- .../model/loader/ModelValidatorTest.java | 12 - .../model/transform/RenameShapesTest.java | 8 - .../model/validation/SuppressionTest.java | 109 -------- .../model/validation/ValidationEventTest.java | 14 + .../errorfiles/loader/suppression-test.errors | 7 + .../errorfiles/loader/suppression-test.smithy | 80 ++++++ ...ors-and-suppressions-must-be-arrays.errors | 2 - .../validators-must-be-arrays.errors | 1 + ...ys.json => validators-must-be-arrays.json} | 2 - .../emit-each-selector-validator.errors | 0 .../emit-each-selector-validator.json | 0 .../emit-none-selector-validator.errors | 0 .../emit-none-selector-validator.json | 0 .../amazon/smithy/model/loader/main.json | 8 - .../transform/rename-shape-test-model.json | 14 +- .../mqtt/traits/errorfiles/job-service.smithy | 3 +- 35 files changed, 326 insertions(+), 579 deletions(-) delete mode 100644 smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors delete mode 100644 smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/traits/SuppressTrait.java rename {smithy-linters/src/main/java/software/amazon/smithy/linters => smithy-model/src/main/java/software/amazon/smithy/model/validation}/EmitEachSelectorValidator.java (88%) rename {smithy-linters/src/main/java/software/amazon/smithy/linters => smithy-model/src/main/java/software/amazon/smithy/model/validation}/EmitNoneSelectorValidator.java (88%) delete mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/validation/Suppression.java create mode 100644 smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService delete mode 100644 smithy-model/src/test/java/software/amazon/smithy/model/validation/SuppressionTest.java create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.smithy delete mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.errors create mode 100644 smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.errors rename smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/{validators-and-suppressions-must-be-arrays.json => validators-must-be-arrays.json} (69%) rename {smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles => smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters}/emit-each-selector-validator.errors (100%) rename {smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles => smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters}/emit-each-selector-validator.json (100%) rename {smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles => smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters}/emit-none-selector-validator.errors (100%) rename {smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles => smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters}/emit-none-selector-validator.json (100%) diff --git a/smithy-aws-protocol-tests/model/awsJson1_1/main.json b/smithy-aws-protocol-tests/model/awsJson1_1/main.json index 4145b860c63..2c46104624c 100644 --- 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 index 255bbf88cf7..fb8ba97747e 100644 --- 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/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService b/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService index cde4964fdfc..f7c6374dd94 100644 --- a/smithy-linters/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService +++ b/smithy-linters/src/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/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors deleted file mode 100644 index 24c49ee7c88..00000000000 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.errors +++ /dev/null @@ -1,5 +0,0 @@ -[DANGER] ns.foo#Ignore_Me: string shape name, `Ignore_Me`, is not upper camel case | CamelCase -[ERROR] -: Each element of `suppressions` must be an object. Found array. | Model -[ERROR] -: Each element of `validators` must be an object. Found string. | Model -[SUPPRESSED] ns.baz#This_is_ignored_too: string shape name, `This_is_ignored_too`, is not upper camel case | CamelCase -[WARNING] -: Unable to locate a validator named `UnknownValidator` | UnknownValidator.UnknownValidator diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json deleted file mode 100644 index 09253a657cb..00000000000 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/validator-loading-test.json +++ /dev/null @@ -1,42 +0,0 @@ -{ - "smithy": "1.0.0", - "shapes": { - "ns.foo#OrphanShape": { - "type": "string" - }, - "ns.foo#Ignore_Me": { - "type": "string" - }, - "ns.baz#This_is_ignored_too": { - "type": "string" - } - }, - "metadata": { - "validators": [ - { - "name": "UnknownValidator" - }, - { - "name": "CamelCase" - }, - "foo" - ], - "suppressions": [ - { - "ids": [ - "UnreferencedShape" - ] - }, - [], - { - "ids": [ - "CamelCase" - ], - "shapes": [ - "ns.foo#IgnoreMe", - "ns.baz#" - ] - } - ] - } -} 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 8f52d3a674c..ac4f7eb899a 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 @@ -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 validators = new ArrayList<>(); - private final List suppressions = new ArrayList<>(); private final List documentNodes = new ArrayList<>(); private final List mergeModels = new ArrayList<>(); private final List> 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() { * *
    *
  • Validators registered via {@link #addValidator}
    • - *
  • Suppressions registered via {@link #addSuppression}
    • *
  • Models registered via {@link #addImport}
    • *
  • Models registered via {@link #addDocumentNode}
    • *
  • Models registered via {@link #addUnparsedModel}
    • @@ -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 validate(Model model, List model } // Validate the model based on the explicit validators and model metadata. - List events = ModelValidator.validate(model, validatorFactory, - assembleValidators(), suppressions); + List 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 index 8961ac53278..66f438becde 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 @@ -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. * *

    ModelValidator is used to apply validation to a loaded Model. This * class is used in tandem with {@link ModelAssembler}. * - *

    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. + *

    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 SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON); + private final List validators; - private final List suppressions; private final ArrayList events = new ArrayList<>(); private final ValidatorFactory validatorFactory; private final Model model; + private final Map> namespaceSuppressions = new HashMap<>(); private ModelValidator( Model model, ValidatorFactory validatorFactory, - List validators, - List suppressions + List 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 validate( Model model, ValidatorFactory validatorFactory, - List validators, - List suppressions + List 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 validate(Model model, ValidatorFactory validatorFactory) { - return validate(model, validatorFactory, ListUtils.of(), ListUtils.of()); + return new ModelValidator(model, validatorFactory, validators).doValidate(); } private List doValidate() { + assembleNamespaceSuppressions(); List 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 assembleValidatorDefinitions() { return result.getResult().orElseGet(Collections::emptyList); } - /** - * Loads suppressions based on the model metadata, aggregating any - * errors along the way. - */ - private void assembleSuppressions() { - ValidatedResult> 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 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.". - .eventId("UnknownValidator." + name) + // Per the spec, the eventID is "UnknownValidator_". + .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 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 matchSuppression(Shape shape, String eventId) { + // Check namespace-wide suppressions. + if (namespaceSuppressions.containsKey(eventId)) { + Map 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 namespaces = namespaceSuppressions.get(eventId); + if (namespaces.containsKey(STAR)) { + return namespaces.get(STAR); + } + } + + return null; + } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/Prelude.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/Prelude.java index be895329ae9..d1075fbd4f1 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/Prelude.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/Prelude.java @@ -83,6 +83,7 @@ import software.amazon.smithy.model.traits.SensitiveTrait; import software.amazon.smithy.model.traits.SinceTrait; import software.amazon.smithy.model.traits.StreamingTrait; +import software.amazon.smithy.model.traits.SuppressTrait; import software.amazon.smithy.model.traits.TagsTrait; import software.amazon.smithy.model.traits.TimestampFormatTrait; import software.amazon.smithy.model.traits.TitleTrait; @@ -191,6 +192,7 @@ public final class Prelude { SensitiveTrait.ID, SinceTrait.ID, StreamingTrait.ID, + SuppressTrait.ID, TagsTrait.ID, TimestampFormatTrait.ID, TitleTrait.ID, diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ValidationLoader.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ValidationLoader.java index 9e36eb92518..925d795b229 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ValidationLoader.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ValidationLoader.java @@ -20,58 +20,39 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.Function; import software.amazon.smithy.model.SourceException; import software.amazon.smithy.model.node.ArrayNode; import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.ObjectNode; -import software.amazon.smithy.model.node.StringNode; import software.amazon.smithy.model.selector.Selector; 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.utils.ListUtils; -/** - * For backward compatibility, this will load both smithy.validators and - * validators; smithy.suppressions and suppressions. - */ final class ValidationLoader { + private static final List SEVERITIES = ListUtils.of("DANGER", "WARNING", "NOTE"); - private static final List SUPPRESSION_PROPERTIES = ListUtils.of("ids", "shapes", "reason"); private static final List VALIDATOR_PROPERTIES = ListUtils.of( "name", "id", "message", "severity", "namespaces", "selector", "configuration"); private ValidationLoader() {} static ValidatedResult> loadValidators(Map metadata) { - return load(metadata, "validators", ValidationLoader::loadSingleValidator); - } - - static ValidatedResult> loadSuppressions(Map metadata) { - return load(metadata, "suppressions", ValidationLoader::loadSingleSuppression); - } - - private static ValidatedResult> load( - Map metadata, - String key, - Function f - ) { - if (!metadata.containsKey(key)) { + if (!metadata.containsKey("validators")) { return ValidatedResult.empty(); } List events = new ArrayList<>(); - List result = new ArrayList<>(); - Node node = metadata.get(key); + List result = new ArrayList<>(); + Node node = metadata.get("validators"); try { - ArrayNode values = node.expectArrayNode(key + " must be an array. Found {type}."); + ArrayNode values = node.expectArrayNode("validators must be an array. Found {type}."); for (Node element : values.getElements()) { try { ObjectNode definition = element.expectObjectNode( - "Each element of `" + key + "` must be an object. Found {type}."); - result.add(f.apply(definition)); + "Each element of `validators` must be an object. Found {type}."); + result.add(ValidationLoader.loadSingleValidator(definition)); } catch (SourceException e) { events.add(ValidationEvent.fromSourceException(e)); } @@ -79,15 +60,16 @@ private static ValidatedResult> load( } catch (SourceException e) { events.add(ValidationEvent.fromSourceException(e)); } + return new ValidatedResult<>(result, events); } private static ValidatorDefinition loadSingleValidator(ObjectNode node) { node.warnIfAdditionalProperties(VALIDATOR_PROPERTIES); String name = node.expectStringMember("name").getValue(); - ValidatorDefinition def = new ValidatorDefinition( - name, node.getStringMember("id").map(StringNode::getValue).orElse(name)); - def.message = node.getStringMember("message").map(StringNode::getValue).orElse(null); + String id = node.getStringMemberOrDefault("id", name); + ValidatorDefinition def = new ValidatorDefinition(name, id); + def.message = node.getStringMemberOrDefault("message", null); def.severity = node.getStringMember("severity") .map(value -> value.expectOneOf(SEVERITIES)) .map(value -> Severity.fromString(value).get()) @@ -101,16 +83,4 @@ private static ValidatorDefinition loadSingleValidator(ObjectNode node) { return def; } - - private static Suppression loadSingleSuppression(ObjectNode node) { - node.warnIfAdditionalProperties(SUPPRESSION_PROPERTIES); - Suppression.Builder builder = Suppression.builder().sourceLocation(node.getSourceLocation()); - loadArrayOfString("ids", node.expectMember("ids", "Missing required validator `ids` property.")) - .forEach(builder::addValidatorId); - node.getMember("shapes") - .map(value -> loadArrayOfString("shapes", value)) - .ifPresent(values -> values.forEach(builder::addShape)); - node.getStringMember("reason").map(StringNode::getValue).ifPresent(builder::reason); - return builder.build(); - } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/traits/SuppressTrait.java b/smithy-model/src/main/java/software/amazon/smithy/model/traits/SuppressTrait.java new file mode 100644 index 00000000000..96c84c229f3 --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/traits/SuppressTrait.java @@ -0,0 +1,53 @@ +/* + * 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. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.traits; + +import java.util.List; +import software.amazon.smithy.model.FromSourceLocation; +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.utils.ToSmithyBuilder; + +public final class SuppressTrait extends StringListTrait implements ToSmithyBuilder { + public static final ShapeId ID = ShapeId.from("smithy.api#suppress"); + + private SuppressTrait(List values, FromSourceLocation sourceLocation) { + super(ID, values, sourceLocation); + } + + public static final class Provider extends StringListTrait.Provider { + public Provider() { + super(ID, SuppressTrait::new); + } + } + + @Override + public Builder toBuilder() { + return builder().sourceLocation(getSourceLocation()).values(getValues()); + } + + public static Builder builder() { + return new Builder(); + } + + public static final class Builder extends StringListTrait.Builder { + private Builder() {} + + @Override + public SuppressTrait build() { + return new SuppressTrait(getValues(), getSourceLocation()); + } + } +} diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/EmitEachSelectorValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.java similarity index 88% rename from smithy-linters/src/main/java/software/amazon/smithy/linters/EmitEachSelectorValidator.java rename to smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.java index 209fcc36a70..d35d2da18bc 100644 --- a/smithy-linters/src/main/java/software/amazon/smithy/linters/EmitEachSelectorValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.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. @@ -13,16 +13,13 @@ * permissions and limitations under the License. */ -package software.amazon.smithy.linters; +package software.amazon.smithy.model.validation; import java.util.List; import java.util.stream.Collectors; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.selector.Selector; -import software.amazon.smithy.model.validation.AbstractValidator; -import software.amazon.smithy.model.validation.ValidationEvent; -import software.amazon.smithy.model.validation.ValidatorService; /** * Emits a validation event for each shape that matches a selector. diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/EmitNoneSelectorValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.java similarity index 88% rename from smithy-linters/src/main/java/software/amazon/smithy/linters/EmitNoneSelectorValidator.java rename to smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.java index 186caeced10..641ad8dfd8e 100644 --- a/smithy-linters/src/main/java/software/amazon/smithy/linters/EmitNoneSelectorValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.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. @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package software.amazon.smithy.linters; +package software.amazon.smithy.model.validation; import java.util.List; import java.util.Set; @@ -23,10 +23,6 @@ import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.selector.Selector; import software.amazon.smithy.model.shapes.Shape; -import software.amazon.smithy.model.validation.AbstractValidator; -import software.amazon.smithy.model.validation.Severity; -import software.amazon.smithy.model.validation.ValidationEvent; -import software.amazon.smithy.model.validation.ValidatorService; import software.amazon.smithy.utils.ListUtils; /** diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java index 8251495e5b1..b5b78027681 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/LineValidationEventFormatter.java @@ -23,11 +23,18 @@ public final class LineValidationEventFormatter implements ValidationEventFormatter { @Override public String format(ValidationEvent event) { + String message = event.getMessage(); + + String reason = event.getSuppressionReason().orElse(null); + if (reason != null) { + message += " (" + reason + ")"; + } + return String.format( "[%s] %s: %s | %s %s:%s:%s", event.getSeverity(), event.getShapeId().map(ShapeId::toString).orElse("-"), - event.getMessage(), + message, event.getEventId(), event.getSourceLocation().getFilename(), event.getSourceLocation().getLine(), diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/Suppression.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/Suppression.java deleted file mode 100644 index 7ddddeb637a..00000000000 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/Suppression.java +++ /dev/null @@ -1,243 +0,0 @@ -/* - * Copyright 2019 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. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package software.amazon.smithy.model.validation; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; -import software.amazon.smithy.model.FromSourceLocation; -import software.amazon.smithy.model.SourceException; -import software.amazon.smithy.model.SourceLocation; -import software.amazon.smithy.model.shapes.ShapeId; -import software.amazon.smithy.model.shapes.ShapeIdSyntaxException; -import software.amazon.smithy.utils.SmithyBuilder; -import software.amazon.smithy.utils.ToSmithyBuilder; - -/** - * Validator suppression. - */ -public final class Suppression implements FromSourceLocation, ToSmithyBuilder { - - private final SourceLocation sourceLocation; - private final String reason; - private final Set shapes; - private final Set validatorIds; - private final Set namespaceNames; - private Predicate predicate; - - private Suppression(Builder builder) { - if (builder.validatorIds.isEmpty()) { - throw new SourceException("Suppressions require at least one `id`", builder.sourceLocation); - } - - builder.validatorIds.forEach(id -> { - if (id.isEmpty()) { - throw new SourceException("Suppression `ids` element must not be empty", builder.sourceLocation); - } else if (id.indexOf('*') > 0) { - throw new SourceException( - String.format("Invalid suppression `ids` wildcard: `%s`", id), builder.sourceLocation); - } - }); - validatorIds = Collections.unmodifiableSet(new LinkedHashSet<>(builder.validatorIds)); - - // Load each shape string into a Shape.Id, wrapping exceptions. - Set shapeIds = new LinkedHashSet<>(); - builder.shapes.forEach(shape -> { - try { - shapeIds.add(ShapeId.from(shape)); - } catch (ShapeIdSyntaxException e) { - throw new SourceException(e.getMessage(), builder.sourceLocation); - } - }); - shapes = Collections.unmodifiableSet(shapeIds); - - sourceLocation = builder.sourceLocation; - reason = builder.reason; - namespaceNames = new HashSet<>(builder.namespaceNames); - predicate = createPredicate(validatorIds, shapes, namespaceNames); - } - - public static Builder builder() { - return new Builder(); - } - - @Override - public Builder toBuilder() { - Builder builder = builder().sourceLocation(sourceLocation).reason(reason); - namespaceNames.forEach(builder::addShape); - shapes.forEach(builder::addShape); - validatorIds.forEach(builder::addValidatorId); - return builder; - } - - /** - *

    Applies a list of suppressions to a validation event. - * - *

    Suppressions are applied, in order, until the event is marked as - * suppressed or there are no more suppressions to apply. The list of - * suppressions is short-circuited as soon as the first suppression - * suppresses the event. - * - * @param validationEvent Validation event to suppress if needed. - * @param suppressions Ordered list of suppressions to apply. - * @return Returns the event that is mapped over by the list of suppressions. - */ - public static ValidationEvent suppressEvent(ValidationEvent validationEvent, List suppressions) { - Iterator suppressionIterator = suppressions.iterator(); - while (validationEvent.getSeverity().canSuppress() && suppressionIterator.hasNext()) { - validationEvent = suppressionIterator.next().suppress(validationEvent); - } - return validationEvent; - } - - private static Predicate createPredicate( - Set eventIds, - Set shapeIds, - Set namespaceNames - ) { - // Build up a list of predicates that all must match. - List> predicates = new ArrayList<>(); - - // Only suppress things that can be suppressed. - predicates.add(event -> event.getSeverity().canSuppress()); - - eventIds.stream() - .filter(rule -> !rule.equals("*")) - .forEach(rule -> predicates.add(event -> event.getEventId().equals(rule))); - - if (!shapeIds.isEmpty() || !namespaceNames.isEmpty()) { - predicates.add(event -> event.getShapeId() - .filter(id -> namespaceNames.contains(id.getNamespace()) || shapeIds.contains(id)) - .isPresent()); - } - - return event -> predicates.stream().allMatch(p -> p.test(event)); - } - - /** - * @return Returns the optional reason string. - */ - public Optional getReason() { - return Optional.ofNullable(reason); - } - - /** - * @return Returns the list of validator IDs. - */ - public Set getValidatorIds() { - return validatorIds; - } - - /** - * @return Returns the possibly empty list of shapes to suppress. - */ - public Set getShapes() { - return shapes; - } - - /** - * @return Returns the namespaces that are suppressed. - */ - public Set getNamespaceNames() { - return namespaceNames; - } - - /** - * Given an event returns a mapped event that is either the same event - * or a suppressed version of it. - * - * @param event Event to map over. - * @return Returns the suppressed event or the original event. - */ - public ValidationEvent suppress(ValidationEvent event) { - if (!predicate.test(event)) { - return event; - } - - ValidationEvent.Builder builder = event.toBuilder().severity(Severity.SUPPRESSED); - if (reason != null) { - builder.suppressionReason(reason); - } - return builder.build(); - } - - @Override - public SourceLocation getSourceLocation() { - return sourceLocation; - } - - @Override - public String toString() { - return "suppression of `" + validatorIds.stream().collect(Collectors.joining("`, `")) + "`"; - } - - /** - * Builder used to create suppression definitions. - */ - public static final class Builder implements SmithyBuilder { - - private SourceLocation sourceLocation = SourceLocation.none(); - private String reason; - private final Set shapes = new LinkedHashSet<>(); - private final Set validatorIds = new LinkedHashSet<>(); - private final Set namespaceNames = new HashSet<>(); - - private Builder() {} - - public Builder sourceLocation(SourceLocation sourceLocation) { - this.sourceLocation = Objects.requireNonNull(sourceLocation); - return this; - } - - public Builder reason(String reason) { - this.reason = reason; - return this; - } - - public Builder addShape(String shapeId) { - Objects.requireNonNull(shapeId, "shapeId must not be null"); - if (shapeId.endsWith("#")) { - namespaceNames.add(shapeId.substring(0, shapeId.length() - 1)); - } else { - shapes.add(shapeId); - } - return this; - } - - public Builder addShape(ShapeId shapeId) { - shapes.add(Objects.requireNonNull(shapeId.toString(), "shapeId must not be null")); - return this; - } - - public Builder addValidatorId(String id) { - this.validatorIds.add(Objects.requireNonNull(id, "validatorId must not be null")); - return this; - } - - @Override - public Suppression build() { - return new Suppression(this); - } - } -} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestCase.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestCase.java index de5a04cfc00..ffacf99d001 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestCase.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/testrunner/SmithyTestCase.java @@ -99,7 +99,8 @@ public String getModelLocation() { * compared against the expected validation events. An actual event (A) is * considered a match with an expected event (E) if A and E target the * same shape, use the same validation event ID, have the same severity, - * and the message of E starts with the message of A. + * and the message of E starts with the suppression reason or message + * of A. * * @param validatedResult Result of creating and validating the model. * @return Returns the created test case result. @@ -120,11 +121,17 @@ public Result createResult(ValidatedResult validatedResult) { } private static boolean compareEvents(ValidationEvent expected, ValidationEvent actual) { + String normalizedActualMessage = actual.getMessage(); + if (actual.getSuppressionReason().isPresent()) { + normalizedActualMessage += " (" + actual.getSuppressionReason().get() + ")"; + } + + String comparedMessage = expected.getMessage().replace("\n", "\\n"); return expected.getSeverity() == actual.getSeverity() && expected.getEventId().equals(actual.getEventId()) && expected.getShapeId().equals(actual.getShapeId()) // Normalize new lines. - && actual.getMessage().replace("\n", "\\n").startsWith(expected.getMessage().replace("\n", "\\n")); + && normalizedActualMessage.startsWith(comparedMessage); } private static String inferErrorFileLocation(String modelLocation) { diff --git a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService index cfe2361f031..e0d472deeab 100644 --- a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService +++ b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.traits.TraitService @@ -38,6 +38,7 @@ software.amazon.smithy.model.traits.RetryableTrait$Provider software.amazon.smithy.model.traits.SensitiveTrait$Provider software.amazon.smithy.model.traits.SinceTrait$Provider software.amazon.smithy.model.traits.StreamingTrait$Provider +software.amazon.smithy.model.traits.SuppressTrait$Provider software.amazon.smithy.model.traits.TagsTrait$Provider software.amazon.smithy.model.traits.TimestampFormatTrait$Provider software.amazon.smithy.model.traits.TitleTrait$Provider diff --git a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService new file mode 100644 index 00000000000..5c4c75c5bbd --- /dev/null +++ b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService @@ -0,0 +1,2 @@ +software.amazon.smithy.model.validation.EmitEachSelectorValidator$Provider +software.amazon.smithy.model.validation.EmitNoneSelectorValidator$Provider diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy index ff533901ae1..4fe59b63c6e 100644 --- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy +++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy @@ -627,3 +627,11 @@ structure endpoint { @trait(selector: ":test(member:of(structure)[trait|required] > string)") @tags(["diff.error.const"]) structure hostLabel {} + +/// Suppresses a validation event by ID for a given shape. +@trait +list suppress { + @pattern("^[_a-zA-Z][A-Za-z0-9]*$") + @length(min: 1) + member: String +} diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java index 9c80105fccd..be369963122 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelAssemblerTest.java @@ -57,7 +57,6 @@ import software.amazon.smithy.model.traits.MediaTypeTrait; import software.amazon.smithy.model.traits.SensitiveTrait; 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; @@ -78,16 +77,6 @@ public void after() throws IOException { Files.walk(outputDirectory).sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete); } - @Test - public void addsExplicitSuppressions() { - Suppression suppression = Suppression.builder().addValidatorId("foo").build(); - ValidatedResult result = new ModelAssembler() - .addSuppression(suppression) - .assemble(); - - assertThat(result.getValidationEvents(), empty()); - } - @Test public void addsExplicitShapes() { StringShape shape = StringShape.builder().id("ns.foo#Bar").build(); @@ -174,9 +163,6 @@ public void importsSymlinksDirectoryWithAllShapes() throws Exception { .addImport(createSymbolicLink(Paths.get(getClass().getResource("nested").toURI()), "symlink-nested")) .assemble(); - result.getValidationEvents().forEach(event -> { - assertThat(event.getSuppressionReason().get(), is("This shape is being tested for model assembly.")); - }); Model model = result.unwrap(); assertTrue(model.getShape(ShapeId.from("example.namespace#String")).isPresent()); assertThat(model.getShape(ShapeId.from("example.namespace#String")).get().getType(), @@ -270,9 +256,6 @@ public void importsFilesWithAllShapes() throws Exception { .addImport(Paths.get(getClass().getResource("nested").toURI())) .assemble(); - result.getValidationEvents().forEach(event -> { - assertThat(event.getSuppressionReason().get(), is("This shape is being tested for model assembly.")); - }); Model model = result.unwrap(); assertTrue(model.getShape(ShapeId.from("example.namespace#String")).isPresent()); assertThat(model.getShape(ShapeId.from("example.namespace#String")).get().getType(), diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelValidatorTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelValidatorTest.java index 4dc8659dd1b..61090fc828a 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelValidatorTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/loader/ModelValidatorTest.java @@ -18,14 +18,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.hasSize; import java.util.Collections; import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; 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; @@ -34,16 +32,6 @@ */ public class ModelValidatorTest { - @Test - public void addsExplicitSuppressions() { - Suppression suppression = Suppression.builder().addValidatorId("foo").build(); - ValidatedResult result = new ModelAssembler() - .addSuppression(suppression) - .assemble(); - - assertThat(result.getValidationEvents(), empty()); - } - @Test public void addsExplicitValidators() { ValidationEvent event = ValidationEvent.builder() diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/transform/RenameShapesTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/transform/RenameShapesTest.java index 9697088f1c8..63afad8a789 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/transform/RenameShapesTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/transform/RenameShapesTest.java @@ -21,12 +21,9 @@ import java.util.HashMap; import java.util.Map; - import org.junit.jupiter.api.Test; import software.amazon.smithy.model.Model; -import software.amazon.smithy.model.node.ArrayNode; import software.amazon.smithy.model.node.Node; -import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.node.StringNode; import software.amazon.smithy.model.shapes.ListShape; import software.amazon.smithy.model.shapes.MapShape; @@ -112,12 +109,7 @@ public void updatesMetadataReferences() { renamed.put(fooUnreferenced, barUnreferenced); ModelTransformer transformer = ModelTransformer.create(); Model result = transformer.renameShapes(model, renamed); - ArrayNode suppressions = result.getMetadata().get("suppressions").asArrayNode().get(); - ObjectNode suppression = suppressions.getElements().get(0).asObjectNode().get(); - ArrayNode suppressionShapes = suppression.getStringMap().get("shapes").expectArrayNode(); - StringNode suppressionShape = suppressionShapes.getElements().get(0).asStringNode().get(); - assertEquals(suppressionShape.getValue(), "ns.bar#UnreferencedString"); assertTrue(result.getShape(barUnreferenced).isPresent()); assertFalse(result.getShape(fooUnreferenced).isPresent()); } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/validation/SuppressionTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/validation/SuppressionTest.java deleted file mode 100644 index 0d572365e31..00000000000 --- a/smithy-model/src/test/java/software/amazon/smithy/model/validation/SuppressionTest.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright 2019 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. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package software.amazon.smithy.model.validation; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.ArrayList; -import java.util.List; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import software.amazon.smithy.model.SourceException; -import software.amazon.smithy.model.shapes.ShapeId; - -public class SuppressionTest { - - @Test - public void hasGetters() { - Suppression suppression = Suppression.builder() - .addValidatorId("*") - .reason("the reason") - .addShape("ns.foo#bar") - .addShape("ns.foo#") - .build(); - - assertThat(suppression.getReason().get(), equalTo("the reason")); - assertThat(suppression.getValidatorIds(), hasSize(1)); - assertThat(suppression.getShapes(), hasSize(1)); - assertThat(suppression.getNamespaceNames(), hasSize(1)); - assertTrue(suppression.getNamespaceNames().contains("ns.foo")); - assertTrue(suppression.getValidatorIds().contains("*")); - assertTrue(suppression.getShapes().contains(ShapeId.from("ns.foo#bar"))); - } - - @Test - public void doesNotSuppressEventsWhenNoMatches() { - List suppressions = new ArrayList<>(); - suppressions.add(Suppression.builder().addValidatorId("foo").build()); - suppressions.add(Suppression.builder().addValidatorId("baz").build()); - suppressions.add(Suppression.builder().addValidatorId("bar").build()); - ValidationEvent input = ValidationEvent.builder() - .eventId("qux") - .message("test") - .severity(Severity.WARNING) - .build(); - ValidationEvent output = Suppression.suppressEvent(input, suppressions); - - assertThat(output.getSeverity(), is(Severity.WARNING)); - } - - @Test - public void suppressesEventsWhenThereAreMatches() { - List suppressions = new ArrayList<>(); - suppressions.add(Suppression.builder().addValidatorId("foo").build()); - suppressions.add(Suppression.builder().addValidatorId("baz").build()); - suppressions.add(Suppression.builder().addValidatorId("bar").build()); - ValidationEvent input = ValidationEvent.builder() - .eventId("foo") - .message("test") - .severity(Severity.WARNING) - .build(); - ValidationEvent output = Suppression.suppressEvent(input, suppressions); - - assertThat(output.getSeverity(), is(Severity.SUPPRESSED)); - } - - @Test - public void ruleNameCannotBeEmpty() { - Assertions.assertThrows(SourceException.class, () -> Suppression.builder().addValidatorId("").build()); - } - - @Test - public void ruleNameCannotContainMultipleWildcards() { - Assertions.assertThrows(SourceException.class, () -> { - Suppression.builder().addValidatorId("foo.*.bar.*").build(); - }); - } - - @Test - public void wildcardsMustOnlyBeAtEndOfString() { - Assertions.assertThrows(SourceException.class, () -> { - Suppression.builder().addValidatorId("foo.*.bar").build(); - }); - } - - @Test - public void convertsToString() { - Suppression suppression = Suppression.builder().addValidatorId("foo").addValidatorId("baz").build(); - - assertEquals(suppression.toString(), "suppression of `foo`, `baz`"); - } -} diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java index 01b8bb781af..5100a165168 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/validation/ValidationEventTest.java @@ -284,6 +284,20 @@ public void toStringContainsSourceLocation() { assertEquals(a.toString(), "[SUPPRESSED] ns.foo#baz: The message | abc.foo file:1:2"); } + @Test + public void toStringContainsSuppressionReason() { + ValidationEvent a = ValidationEvent.builder() + .message("The message") + .severity(Severity.SUPPRESSED) + .eventId("abc.foo") + .shapeId(ShapeId.from("ns.foo#baz")) + .suppressionReason("Foo baz bar") + .sourceLocation(new SourceLocation("file", 1, 2)) + .build(); + + assertEquals(a.toString(), "[SUPPRESSED] ns.foo#baz: The message (Foo baz bar) | abc.foo file:1:2"); + } + @Test public void convertsToNode() { ValidationEvent a = ValidationEvent.builder() diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.errors new file mode 100644 index 00000000000..33aa0170242 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.errors @@ -0,0 +1,7 @@ +[DANGER] smithy.example#List1: This list must not exist! | NoListsPlease +[WARNING] smithy.example#List1$member: Members? Yuck! | ListMembersAreBadOkay +[SUPPRESSED] smithy.example#List2: This list must not exist! | NoListsPlease +[SUPPRESSED] smithy.example#List2$member: Members? Yuck! | ListMembersAreBadOkay +[SUPPRESSED] -: Unable to locate a validator named `UnknownValidator2` (Please ignore this) | UnknownValidator_UnknownValidator2 +[WARNING] -: Unable to locate a validator named `UnknownValidator1` | UnknownValidator_UnknownValidator1 +[SUPPRESSED] smithy.example#MyService: This is suppressed | SuppressedValidator diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.smithy new file mode 100644 index 00000000000..3e73ffd6f14 --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/suppression-test.smithy @@ -0,0 +1,80 @@ +$version: "1.0.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "NoListsPlease", + message: "This list must not exist!", + configuration: { + selector: "list" + } + }, + { + name: "EmitEachSelector", + id: "ListMembersAreBadOkay", + message: "Members? Yuck!", + severity: "WARNING", + configuration: { + selector: "list > member" + } + }, + { + // This one is suppressed. + name: "EmitEachSelector", + id: "SuppressedValidator", + message: "This is suppressed", + severity: "WARNING", + configuration: { + selector: "service" + } + }, + { + name: "UnknownValidator1" + }, + { + name: "UnknownValidator2" + } +] + +metadata suppressions = [ + { + id: "UnknownValidator_UnknownValidator2", // Matches any event not bound to a shape. + namespace: "*", + reason: "Please ignore this", + }, + { + id: "SuppressedValidator", + namespace: "smithy.example.ignore.this.one", // matches nothing + }, + { + id: "SuppressedValidator", + namespace: "smithy.example" + }, +] + +namespace smithy.example + +@suppress(["IngoreMe"]) +service MyService { + version: "XYZ", + operations: [GetFoo], +} + +operation GetFoo { + input: GetFooInput, +} + +structure GetFooInput { + list1: List1, + list2: List2, +} + +list List1 { + member: String, +} + +@suppress(["NoListsPlease"]) +list List2 { + @suppress(["ListMembersAreBadOkay"]) + member: String, +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.errors deleted file mode 100644 index acd4a7137bb..00000000000 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.errors +++ /dev/null @@ -1,2 +0,0 @@ -[ERROR] -: suppressions must be an array. Found object. | Model -[ERROR] -: validators must be an array. Found object. | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.errors new file mode 100644 index 00000000000..8eacb04df1b --- /dev/null +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.errors @@ -0,0 +1 @@ +[ERROR] -: validators must be an array. Found object. | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.json similarity index 69% rename from smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.json rename to smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.json index 11144e0447b..b980835346e 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-and-suppressions-must-be-arrays.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/version_1_0_0/validators-must-be-arrays.json @@ -2,8 +2,6 @@ "smithy": "1.0.0", "metadata": { "validators": { - }, - "suppressions": { } } } diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-each-selector-validator.errors similarity index 100% rename from smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.errors rename to smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-each-selector-validator.errors diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-each-selector-validator.json similarity index 100% rename from smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-each-selector-validator.json rename to smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-each-selector-validator.json diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-none-selector-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-none-selector-validator.errors similarity index 100% rename from smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-none-selector-validator.errors rename to smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-none-selector-validator.errors diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-none-selector-validator.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-none-selector-validator.json similarity index 100% rename from smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/emit-none-selector-validator.json rename to smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/linters/emit-none-selector-validator.json diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/main.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/main.json index a35f426b227..805390e90b9 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/main.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/main.json @@ -81,14 +81,6 @@ "list": [ "a", "b" - ], - "suppressions": [ - { - "ids": ["UnreferencedShape"], - "shapes": [ - "example.namespace#"], - "reason": "This shape is being tested for model assembly." - } ] } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/rename-shape-test-model.json b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/rename-shape-test-model.json index 10b5270e5b0..4012b74786d 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/transform/rename-shape-test-model.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/transform/rename-shape-test-model.json @@ -157,16 +157,10 @@ "type": "integer" }, "ns.foo#UnreferencedString": { - "type": "string" - } - }, - "metadata": { - "suppressions": [ - { - "ids": ["UnreferencedShape"], - "shapes": ["ns.foo#UnreferencedString"], - "reason": "unreferenced shape" + "type": "string", + "traits": { + "smithy.api#suppress": ["UnreferencedShape"] } - ] + } } } diff --git a/smithy-mqtt-traits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy b/smithy-mqtt-traits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy index 890a99cf6b8..a23dce7b2a6 100644 --- a/smithy-mqtt-traits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy +++ b/smithy-mqtt-traits/src/test/resources/software/amazon/smithy/mqtt/traits/errorfiles/job-service.smithy @@ -1,5 +1,3 @@ -metadata suppressions = [{"ids": ["UnstableFeature"], "shapes": ["aws.iotjobs#JobDocument"]}] - namespace aws.iotjobs use smithy.mqtt#mqttJson @@ -365,6 +363,7 @@ structure JobExecutionState { versionNumber: smithy.api#Integer, } +@suppress(["UnstableFeature"]) document JobDocument From 1c92a0ddd529ddc913b085a40fbcbcce87ac3e75 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Thu, 23 Apr 2020 16:27:59 -0700 Subject: [PATCH 2/2] Move validation into core spec This commit moves validation concepts into the core Smithy spec while leaving more specific linters in a linters guide. --- CHANGELOG.md | 3 + docs/source/guides/index.rst | 1 + .../model-linters.rst} | 573 +++++++----------- .../source/spec/core/documentation-traits.rst | 2 +- docs/source/spec/core/index.rst | 1 + docs/source/spec/core/model-validation.rst | 442 ++++++++++++++ docs/source/spec/index.rst | 1 - .../smithy/model/loader/ModelValidator.java | 16 +- .../EmitEachSelectorValidator.java | 5 +- .../EmitNoneSelectorValidator.java | 6 +- ...n.smithy.model.validation.ValidatorService | 4 +- .../smithy/model/loader/prelude-traits.smithy | 2 +- 12 files changed, 674 insertions(+), 382 deletions(-) rename docs/source/{spec/validation.rst => guides/model-linters.rst} (51%) create mode 100644 docs/source/spec/core/model-validation.rst rename smithy-model/src/main/java/software/amazon/smithy/model/validation/{ => linters}/EmitEachSelectorValidator.java (89%) rename smithy-model/src/main/java/software/amazon/smithy/model/validation/{ => linters}/EmitNoneSelectorValidator.java (90%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1743db29bff..410a6d31804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,7 +74,10 @@ keys, was added. #### Validation updates * Services must now contain a closure of shapes that have case-insensitively unique names. [BC] ([#337](https://github.com/awslabs/smithy/pull/337)) +* `suppressions` has been updated to now only suppress validation events that occur for an entire namespace or across + the entire model. The `@suppress` trait was added to suppress validation events for a specific shape. [BC] ([#397](https://github.com/awslabs/smithy/pull/397)). * The `UnreferencedShape` validator has moved to `smithy-model` and is now always run. [BC] ([#319](https://github.com/awslabs/smithy/pull/319)) +* `EmitEachSelector` and `EmitNoneSelector` were moved from `smithy-linters` into `smithy-model`. #### JSON Schema conversion diff --git a/docs/source/guides/index.rst b/docs/source/guides/index.rst index 9ff0bf6363c..95b7c9b1598 100644 --- a/docs/source/guides/index.rst +++ b/docs/source/guides/index.rst @@ -8,6 +8,7 @@ Smithy Guides :maxdepth: 2 building-models/index + model-linters evolving-models style-guide converting-to-openapi diff --git a/docs/source/spec/validation.rst b/docs/source/guides/model-linters.rst similarity index 51% rename from docs/source/spec/validation.rst rename to docs/source/guides/model-linters.rst index 3959ceda094..628238bda5b 100644 --- a/docs/source/spec/validation.rst +++ b/docs/source/guides/model-linters.rst @@ -1,12 +1,8 @@ -.. _validation: +============= +Model linters +============= -================ -Model Validation -================ - -This specification defines a customizable validation system for Smithy -models that can be used by API designers and organizations to ensure that -their APIs adhere to their own standards and best practices. +This guide describes how to apply optional linters to a Smithy model. .. contents:: Table of contents :depth: 2 @@ -14,232 +10,43 @@ their APIs adhere to their own standards and best practices. :backlinks: none -Introduction -============ - -APIs require a great deal of care and discipline to ensure that they provide -a coherent interface to customers, particularly after an API is released and -new features are added. This specification defines metadata that is used to -validate a model against configurable validator definitions, ensuring that -developers adhere to an organization's API standards. - -Tools like Checkstyle and Findbugs help to ensure that developers avoid common -bugs and pitfalls when writing code. This is a very powerful concept, -particularly for developers that are new to a programming language. This -concept is even more powerful when teams use the configurability of these -tools to communicate the coding standards of an organization and automate -their enforcement. This validation standard allows the same level of -conformity and rigor to be applied to Smithy models and API definitions. - - -.. _validator-definition: - -Validators -========== - -The ``validators`` metadata property contains an array of validator -objects that are used to constrain a model. Each object in the -``validators`` array supports the following properties: - -.. list-table:: - :header-rows: 1 - :widths: 20 20 60 - - * - Property - - Type - - Description - * - name - - ``string`` - - **Required**. The name of the validator to apply. This name is used in - implementations to find and configure the appropriate validator - implementation. Validators only take effect if a Smithy processor - implements the validator. - * - id - - ``string`` - - Defines a custom identifier for the validator. - - Multiple instances of a single validator can be configured for a model. - Providing an ``id`` allows suppressions to suppress a specific instance - of a validator. - - If ``id`` is not specified, it will default to the ``name`` property of - the validator definition. - * - message - - ``string`` - - Provides a custom message to use when emitting validation events. The - special ``{super}`` string can be added to a custom message to inject - the original error message of the validation event into the custom - message. - * - severity - - ``string`` - - Provides a custom :ref:`severity ` level to use - when a validation event occurs. If no severity is provided, then the - default severity of the validator is used. - - .. note:: - - The severity of user-defined validators cannot be set to ERROR. - * - namespaces - - [ ``string`` ] - - Provides a list of the namespaces that are targeted by the validator. - The validator will ignore any validation events encountered that are - not specific to the given namespaces. - * - selector - - ``string`` - - A valid :ref:`selector ` that causes the validator to only - validate shapes that match the selector. The validator will ignore any - validation events encountered that do not satisfy the selector. - * - configuration - - ``object`` - - Object that provides validator configuration. The available properties - are defined by each validator. Validators MAY require that specific - configuration properties are provided. - -The following Smithy document applies a custom validator named "SomeValidator": - -.. code-block:: smithy - - metadata validators = [ - { - // The name of the validator. - name: "SomeValidator", - // Uses a custom event ID for each validation event emitted. - id: "CustomEventId", - // Uses a custom message that also includes the default message. - message: "My custom message name. {super}", - // Applies the rule only to the following namespaces. - namespaces: ["foo.baz", "bar.qux"], - // The following properties are specific to the validator. - configuration: { - "someProperty": "foo", - } - } - ] - - -.. _missing-validators: - -Missing validators ------------------- - -If a Smithy implementation does not have an implementation for a specific -validator by name, the Smithy implementation MUST emit a WARNING validation -event with an event ID that is the concatenation of ``UnknownValidator.`` and -the name property of the validator that could not be found. For example, given -a custom validator that could not be found named ``Foo``, the implementation -MUST emit a validation event with an event ID of ``UnknownValidator.Foo`` and -a severity of WARNING. - - -.. _severity-definition: - -Severity -======== - -When a model is in violation of a validator, a *validation event* is emitted. -This validation event contains metadata about the violation, including the -optional shape that was in violation, the source location of the violation, -the validator ID, and the severity of the violation. *Severity* is used -to define the importance or impact of a violation. - -**ERROR** - Indicates that something is structurally wrong with the model and cannot - be suppressed. - - Validation events with a severity of ERROR are reserved for enforcing that - models adhere to the Smithy specification. Validators cannot emit a - validation event with a severity of ERROR. - -**DANGER** - Indicates that something is very likely wrong with the model. Unsuppressed - DANGER validation events indicate that a model is invalid. - -**WARNING** - Indicates that something might be wrong with the model. - -**NOTE** - Informational message that does not imply anything is wrong with the model. - - -.. _suppression-definition: - -Suppressions -============ +---------------- +Linting overview +---------------- -The ``suppressions`` metadata property contains an array of -suppression objects. Suppressions are used to suppress specific validation -events. +A *linter* is a special kind of :ref:`model validator ` +that is configurable. Linter implementations are found in code. The +`smithy-linters`_ package in Maven Central contains several linters that +can be used to apply additional validation to Smithy models. -.. note:: +The following example adds ``smithy-linters`` as a Gradle dependency +to a ``build.gradle.kts`` file: - Validation events with a severity of ``ERROR`` cannot be suppressed. - -Each suppression object in the ``suppressions`` array supports the -following properties: - -.. list-table:: - :header-rows: 1 - :widths: 20 20 60 - - * - Property - - Type - - Description - * - ids - - [ ``string`` ] - - **Required**. An array of validator event IDs to suppress. One or more - event IDs MUST be provided. A value of ``*`` MAY be provided in order - to suppress all validation event IDs (e.g., ``["*"]``). - * - shapes - - [ ``string`` ] - - A array of absolute :ref:`shape IDs ` to suppress. An entire - namespace can be suppressed by suffixing a namespace name with ``#``. - For example, ``foo.baz#`` can be used to suppress all validation events - on shapes in the "foo.baz" namespace. - * - reason - - ``string`` - - Provides a reason for the suppression. - -One or more entries from the ``ids`` list and one or more entries from the -``shapes`` list (if provided) MUST match in order for a validation event to be -suppressed. - -An example suppression for the "UnreferencedShape" validator: +.. tabs:: -.. code-block:: smithy + .. code-tab:: kotlin - metadata suppressions = [ - { - // The list of rules to suppress. - ids: ["UnreferencedShape"], - // The optional list of shapes that are suppressed. - shapes: ["foo.baz#SomeShape/members/someMemberName"], - // The optional reason that the rule is suppressed. - reason: "This shape is used for code generation." + dependencies { + implementation("software.amazon.smithy:smithy-model:1.0.0") + implementation("software.amazon.smithy:smithy-linters:1.0.0") } - ] - -An example suppression that suppresses all validation events for all shapes -within a specific namespace: -.. code-block:: smithy +After the dependency is added and available on the Java classpath, validators +defined in the package and registered using `Java SPI`_ are available for +use in Smithy models. - metadata suppressions = [ - { - ids: ["*"], - shapes: ["smithy.testing#"], - reason: "smithy.testing is used only for testing" - } - ] +----------------------------- +Linters in ``smithy-linters`` +----------------------------- -Naming validators -================= +The ``smithy-linters`` package defines the following linters. .. _AbbreviationName: AbbreviationName ----------------- +================ Validates that shape names and member names do not represent abbreviations with all uppercase letters. For example, instead of using "XMLRequest" or @@ -272,13 +79,17 @@ Example: .. code-block:: smithy - metadata validators = [{name: "AbbreviationName"}] + $version: "1.0.0" + + metadata validators = [ + {name: "AbbreviationName"} + ] .. _CamelCase: CamelCase ---------- +========= Validates that shape names and member names adhere to a consistent style of camel casing. By default, this validator will ensure that shape names use @@ -310,13 +121,17 @@ Example: .. code-block:: smithy - metadata validators = [{name: "CamelCase"}] + $version: "1.0.0" + + metadata validators = [ + {name: "CamelCase"} + ] .. _ReservedWords: ReservedWords -------------- +============= Validates that shape names and member names do not match a configured set of reserved words. @@ -368,6 +183,8 @@ Example: .. code-block:: smithy + $version: "1.0.0" + metadata validators = [{ id: "FooReservedWords" name: "ReservedWords", @@ -385,7 +202,7 @@ Example: .. _reserved-words-wildcards: Wildcards in ReservedWords -~~~~~~~~~~~~~~~~~~~~~~~~~~ +-------------------------- The ReservedWords validator allows leading and trailing wildcard characters to be specified. @@ -471,7 +288,7 @@ be specified. .. _StandardOperationVerb: StandardOperationVerb ---------------------- +===================== Looks at each operation shape name and determines if the first word in the operation shape name is one of the defined standard verbs or if it is a verb @@ -521,6 +338,8 @@ Example: .. code-block:: smithy + $version: "1.0.0" + metadata validators = [{ name: "StandardOperationVerb", configuration: { @@ -537,7 +356,7 @@ Example: .. _StutteredShapeName: StutteredShapeName ------------------- +================== Validators that :ref:`structure` member names and :ref:`union` member names do not stutter their shape names. @@ -553,13 +372,10 @@ Default severity ``WARNING`` -Best practice validators -======================== - .. _InputOutputStructureReuse: InputOutputStructureReuse -------------------------- +========================= Detects when a structure is used as both input and output or if a structure is referenced as the input or output for multiple operations. @@ -583,7 +399,7 @@ Default severity .. _MissingPaginatedTrait: MissingPaginatedTrait ---------------------- +===================== Checks for operations that look like they should be paginated but do not have the :ref:`paginated-trait`. @@ -636,17 +452,15 @@ Example: .. code-block:: smithy - metadata validators = [{name: "MissingPaginatedTrait"}] - - -Modeling validators -=================== + metadata validators = [ + {name: "MissingPaginatedTrait"} + ] .. _ShouldHaveUsedTimestamp: ShouldHaveUsedTimestamp ------------------------ +======================= Looks for shapes that likely represent time, but that do not use a timestamp shape. @@ -696,174 +510,199 @@ Configuration represent time. -.. _UnreferencedShape: +------------------------- +Writing custom validators +------------------------- -UnreferencedShape ------------------ +Custom validators can be written in Java to apply more advanced model validation. +Writing a custom validator involves writing an implementation of a +Smithy validator in Java, creating a JAR, and making the JAR available on the +classpath. -Looks for shapes that are not connected to from any service shape within -the model. +Custom validators are implementations of the +``software.amazon.smithy.model.validation.Validator`` interface. Most +validators should extend from ``software.amazon.smithy.model.validation.AbstractValidator``. -Rationale - Unreferenced shapes are good candidates for removal from a model. +The following linter emits a ``ValidationEvent`` for every shape in the +model that is not documented. -Default severity - ``NOTE`` +.. code-block:: java + package com.example.mypackage; -Misc validators -=============== + import java.util.List; + import java.util.stream.Collectors; + import software.amazon.smithy.model.Model; + import software.amazon.smithy.model.traits.DocumentationTrait; + import software.amazon.smithy.model.validation.AbstractValidator; + import software.amazon.smithy.model.validation.ValidationEvent; -.. _EmitEachSelector: + public class DocumentationValidator extends AbstractValidator { + @Override + public List validate(Model model) { + return model.shapes() + .filter(shape -> !shape.hasTrait(DocumentationTrait.class)) + .map(shape -> error(shape, "This shape is not documented!")) + .collect(Collectors.toList()); + } + } -EmitEachSelector ----------------- +Validators need to be registered as Java service providers. Add the following +class name to a file named ``software.amazon.smithy.model.validation.Validator`` +found in the ``src/main/resources/META-INF/services`` directory of a standard Gradle +Java package: -Emits a validation event for each shape that matches the given -:ref:`selector `. +.. code-block:: none -Rationale - Detecting shapes that violate a validation rule using customizable - validators allows organizations to create custom Smithy validators without - needing to write code. + com.example.mypackage.DocumentationValidator -Default severity - ``DANGER`` +When added to the classpath (typically as a dependency of a published JAR), +the custom validator is automatically applied to a model each time the +model is loaded. -Configuration - .. list-table:: - :header-rows: 1 - :widths: 20 20 60 - * - Property - - Type - - Description - * - selector - - ``string`` - - **Required**. A valid :ref:`selector `. Each shape in - the model that is returned from the selector with emit a validation - event. +---------------------- +Writing custom Linters +---------------------- -Example: +Like custom validators, custom linters can be written in Java to apply more +advanced model validation. -The following example detects if a shape is missing documentation with the -following constraints: +Custom linters are implementations of the +``software.amazon.smithy.model.validation.Validator`` interface. Because +linters are configurable, they are created using an implementation of the +``software.amazon.smithy.model.validation.ValidatorService`` interface. -- Shapes that have the documentation trait are excluded. -- Members that target shapes that have the documentation trait are excluded. -- Simple types are excluded. -- List and map members are excluded. +The following validator emits a ``ValidationEvent`` for every shape in the +model that has documentation that contains a forbidden string. -.. code-block:: smithy +.. code-block:: java - metadata validators = [{ - name: "EmitEachSelector", - id: "MissingDocumentation", - message: "This shape is missing documentation" - configuration: { - selector: """ - :not([trait|documentation]) - :not(simpleType) - :not(member:of(:is(list, map))) - :not(:test(member > [trait|documentation]))""" - } - }] + package com.example.mypackage; -The following example emits a validation event for each structure referenced as -input/output that has a shape name that does not case-insensitively end with -"Input"/"Output": + import java.util.List; + import java.util.Optional; + import java.util.stream.Collectors; + import java.util.stream.Stream; + import software.amazon.smithy.model.Model; + import software.amazon.smithy.model.node.NodeMapper; + import software.amazon.smithy.model.shapes.Shape; + import software.amazon.smithy.model.traits.DocumentationTrait; + import software.amazon.smithy.model.validation.AbstractValidator; + import software.amazon.smithy.model.validation.ValidationEvent; + import software.amazon.smithy.model.validation.ValidatorService; -.. code-block:: smithy + public class ForbiddenDocumentationValidator extends AbstractValidator { - metadata validators = [ - { - name: "EmitEachSelector", - id: "OperationInputName", - message: "This shape is referenced as input but the name does not end with 'Input'", - configuration: { - selector: "operation -[input]-> :not([id|name$=Input i])" + /** + * ForbiddenDocumentation configuration settings. + */ + public static final class Config { + private List forbid; + + public List getForbid() { + return forbid; } - }, - { - name: "EmitEachSelector", - id: "OperationOutputName", - message: "This shape is referenced as output but the name does not end with 'Output'", - configuration: { - selector: "operation -[output]-> :not([id|name$=Output i])" + + public void setForbid(List forbid) { + this.forbid = forbid; } } - ] -The following example emits a validation event for each operation referenced -as lifecycle 'read' or 'delete' that has a shape name that does not start with -"Get" or "Delete": + // Does the actual work of converting metadata found in a Smithy + // model into an actual implementation of a Validator. + public static final class Provider extends ValidatorService.Provider { + public Provider() { + super(ForbiddenDocumentationValidator.class, configuration -> { + // Deserialize the Node value into the Config POJO. + NodeMapper mapper = new NodeMapper(); + ForbiddenDocumentationValidator.Config config = mapper.deserialize(configuration, Config.class); + return new ForbiddenDocumentationValidator(config); + }); + } + } -.. code-block:: smithy + private final List forbid; - metadata validators = [ - { - name: "EmitEachSelector", - id: "LifecycleGetName", - message: "Lifecycle 'read' operation shape names should start with 'Get'", - configuration: { - selector: "operation [read]-> :not([id|name^=Get i])" - } - }, - { - name: "EmitEachSelector", - id: "LifecycleDeleteName", - message: "Lifecycle 'delete' operation shape names should start with 'Delete'", - configuration: { - selector: "operation -[delete]-> :not([id|name^=Delete i])" + // The constructor is private since the validator is only intended to + // be created when loading a model via the Provider class. + private ForbiddenDocumentationValidator(Config config) { + this.forbid = config.forbid; + } + + @Override + public List validate(Model model) { + // Find every shape that violates the linter and return a list + // of ValidationEvents. + return model.shapes() + .filter(shape -> shape.hasTrait(DocumentationTrait.class)) + .flatMap(shape -> validateShape(shape).map(Stream::of).orElseGet(Stream::empty)) + .collect(Collectors.toList()); + } + + private Optional validateShape(Shape shape) { + // Grab the trait by type. + DocumentationTrait trait = shape.expectTrait(DocumentationTrait.class); + String docString = trait.getValue(); + + for (String text : forbid) { + if (docString.contains(text)) { + // Emit an event that points at the location of the trait + // and associates the warning with the shape. + return Optional.of(warning(shape, trait, "Documentation uses forbidden text: " + text)); + } } + + return Optional.empty(); } - ] + } +Configurable linters need to be registered as Java service providers. Add the following +class name to a file named ``software.amazon.smithy.model.validation.ValidatorService`` +found in the ``src/main/resources/META-INF/services`` directory of a standard Gradle +Java package: -.. _EmitNoneSelector: +.. code-block:: none -EmitNoneSelector ----------------- + com.example.mypackage.ForbiddenDocumentationValidator$Provider -Emits a validation event if no shape in the model matches the given -:ref:`selector `. +When added to the classpath (typically as a dependency of a published JAR), +the custom validator is available to be used as a validator. The following +example warns each time the word "meow" appears in documentation: -Rationale - Detecting the omission of a specific trait, pattern, or other requirement - can help developers to remember to apply constraint traits, documentation, - etc. +.. code-block:: smithy -Default severity - ``DANGER`` + $version: "1.0.0" -Configuration - .. list-table:: - :header-rows: 1 - :widths: 20 20 60 + metadata validators = [ + { + name: "ForbiddenDocumentation", + configuration: { + forbid: ["meow"] + } + } + ] - * - Property - - Type - - Description - * - selector - - ``string`` - - **Required**. A valid :ref:`selector `. If no shape - in the model is returned by the selector, then a validation event - is emitted. +.. tip:: -Example: + The :ref:`EmitEachSelector` can get you pretty far without needing to + write any Java code. For example, the above linter can be implemented + using the following Smithy model: -The following example detects if the model does not contain any constraint -traits. + .. code-block:: smithy -.. code-block:: smithy + $version: "1.0.0" - metadata validators = [{ - name: "EmitNoneSelector", - id: "MissingConstraintTraits", - message: """ - No instances of the enum, pattern, length, or range trait - could be found. Did you forget to apply these traits?""", - configuration: { - selector: ":is([trait|enum], [trait|pattern], [trait|length], [trait|range])", - } - }] + metadata validators = [ + { + name: "EmitEachSelector", + id: "ForbiddenDocumentation", + message: "Documentation uses forbidden text", + configuration: { + selector: "[trait|documentation*='meow']" + } + } + ] + +.. _smithy-linters: https://search.maven.org/artifact/software.amazon.smithy/smithy-linters +.. _Java SPI: https://docs.oracle.com/javase/tutorial/sound/SPI-intro.html diff --git a/docs/source/spec/core/documentation-traits.rst b/docs/source/spec/core/documentation-traits.rst index 45ffe51b0a1..8218ded3939 100644 --- a/docs/source/spec/core/documentation-traits.rst +++ b/docs/source/spec/core/documentation-traits.rst @@ -320,7 +320,7 @@ Summary Trait selector ``*`` Value type - ``list`` + ``[string]`` Tools can use these tags to filter shapes that should not be visible for a particular consumer of a model. The string values that can be provided to the diff --git a/docs/source/spec/core/index.rst b/docs/source/spec/core/index.rst index 27516a11998..8572a1a371b 100644 --- a/docs/source/spec/core/index.rst +++ b/docs/source/spec/core/index.rst @@ -26,5 +26,6 @@ Smithy specification endpoint-traits selectors model-metadata + model-validation merging-models json-ast diff --git a/docs/source/spec/core/model-validation.rst b/docs/source/spec/core/model-validation.rst new file mode 100644 index 00000000000..c56ffb2e3a7 --- /dev/null +++ b/docs/source/spec/core/model-validation.rst @@ -0,0 +1,442 @@ +.. _validation: + +================ +Model validation +================ + +Smithy provides a customizable validation system that can be used by +API designers and organizations to ensure that their APIs adhere to their +own standards and best practices. + +.. contents:: Table of contents + :depth: 2 + :local: + :backlinks: none + + +------------ +Introduction +------------ + +APIs require a great deal of care and discipline to ensure that they provide +a coherent interface to customers, particularly after an API is released and +new features are added. This specification defines metadata that is used to +validate a model against configurable validator definitions, ensuring that +developers adhere to an organization's API standards. + +Tools like Checkstyle and Findbugs help to ensure that developers avoid common +bugs and pitfalls when writing code. This is a very powerful concept, +particularly for developers that are new to a programming language. This +concept is even more powerful when teams use the configurability of these +tools to communicate the coding standards of an organization and automate +their enforcement. This validation standard allows the same level of +conformity and rigor to be applied to Smithy models and API definitions. + + +.. _validator-definition: + +---------- +Validators +---------- + +The ``validators`` metadata property contains an array of validator +objects that are used to constrain a model. Each object in the +``validators`` array supports the following properties: + +.. list-table:: + :header-rows: 1 + :widths: 20 20 60 + + * - Property + - Type + - Description + * - name + - ``string`` + - **Required**. The name of the validator to apply. This name is used in + implementations to find and configure the appropriate validator + implementation. Validators only take effect if a Smithy processor + implements the validator. + * - id + - ``string`` + - Defines a custom identifier for the validator. + + Multiple instances of a single validator can be configured for a model. + Providing an ``id`` allows suppressions to suppress a specific instance + of a validator. + + If ``id`` is not specified, it will default to the ``name`` property of + the validator definition. + * - message + - ``string`` + - Provides a custom message to use when emitting validation events. The + special ``{super}`` string can be added to a custom message to inject + the original error message of the validation event into the custom + message. + * - severity + - ``string`` + - Provides a custom :ref:`severity ` level to use + when a validation event occurs. If no severity is provided, then the + default severity of the validator is used. + + .. note:: + + The severity of user-defined validators cannot be set to ERROR. + * - namespaces + - [ ``string`` ] + - Provides a list of the namespaces that are targeted by the validator. + The validator will ignore any validation events encountered that are + not specific to the given namespaces. + * - selector + - ``string`` + - A valid :ref:`selector ` that causes the validator to only + validate shapes that match the selector. The validator will ignore any + validation events encountered that do not satisfy the selector. + * - configuration + - ``object`` + - Object that provides validator configuration. The available properties + are defined by each validator. Validators MAY require that specific + configuration properties are provided. + +The following Smithy document applies a custom validator named "SomeValidator": + +.. code-block:: smithy + + $version: "1.0.0" + + metadata validators = [ + { + // The name of the validator. + name: "SomeValidator", + // Uses a custom event ID for each validation event emitted. + id: "CustomEventId", + // Uses a custom message that also includes the default message. + message: "My custom message name. {super}", + // Applies the rule only to the following namespaces. + namespaces: ["foo.baz", "bar.qux"], + // The following properties are specific to the validator. + configuration: { + "someProperty": "foo", + } + } + ] + + namespace smithy.example + + // shapes are defined here... + + +.. _missing-validators: + +Missing validators +================== + +The actual implementation of a validator is defined in code and is +not defined in the Smithy model itself. If a Smithy implementation does not +have an implementation for a specific validator by name, the Smithy +implementation MUST emit a WARNING validation event with an event ID that is +the concatenation of ``UnknownValidator_`` and the ``name`` property of the +validator that could not be found. For example, given a custom validator +that could not be found named ``Foo``, the implementation MUST emit a +validation event with an event ID of ``UnknownValidator_Foo`` and a +severity of WARNING. + + +.. _severity-definition: + +-------- +Severity +-------- + +When a model is in violation of a validator, a *validation event* is emitted. +This validation event contains metadata about the violation, including the +optional shape that was in violation, the validator ID, and the severity of +the violation. *Severity* is used to define the importance or impact of +a violation. + +**ERROR** + Indicates that something is structurally wrong with the model and cannot + be suppressed. + + Validation events with a severity of ERROR are reserved for enforcing that + models adhere to the Smithy specification. Validators cannot emit a + validation event with a severity of ERROR. + +**DANGER** + Indicates that something is very likely wrong with the model. Unsuppressed + DANGER validation events indicate that a model is invalid. + +**WARNING** + Indicates that something might be wrong with the model. + +**NOTE** + Informational message that does not imply anything is wrong with the model. + + +.. _suppression-definition: + +------------ +Suppressions +------------ + +Suppressions are used to suppress specific validation events. +Suppressions are created using the :ref:`suppression-trait` and +:ref:`suppressions metadata `. + + +.. _suppression-trait: + +``suppression`` trait +===================== + +Summary + The suppression trait is used to suppress validation events(s) for a + specific shape. Each value in the ``suppression`` trait is a + validation event ID to suppress for the shape. +Trait selector + ``*`` +Value type + ``[string]`` + +The following example suppresses the ``Foo`` and ``Bar`` validation events +for the ``smithy.example#MyString`` shape: + +.. tabs:: + + .. code-tab:: smithy + + $version: "1.0.0" + + namespace smithy.example + + @suppression(["Foo", "Bar"]) + string MyString + + +.. _suppressions-metadata: + +Suppression metadata +==================== + +The ``suppressions`` metadata property contains an array of suppression objects +that are used to suppress validation events for the entire model or for an +entire namespace. + +Each suppression object in the ``suppressions`` array supports the +following properties: + +.. list-table:: + :header-rows: 1 + :widths: 20 20 60 + + * - Property + - Type + - Description + * - id + - ``string`` + - **Required**. The validation event ID to suppress. + * - namespace + - ``string`` + - **Required**. The validation event is only suppressed if it matches the + supplied namespace. A value of ``*`` can be provided to match any namespace. + ``*`` is useful for suppressing validation events that are not bound to any + specific shape. + * - reason + - ``string`` + - Provides an optional reason for the suppression. + +The following example suppresses all validation events on shapes +in the ``foo.baz`` namespace with an ID of ``UnreferencedShape``: + +.. code-block:: smithy + + $version: "1.0.0" + + metadata suppressions = [ + { + id: "UnreferencedShape", + namespace: "foo.baz", + reason: "This is a test namespace." + } + ] + +The following example suppresses all validation events with an +ID of ``OverlyBroadValidator``: + +.. code-block:: smithy + + $version: "1.0.0" + + metadata suppressions = [ + { + id: "OverlyBroadValidator", + namespace: "*" + } + ] + + +------------------- +Built-in validators +------------------- + +Smithy provides built-in validators that can be used in any model in +the ``validators`` metadata property. Implementations MAY support +additional validators. + + +.. _EmitEachSelector: + +EmitEachSelector +================ + +Emits a validation event for each shape that matches the given +:ref:`selector `. + +Rationale + Detecting shapes that violate a validation rule using customizable + validators allows organizations to create custom Smithy validators + without needing to write code. + +Default severity + ``DANGER`` + +Configuration + .. list-table:: + :header-rows: 1 + :widths: 20 20 60 + + * - Property + - Type + - Description + * - selector + - ``string`` + - **Required**. A valid :ref:`selector `. Each shape in + the model that is returned from the selector with emit a validation + event. + +The following example detects if a shape is missing documentation with the +following constraints: + +- Shapes that have the documentation trait are excluded. +- Members that target shapes that have the documentation trait are excluded. +- Simple types are excluded. +- List and map members are excluded. + +.. code-block:: smithy + + $version: "1.0.0" + + metadata validators = [{ + name: "EmitEachSelector", + id: "MissingDocumentation", + message: "This shape is missing documentation" + configuration: { + selector: """ + :not([trait|documentation]) + :not(simpleType) + :not(member:of(:is(list, map))) + :not(:test(member > [trait|documentation]))""" + } + }] + +The following example emits a validation event for each structure referenced as +input/output that has a shape name that does not case-insensitively end with +"Input"/"Output": + +.. code-block:: smithy + + $version: "1.0.0" + + metadata validators = [ + { + name: "EmitEachSelector", + id: "OperationInputName", + message: "This shape is referenced as input but the name does not end with 'Input'", + configuration: { + selector: "operation -[input]-> :not([id|name$=Input i])" + } + }, + { + name: "EmitEachSelector", + id: "OperationOutputName", + message: "This shape is referenced as output but the name does not end with 'Output'", + configuration: { + selector: "operation -[output]-> :not([id|name$=Output i])" + } + } + ] + +The following example emits a validation event for each operation referenced +as lifecycle 'read' or 'delete' that has a shape name that does not start with +"Get" or "Delete": + +.. code-block:: smithy + + $version: "1.0.0" + + metadata validators = [ + { + name: "EmitEachSelector", + id: "LifecycleGetName", + message: "Lifecycle 'read' operation shape names should start with 'Get'", + configuration: { + selector: "operation [read]-> :not([id|name^=Get i])" + } + }, + { + name: "EmitEachSelector", + id: "LifecycleDeleteName", + message: "Lifecycle 'delete' operation shape names should start with 'Delete'", + configuration: { + selector: "operation -[delete]-> :not([id|name^=Delete i])" + } + } + ] + + +.. _EmitNoneSelector: + +EmitNoneSelector +================ + +Emits a validation event if no shape in the model matches the given +:ref:`selector `. + +Rationale + Detecting the omission of a specific trait, pattern, or other requirement + can help developers to remember to apply constraint traits, documentation, + etc. + +Default severity + ``DANGER`` + +Configuration + .. list-table:: + :header-rows: 1 + :widths: 20 20 60 + + * - Property + - Type + - Description + * - selector + - ``string`` + - **Required**. A valid :ref:`selector `. If no shape + in the model is returned by the selector, then a validation event + is emitted. + +The following example detects if the model does not contain any constraint +traits. + +.. code-block:: smithy + + $version: "1.0.0" + + metadata validators = [{ + name: "EmitNoneSelector", + id: "MissingConstraintTraits", + message: """ + No instances of the enum, pattern, length, or range trait + could be found. Did you forget to apply these traits?""", + configuration: { + selector: ":is([trait|enum], [trait|pattern], [trait|length], [trait|range])", + } + }] diff --git a/docs/source/spec/index.rst b/docs/source/spec/index.rst index 2645e41b7b8..9f7ad0930ef 100644 --- a/docs/source/spec/index.rst +++ b/docs/source/spec/index.rst @@ -30,7 +30,6 @@ Additional specifications :maxdepth: 1 :caption: Defines additional specifications. - validation http-protocol-compliance-tests mqtt 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 66f438becde..7dbba6e624b 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 @@ -205,14 +205,6 @@ private String resolveReason(ValidationEvent event) { } private Optional matchSuppression(Shape shape, String eventId) { - // Check namespace-wide suppressions. - if (namespaceSuppressions.containsKey(eventId)) { - Map 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)) { @@ -222,6 +214,14 @@ private Optional matchSuppression(Shape shape, String eventId) { } } + // Check namespace-wide suppressions. + if (namespaceSuppressions.containsKey(eventId)) { + Map namespaces = namespaceSuppressions.get(eventId); + if (namespaces.containsKey(shape.getId().getNamespace())) { + return Optional.of(namespaces.get(shape.getId().getNamespace())); + } + } + return Optional.empty(); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitEachSelectorValidator.java similarity index 89% rename from smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.java rename to smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitEachSelectorValidator.java index d35d2da18bc..5adf8faa997 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitEachSelectorValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitEachSelectorValidator.java @@ -13,13 +13,16 @@ * permissions and limitations under the License. */ -package software.amazon.smithy.model.validation; +package software.amazon.smithy.model.validation.linters; import java.util.List; import java.util.stream.Collectors; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.selector.Selector; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidatorService; /** * Emits a validation event for each shape that matches a selector. diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitNoneSelectorValidator.java similarity index 90% rename from smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.java rename to smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitNoneSelectorValidator.java index 641ad8dfd8e..a48427b4d32 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/EmitNoneSelectorValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/linters/EmitNoneSelectorValidator.java @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package software.amazon.smithy.model.validation; +package software.amazon.smithy.model.validation.linters; import java.util.List; import java.util.Set; @@ -23,6 +23,10 @@ import software.amazon.smithy.model.node.NodeMapper; import software.amazon.smithy.model.selector.Selector; import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.validation.AbstractValidator; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.ValidatorService; import software.amazon.smithy.utils.ListUtils; /** diff --git a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService index 5c4c75c5bbd..c9fc2570d6f 100644 --- a/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService +++ b/smithy-model/src/main/resources/META-INF/services/software.amazon.smithy.model.validation.ValidatorService @@ -1,2 +1,2 @@ -software.amazon.smithy.model.validation.EmitEachSelectorValidator$Provider -software.amazon.smithy.model.validation.EmitNoneSelectorValidator$Provider +software.amazon.smithy.model.validation.linters.EmitEachSelectorValidator$Provider +software.amazon.smithy.model.validation.linters.EmitNoneSelectorValidator$Provider diff --git a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy index 4fe59b63c6e..d8643dcd27e 100644 --- a/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy +++ b/smithy-model/src/main/resources/software/amazon/smithy/model/loader/prelude-traits.smithy @@ -628,7 +628,7 @@ structure endpoint { @tags(["diff.error.const"]) structure hostLabel {} -/// Suppresses a validation event by ID for a given shape. +/// Suppresses validation events by ID for a given shape. @trait list suppress { @pattern("^[_a-zA-Z][A-Za-z0-9]*$")