From 2eef778f27e3cc3f6b0bab3239a1567e71955360 Mon Sep 17 00:00:00 2001 From: rchache <105247787+rchache@users.noreply.github.com> Date: Wed, 15 Feb 2023 17:18:19 -0500 Subject: [PATCH] Extend validation event ids to make them unique (#1527) * Extend validation event ids to make them unique * address comments, mostly to simplify some of the eventIds * optimize assembleFullEventId method * minor edits for style * unroll eventId varargs in AbstractValidator for performance * fix eventIds for Waiter violations * remove unused method overloads --- .../traits/HttpChecksumTraitValidator.java | 18 +- .../http-checksum-header-conflicts.errors | 12 +- .../InputOutputStructureReuseValidator.java | 9 +- .../linters/NoninclusiveTermsValidator.java | 105 +++--- .../input-output-structure-reuse.errors | 4 +- .../noninclusive-term-filter.errors | 26 +- .../model/validation/AbstractValidator.java | 347 +++++++++++++++++- .../validation/NodeValidationVisitor.java | 20 +- .../validation/testrunner/SmithyTestCase.java | 6 +- .../validators/EnumTraitValidator.java | 12 +- .../HttpBindingsMissingValidator.java | 2 +- .../HttpMethodSemanticsValidator.java | 50 ++- .../validators/OperationValidator.java | 2 +- .../validators/PaginatedTraitValidator.java | 35 +- .../validators/TargetValidator.java | 2 +- .../validators/UnstableTraitValidator.java | 8 +- .../amazon/smithy/model/ShapeMatcher.java | 2 +- .../annotation-trait-extra-properties.errors | 2 +- .../deprecated-shape/deprecated-shape.errors | 8 +- .../enums/enum-trait-validation.errors | 10 +- .../http-method-semantics-validator.errors | 14 +- .../http-method-semantics-validator.json | 6 + ...alid-multiple-operations-same-shape.errors | 4 +- .../paginated/paginated-deeply-nested.errors | 4 +- .../paginated/paginated-invalid.errors | 2 +- .../paginated/paginated-map-tokens.errors | 4 +- .../trait-value/extra-trait-property.errors | 2 +- .../unstable-trait-validator.errors | 2 +- .../waiters/WaiterMatcherValidator.java | 39 +- ...emits-danger-and-warning-typechecks.errors | 6 +- .../errorfiles/invalid-errorType.errors | 2 +- .../errorfiles/invalid-jmespath-syntax.errors | 2 +- .../errorfiles/invalid-return-types.errors | 8 +- 33 files changed, 581 insertions(+), 194 deletions(-) diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java index f8a87f64c55..047b38d6815 100644 --- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java +++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java @@ -185,11 +185,13 @@ private List validateHeaderConflicts(OperationShape operation, .map(HttpPrefixHeadersTrait::getValue) .ifPresent(headerPrefix -> { if (HttpChecksumTrait.CHECKSUM_PREFIX.startsWith(headerPrefix)) { + String memberName = member.getId().getName(); + String prefixString = headerPrefix.toLowerCase(Locale.US); events.add(danger(operation, format("The `httpPrefixHeaders` binding of `%s` uses" - + " the prefix `%s` that conflicts with the prefix `%s` used by the" - + " `httpChecksum` trait.", - member.getId().getName(), headerPrefix.toLowerCase(Locale.US), - HttpChecksumTrait.CHECKSUM_PREFIX))); + + " the prefix `%s` that conflicts with the prefix `%s` used by the" + + " `httpChecksum` trait.", + memberName, prefixString, HttpChecksumTrait.CHECKSUM_PREFIX), + "HttpPrefixHeaders", memberName, prefixString)); } }); @@ -200,10 +202,12 @@ private List validateHeaderConflicts(OperationShape operation, .map(HttpHeaderTrait::getValue) .ifPresent(headerName -> { if (headerName.startsWith(HttpChecksumTrait.CHECKSUM_PREFIX)) { + String memberName = member.getId().getName(); + String headerString = headerName.toLowerCase(Locale.US); events.add(warning(operation, format("The `httpHeader` binding of `%s` on `%s`" - + " starts with the prefix `%s` used by the `httpChecksum` trait.", - headerName.toLowerCase(Locale.US), member.getId().getName(), - HttpChecksumTrait.CHECKSUM_PREFIX))); + + " starts with the prefix `%s` used by the `httpChecksum` trait.", + headerString, memberName, HttpChecksumTrait.CHECKSUM_PREFIX), + "HttpHeader", memberName, headerString)); } }); } diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-header-conflicts.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-header-conflicts.errors index a5ad203e924..3176f0fe82f 100644 --- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-header-conflicts.errors +++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-header-conflicts.errors @@ -1,9 +1,9 @@ [SUPPRESSED] smithy.example#HeaderConflicts: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait [SUPPRESSED] smithy.example#HeadersConflicts: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait [SUPPRESSED] smithy.example#NoConflicts: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait -[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictError` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait -[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictsInput` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait -[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictsOutput` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait -[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictError` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait -[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictsInput` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait -[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictsOutput` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait +[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictError` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpHeader.HeaderConflictError.x-amz-checksum-crc32 +[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictsInput` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpHeader.HeaderConflictsInput.x-amz-checksum-crc32 +[WARNING] smithy.example#HeaderConflicts: The `httpHeader` binding of `x-amz-checksum-crc32` on `HeaderConflictsOutput` starts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpHeader.HeaderConflictsOutput.x-amz-checksum-crc32 +[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictError` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpPrefixHeaders.HeadersConflictError.x-amz-checksum- +[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictsInput` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpPrefixHeaders.HeadersConflictsInput.x-amz-checksum- +[DANGER] smithy.example#HeadersConflicts: The `httpPrefixHeaders` binding of `HeadersConflictsOutput` uses the prefix `x-amz-checksum-` that conflicts with the prefix `x-amz-checksum-` used by the `httpChecksum` trait. | HttpChecksumTrait.HttpPrefixHeaders.HeadersConflictsOutput.x-amz-checksum- diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/InputOutputStructureReuseValidator.java b/smithy-linters/src/main/java/software/amazon/smithy/linters/InputOutputStructureReuseValidator.java index 4281fb0c9b7..9bbc3b0010a 100644 --- a/smithy-linters/src/main/java/software/amazon/smithy/linters/InputOutputStructureReuseValidator.java +++ b/smithy-linters/src/main/java/software/amazon/smithy/linters/InputOutputStructureReuseValidator.java @@ -39,6 +39,9 @@ public Provider() { } } + private static final String INPUT = "Input"; + private static final String OUTPUT = "Output"; + @Override public List validate(Model model) { List events = new ArrayList<>(); @@ -62,13 +65,15 @@ private void validateInputOutputSet( "This structure is the input of `%s`, but it is not marked with the " + "@input trait. The @input trait gives operations more flexibility to " + "evolve their top-level input members in ways that would otherwise " - + "be backward incompatible.", operation.getId()))); + + "be backward incompatible.", operation.getId()), + INPUT, operation.getId().getName())); } if (!output.hasTrait(OutputTrait.class)) { events.add(warning(output, String.format( "This structure is the output of `%s`, but it is not marked with " - + "the @output trait.", operation.getId()))); + + "the @output trait.", operation.getId()), + OUTPUT, operation.getId().getName())); } } } diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/NoninclusiveTermsValidator.java b/smithy-linters/src/main/java/software/amazon/smithy/linters/NoninclusiveTermsValidator.java index 4e688004be3..5ea29032692 100644 --- a/smithy-linters/src/main/java/software/amazon/smithy/linters/NoninclusiveTermsValidator.java +++ b/smithy-linters/src/main/java/software/amazon/smithy/linters/NoninclusiveTermsValidator.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.stream.Collectors; import software.amazon.smithy.model.Model; @@ -58,6 +59,10 @@ public Provider() { } } + private static final String TRAIT = "Trait"; + private static final String SHAPE = "Shape"; + private static final String NAMESPACE = "Namespace"; + /** * NoninclusiveTermsValidator configuration. */ @@ -121,7 +126,7 @@ public List validate(Model model) { /** * Generates zero or more @see ValidationEvents and returns them in a collection. * - * @param occurrence text occurrence found in the body of the model + * @param instance text occurrence found in the body of the model */ private Collection getValidationEvents(TextInstance instance) { final Collection events = new ArrayList<>(); @@ -130,70 +135,72 @@ private Collection getValidationEvents(TextInstance instance) { final int startIndex = instance.getText().toLowerCase().indexOf(termLower); if (startIndex != -1) { final String matchedText = instance.getText().substring(startIndex, startIndex + termLower.length()); - switch (instance.getLocationType()) { - case NAMESPACE: - //Cannot use any warning() overloads because there is no shape associated with the event. - events.add(ValidationEvent.builder() - .sourceLocation(SourceLocation.none()) - .id(this.getClass().getSimpleName().replaceFirst("Validator$", "")) - .severity(Severity.WARNING) - .message(formatNonInclusiveTermsValidationMessage(termEntry, matchedText, instance)) - .build()); - break; - case APPLIED_TRAIT: - events.add(warning(instance.getShape(), - instance.getTrait().getSourceLocation(), - formatNonInclusiveTermsValidationMessage(termEntry, matchedText, instance))); - break; - case SHAPE: - default: - events.add(warning(instance.getShape(), - instance.getShape().getSourceLocation(), - formatNonInclusiveTermsValidationMessage(termEntry, matchedText, instance))); - } + events.add(constructValidationEvent(instance, termEntry.getValue(), matchedText)); } } return events; } - private static String formatNonInclusiveTermsValidationMessage( - Map.Entry> termEntry, - String matchedText, - TextInstance instance - ) { - final List caseCorrectedEntryValue = termEntry.getValue().stream() - .map(replacement -> Character.isUpperCase(matchedText.charAt(0)) - ? StringUtils.capitalize(replacement) - : StringUtils.uncapitalize(replacement)) - .collect(Collectors.toList()); - String replacementAddendum = !termEntry.getValue().isEmpty() - ? String.format(" Consider using one of the following terms instead: %s", - ValidationUtils.tickedList(caseCorrectedEntryValue)) - : ""; + private ValidationEvent constructValidationEvent(TextInstance instance, + List replacements, + String matchedText) { + String replacementAddendum = getReplacementAddendum(matchedText, replacements); switch (instance.getLocationType()) { - case SHAPE: - return String.format("%s shape uses a non-inclusive term `%s`.%s", - StringUtils.capitalize(instance.getShape().getType().toString()), - matchedText, replacementAddendum); case NAMESPACE: - return String.format("%s namespace uses a non-inclusive term `%s`.%s", - instance.getText(), matchedText, replacementAddendum); + //Cannot use any warning() overloads because there is no shape associated with the event. + return ValidationEvent.builder() + .severity(Severity.WARNING) + .sourceLocation(SourceLocation.none()) + .id(getName() + "." + NAMESPACE + "." + instance.getText() + + "." + matchedText.toLowerCase(Locale.US)) + .message(String.format("%s namespace uses a non-inclusive term `%s`.%s", + instance.getText(), matchedText, replacementAddendum)) + .build(); case APPLIED_TRAIT: + ValidationEvent validationEvent = + warning(instance.getShape(), instance.getTrait().getSourceLocation(), ""); + String idiomaticTraitName = Trait.getIdiomaticTraitName(instance.getTrait()); if (instance.getTraitPropertyPath().isEmpty()) { - return String.format("'%s' trait has a value that contains a non-inclusive term `%s`.%s", - Trait.getIdiomaticTraitName(instance.getTrait()), matchedText, - replacementAddendum); + return validationEvent.toBuilder() + .message(String.format("'%s' trait has a value that contains a non-inclusive term `%s`.%s", + idiomaticTraitName, matchedText, replacementAddendum)) + .id(getName() + "." + TRAIT + "." + + matchedText.toLowerCase(Locale.US) + "." + idiomaticTraitName) + .build(); } else { String valuePropertyPathFormatted = formatPropertyPath(instance.getTraitPropertyPath()); - return String.format("'%s' trait value at path {%s} contains a non-inclusive term `%s`.%s", - Trait.getIdiomaticTraitName(instance.getTrait()), valuePropertyPathFormatted, - matchedText, replacementAddendum); + return validationEvent.toBuilder() + .message(String.format( + "'%s' trait value at path {%s} contains a non-inclusive term `%s`.%s", + idiomaticTraitName, valuePropertyPathFormatted, matchedText, replacementAddendum)) + .id(getName() + "." + TRAIT + "." + matchedText.toLowerCase(Locale.US) + + "." + idiomaticTraitName + "." + valuePropertyPathFormatted) + .build(); } + case SHAPE: default: - throw new IllegalStateException(); + return warning(instance.getShape(), + instance.getShape().getSourceLocation(), + String.format("%s shape uses a non-inclusive term `%s`.%s", + StringUtils.capitalize(instance.getShape().getType().toString()), + matchedText, replacementAddendum), + SHAPE, matchedText.toLowerCase(Locale.US)); } } + private static String getReplacementAddendum(String matchedText, List replacements) { + List caseCorrectedEntryValue = replacements.stream() + .map(replacement -> Character.isUpperCase(matchedText.charAt(0)) + ? StringUtils.capitalize(replacement) + : StringUtils.uncapitalize(replacement)) + .collect(Collectors.toList()); + String replacementAddendum = !replacements.isEmpty() + ? String.format(" Consider using one of the following terms instead: %s", + ValidationUtils.tickedList(caseCorrectedEntryValue)) + : ""; + return replacementAddendum; + } + private static String formatPropertyPath(List traitPropertyPath) { return String.join("/", traitPropertyPath); } diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/input-output-structure-reuse.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/input-output-structure-reuse.errors index b8d4e86eaa0..630f04f8f10 100644 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/input-output-structure-reuse.errors +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/input-output-structure-reuse.errors @@ -1,2 +1,2 @@ -[WARNING] smithy.example#GetFooInput: This structure is the input of `smithy.example#GetFoo`, but it is not marked with the @input trait. The @input trait gives operations more flexibility to evolve their top-level input members in ways that would otherwise be backward incompatible. | InputOutputStructureReuse -[WARNING] smithy.example#GetFooOutput: This structure is the output of `smithy.example#GetFoo`, but it is not marked with the @output trait. | InputOutputStructureReuse +[WARNING] smithy.example#GetFooInput: This structure is the input of `smithy.example#GetFoo`, but it is not marked with the @input trait. The @input trait gives operations more flexibility to evolve their top-level input members in ways that would otherwise be backward incompatible. | InputOutputStructureReuse.Input.GetFoo +[WARNING] smithy.example#GetFooOutput: This structure is the output of `smithy.example#GetFoo`, but it is not marked with the @output trait. | InputOutputStructureReuse.Output.GetFoo diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors index b9df6a47cb7..50e535aeaee 100644 --- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors +++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors @@ -1,13 +1,13 @@ -[WARNING] ns.foo#MyMasterService: Service shape uses a non-inclusive term `Master`. Consider using one of the following terms instead: `Main`, `Parent`, `Primary` | NoninclusiveTerms -[WARNING] ns.foo#BlackListThings: Operation shape uses a non-inclusive term `BlackList`. Consider using one of the following terms instead: `DenyList` | NoninclusiveTerms -[WARNING] ns.foo#MyWhitelistTrait: Structure shape uses a non-inclusive term `Whitelist`. Consider using one of the following terms instead: `AllowList` | NoninclusiveTerms -[WARNING] ns.foo#AInput$master_member_name: Member shape uses a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms -[WARNING] ns.foo#NestedTraitStructure$master_key_violation: Member shape uses a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms -[WARNING] ns.foo#AInput$foo: 'ns.foo#MySimpleValueTrait' trait has a value that contains a non-inclusive term `slave`. Consider using one of the following terms instead: `child`, `clone`, `replica`, `secondary` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {document/foo/0/bar} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {nested/master_key_violation} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {string_value} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {document/whitelist_doc_key_violation_1} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {collection/2/key} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms -[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {collection/1/blacklist_key} contains a non-inclusive term `blacklist`. Consider using one of the following terms instead: `denyList` | NoninclusiveTerms -[WARNING] ns.foo#MyUnionTrait$int_whitelist: Member shape uses a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms +[WARNING] ns.foo#MyMasterService: Service shape uses a non-inclusive term `Master`. Consider using one of the following terms instead: `Main`, `Parent`, `Primary` | NoninclusiveTerms.Shape.master +[WARNING] ns.foo#BlackListThings: Operation shape uses a non-inclusive term `BlackList`. Consider using one of the following terms instead: `DenyList` | NoninclusiveTerms.Shape.blacklist +[WARNING] ns.foo#MyWhitelistTrait: Structure shape uses a non-inclusive term `Whitelist`. Consider using one of the following terms instead: `AllowList` | NoninclusiveTerms.Shape.whitelist +[WARNING] ns.foo#AInput$master_member_name: Member shape uses a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms.Shape.master +[WARNING] ns.foo#NestedTraitStructure$master_key_violation: Member shape uses a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms.Shape.master +[WARNING] ns.foo#AInput$foo: 'ns.foo#MySimpleValueTrait' trait has a value that contains a non-inclusive term `slave`. Consider using one of the following terms instead: `child`, `clone`, `replica`, `secondary` | NoninclusiveTerms.Trait.slave.ns.foo#MySimpleValueTrait +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {document/foo/0/bar} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Trait.whitelist.ns.foo#MyWhitelistTrait.document/foo/0/bar +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {nested/master_key_violation} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Trait.whitelist.ns.foo#MyWhitelistTrait.nested/master_key_violation +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {string_value} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Trait.whitelist.ns.foo#MyWhitelistTrait.string_value +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {document/whitelist_doc_key_violation_1} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Trait.whitelist.ns.foo#MyWhitelistTrait.document/whitelist_doc_key_violation_1 +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {collection/2/key} contains a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Trait.whitelist.ns.foo#MyWhitelistTrait.collection/2/key +[WARNING] ns.foo#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {collection/1/blacklist_key} contains a non-inclusive term `blacklist`. Consider using one of the following terms instead: `denyList` | NoninclusiveTerms.Trait.blacklist.ns.foo#MyWhitelistTrait.collection/1/blacklist_key +[WARNING] ns.foo#MyUnionTrait$int_whitelist: Member shape uses a non-inclusive term `whitelist`. Consider using one of the following terms instead: `allowList` | NoninclusiveTerms.Shape.whitelist diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/AbstractValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/AbstractValidator.java index 22065cf4185..35fbeb2a4f5 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/AbstractValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/AbstractValidator.java @@ -28,48 +28,365 @@ public String getName() { return defaultName; } - protected final ValidationEvent error(Shape shape, String message) { + protected final ValidationEvent error( + Shape shape, + String message + ) { return createEvent(Severity.ERROR, shape, shape.getSourceLocation(), message); } - protected final ValidationEvent error(Shape shape, FromSourceLocation location, String message) { + protected final ValidationEvent error( + Shape shape, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.ERROR, shape, shape.getSourceLocation(), message, eventIdSubpart1); + } + + protected final ValidationEvent error( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.ERROR, shape, shape.getSourceLocation(), message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent error( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.ERROR, shape, shape.getSourceLocation(), message, eventIdSubpart1, eventIdSubpart2, + eventIdSubpart3); + } + + protected final ValidationEvent error( + Shape shape, + FromSourceLocation location, + String message + ) { return createEvent(Severity.ERROR, shape, location, message); } - protected final ValidationEvent danger(Shape shape, String message) { + protected final ValidationEvent error( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.ERROR, shape, location, message, eventIdSubpart1); + } + + protected final ValidationEvent error( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.ERROR, shape, location, message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent error( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.ERROR, shape, location, message, eventIdSubpart1, eventIdSubpart2, eventIdSubpart3); + } + + protected final ValidationEvent danger( + Shape shape, + String message + ) { return createEvent(Severity.DANGER, shape, shape.getSourceLocation(), message); } - protected final ValidationEvent danger(Shape shape, FromSourceLocation location, String message) { + protected final ValidationEvent danger( + Shape shape, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.DANGER, shape, shape.getSourceLocation(), message, eventIdSubpart1); + } + + protected final ValidationEvent danger( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.DANGER, shape, shape.getSourceLocation(), message, eventIdSubpart1, + eventIdSubpart2); + } + + protected final ValidationEvent danger( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.DANGER, shape, shape.getSourceLocation(), message, eventIdSubpart1, eventIdSubpart2, + eventIdSubpart3); + } + + protected final ValidationEvent danger( + Shape shape, + FromSourceLocation location, + String message + ) { return createEvent(Severity.DANGER, shape, location, message); } - protected final ValidationEvent warning(Shape shape, String message) { + protected final ValidationEvent danger( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.DANGER, shape, location, message, eventIdSubpart1); + } + + protected final ValidationEvent danger( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.DANGER, shape, location, message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent danger( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.DANGER, shape, location, message, eventIdSubpart1, eventIdSubpart2, + eventIdSubpart3); + } + + protected final ValidationEvent warning( + Shape shape, + String message + ) { return createEvent(Severity.WARNING, shape, shape.getSourceLocation(), message); } - protected final ValidationEvent warning(Shape shape, FromSourceLocation location, String message) { + protected final ValidationEvent warning( + Shape shape, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.WARNING, shape, shape.getSourceLocation(), message, eventIdSubpart1); + } + + protected final ValidationEvent warning( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.WARNING, shape, shape.getSourceLocation(), message, eventIdSubpart1, + eventIdSubpart2); + } + + protected final ValidationEvent warning( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.WARNING, shape, shape.getSourceLocation(), message, eventIdSubpart1, + eventIdSubpart2, eventIdSubpart3); + } + + protected final ValidationEvent warning( + Shape shape, + FromSourceLocation location, + String message + ) { return createEvent(Severity.WARNING, shape, location, message); } - protected final ValidationEvent note(Shape shape, String message) { + protected final ValidationEvent warning( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.WARNING, shape, location, message, eventIdSubpart1); + } + + protected final ValidationEvent warning( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.WARNING, shape, location, message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent warning( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.WARNING, shape, location, message, eventIdSubpart1, eventIdSubpart2, + eventIdSubpart3); + } + + protected final ValidationEvent note( + Shape shape, + String message + ) { return createEvent(Severity.NOTE, shape, shape.getSourceLocation(), message); } - protected final ValidationEvent note(Shape shape, FromSourceLocation location, String message) { + protected final ValidationEvent note( + Shape shape, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.NOTE, shape, shape.getSourceLocation(), message, eventIdSubpart1); + } + + protected final ValidationEvent note( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.NOTE, shape, shape.getSourceLocation(), message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent note( + Shape shape, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.NOTE, shape, shape.getSourceLocation(), message, eventIdSubpart1, eventIdSubpart2, + eventIdSubpart3); + } + + protected final ValidationEvent note( + Shape shape, + FromSourceLocation location, + String message + ) { return createEvent(Severity.NOTE, shape, location, message); } - protected final ValidationEvent createEvent(Severity severity, Shape shape, String message) { - return createEvent(severity, shape, shape.getSourceLocation(), message); + protected final ValidationEvent note( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1 + ) { + return createEvent(Severity.NOTE, shape, location, message, eventIdSubpart1); + } + + protected final ValidationEvent note( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return createEvent(Severity.NOTE, shape, location, message, eventIdSubpart1, eventIdSubpart2); + } + + protected final ValidationEvent note( + Shape shape, + FromSourceLocation location, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return createEvent(Severity.NOTE, shape, location, message, eventIdSubpart1, eventIdSubpart2, eventIdSubpart3); + } + + protected final ValidationEvent createEvent( + Severity severity, + Shape shape, + FromSourceLocation loc, + String message + ) { + return ValidationEvent.builder() + .severity(severity) + .message(message) + .shapeId(shape.getId()) + .sourceLocation(loc.getSourceLocation()) + .id(getName()) + .build(); + } + + protected final ValidationEvent createEvent( + Severity severity, + Shape shape, + FromSourceLocation loc, + String message, + String eventIdSubpart1 + ) { + return ValidationEvent.builder() + .severity(severity) + .message(message) + .shapeId(shape.getId()) + .sourceLocation(loc.getSourceLocation()) + .id(getName() + "." + eventIdSubpart1) + .build(); } - protected final ValidationEvent createEvent(Severity severity, Shape shape, FromSourceLocation loc, String msg) { - return createEvent(ValidationEvent.builder().severity(severity).message(msg) - .shapeId(shape.getId()).sourceLocation(loc.getSourceLocation())); + protected final ValidationEvent createEvent( + Severity severity, + Shape shape, + FromSourceLocation loc, + String message, + String eventIdSubpart1, + String eventIdSubpart2 + ) { + return ValidationEvent.builder() + .severity(severity) + .message(message) + .shapeId(shape.getId()) + .sourceLocation(loc.getSourceLocation()) + .id(getName() + "." + eventIdSubpart1 + "." + eventIdSubpart2) + .build(); } - private ValidationEvent createEvent(ValidationEvent.Builder builder) { - return builder.id(getName()).build(); + protected final ValidationEvent createEvent( + Severity severity, + Shape shape, + FromSourceLocation loc, + String message, + String eventIdSubpart1, + String eventIdSubpart2, + String eventIdSubpart3 + ) { + return ValidationEvent.builder() + .severity(severity) + .message(message) + .shapeId(shape.getId()) + .sourceLocation(loc.getSourceLocation()) + .id(getName() + "." + eventIdSubpart1 + "." + eventIdSubpart2 + "." + eventIdSubpart3) + .build(); } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/NodeValidationVisitor.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/NodeValidationVisitor.java index 063c8605418..7ba0957397e 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/NodeValidationVisitor.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/NodeValidationVisitor.java @@ -309,7 +309,7 @@ public List structureShape(StructureShape shape) { if (!members.containsKey(entryKey)) { String message = String.format( "Invalid structure member `%s` found for `%s`", entryKey, shape.getId()); - events.add(event(message, Severity.WARNING)); + events.add(event(message, Severity.WARNING, shape.getId().toString(), entryKey)); } else { events.addAll(traverse(entryKey, entryValue).memberShape(members.get(entryKey))); } @@ -402,17 +402,23 @@ private List invalidSchema(Shape shape) { return ListUtils.of(event("Encountered invalid shape type: " + shape.getType())); } - private ValidationEvent event(String message) { - return event(message, Severity.ERROR); + private ValidationEvent event(String message, String... additionalEventIdParts) { + return event(message, Severity.ERROR, additionalEventIdParts); } - private ValidationEvent event(String message, Severity severity) { - return event(message, severity, value.getSourceLocation()); + private ValidationEvent event(String message, Severity severity, String... additionalEventIdParts) { + return event(message, severity, value.getSourceLocation(), additionalEventIdParts); } - private ValidationEvent event(String message, Severity severity, SourceLocation sourceLocation) { + private ValidationEvent event( + String message, + Severity severity, + SourceLocation sourceLocation, + String... additionalEventIdParts + ) { return ValidationEvent.builder() - .id(eventId) + .id(additionalEventIdParts.length > 0 + ? eventId + "." + String.join(".", additionalEventIdParts) : eventId) .severity(severity) .sourceLocation(sourceLocation) .shapeId(eventShapeId) 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 82196018cc0..94a54f83335 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,8 +99,8 @@ public String getModelLocation() { *

The validation events encountered while validating a model are * 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 suppression reason or message + * same shape, have the same severity, the eventId of A contains the eventId + * of E, and the message of E starts with the suppression reason or message * of A. * * @param validatedResult Result of creating and validating the model. @@ -135,7 +135,7 @@ private static boolean compareEvents(ValidationEvent expected, ValidationEvent a String comparedMessage = expected.getMessage().replace("\n", "\\n").replace("\r", "\\n"); return expected.getSeverity() == actual.getSeverity() - && expected.getId().equals(actual.getId()) + && actual.containsId(expected.getId()) && expected.getShapeId().equals(actual.getShapeId()) // Normalize new lines. && normalizedActualMessage.startsWith(comparedMessage); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java index 1465406efa8..686e05ade3f 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/EnumTraitValidator.java @@ -71,10 +71,14 @@ private List validateEnumTrait(Shape shape, EnumTrait trait) { "Duplicate enum trait values found with the same `name` property of '%s'", name))); } if (!RECOMMENDED_NAME_PATTERN.matcher(name).find()) { - events.add(warning(shape, trait, String.format( - "The name `%s` does not match the recommended enum name format of beginning with an " - + "uppercase letter, followed by any number of uppercase letters, numbers, or underscores.", - name))); + events.add(warning( + shape, + trait, + String.format("The name `%s` does not match the recommended enum name format of beginning " + + "with an uppercase letter, followed by any number of uppercase letters, numbers, " + + "or underscores.", name), + name) + ); } } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpBindingsMissingValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpBindingsMissingValidator.java index b3e1822d35c..a51072c05c9 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpBindingsMissingValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpBindingsMissingValidator.java @@ -79,7 +79,7 @@ private boolean hasBindings(OperationShape op) { } private ValidationEvent createEvent(Severity severity, ServiceShape service, OperationShape operation) { - return createEvent(severity, operation, String.format( + return createEvent(severity, operation, operation.getSourceLocation(), String.format( "One or more operations in the `%s` service define the `http` trait, but this " + "operation is missing the `http` trait.", service.getId())); } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java index 3bb8e85584c..e34024072a4 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpMethodSemanticsValidator.java @@ -67,6 +67,12 @@ public final class HttpMethodSemanticsValidator extends AbstractValidator { "PUT", new HttpMethodSemantics(false, true, true, null), "PATCH", new HttpMethodSemantics(false, null, true, null)); + private static final String UNEXPECTED_PAYLOAD = "UnexpectedPayload"; + private static final String MISSING_READONLY_TRAIT = "MissingReadonlyTrait"; + private static final String MISSING_IDEMPOTENT_TRAIT = "MissingIdempotentTrait"; + private static final String UNNECESSARY_READONLY_TRAIT = "UnnecessaryReadonlyTrait"; + private static final String UNNECESSARY_IDEMPOTENT_TRAIT = "UnnecessaryIdempotentTrait"; + @Override public List validate(Model model) { if (!model.isTraitApplied(HttpTrait.class)) { @@ -97,21 +103,23 @@ private List validateOperation( HttpMethodSemantics semantics = EXPECTED.get(method); if (semantics.warningWhenModeled != null) { - events.add(warning(shape, trait, semantics.warningWhenModeled)); + events.add(warning(shape, trait, semantics.warningWhenModeled, method)); } boolean isReadonly = shape.getTrait(ReadonlyTrait.class).isPresent(); if (semantics.isReadonly != null && semantics.isReadonly != isReadonly) { events.add(warning(shape, trait, String.format( "This operation uses the `%s` method in the `http` trait, but %s marked with the readonly trait", - method, isReadonly ? "is" : "is not"))); + method, isReadonly ? "is" : "is not"), + isReadonly ? UNNECESSARY_READONLY_TRAIT : MISSING_READONLY_TRAIT)); } boolean isIdempotent = shape.getTrait(IdempotentTrait.class).isPresent(); if (semantics.isIdempotent != null && semantics.isIdempotent != isIdempotent) { events.add(warning(shape, trait, String.format( "This operation uses the `%s` method in the `http` trait, but %s marked with the idempotent trait", - method, isIdempotent ? "is" : "is not"))); + method, isIdempotent ? "is" : "is not"), + isIdempotent ? UNNECESSARY_IDEMPOTENT_TRAIT : MISSING_IDEMPOTENT_TRAIT)); } List payloadBindings = bindingIndex.getRequestBindings(shape, HttpBinding.Location.PAYLOAD); @@ -119,25 +127,27 @@ private List validateOperation( if (semantics.allowsRequestPayload != null && !semantics.allowsRequestPayload) { if (!payloadBindings.isEmpty()) { events.add(danger(shape, trait, String.format( - "This operation uses the `%s` method in the `http` trait, but the `%s` member is sent as the " - + "payload of the request because it is marked with the `httpPayload` trait. Many HTTP " - + "clients do not support payloads with %1$s requests. Consider binding this member to " - + "other parts of the HTTP request such as a query string parameter using the `httpQuery` " - + "trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` " - + "trait.", - method, payloadBindings.get(0).getMemberName() - ))); + "This operation uses the `%s` method in the `http` trait, but the `%s` member is sent as the " + + "payload of the request because it is marked with the `httpPayload` trait. Many HTTP " + + "clients do not support payloads with %1$s requests. Consider binding this member to " + + "other parts of the HTTP request such as a query string parameter using the `httpQuery` " + + "trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` " + + "trait.", + method, payloadBindings.get(0).getMemberName()), + UNEXPECTED_PAYLOAD) + ); } else if (!documentBindings.isEmpty()) { events.add(danger(shape, trait, String.format( - "This operation uses the `%s` method in the `http` trait, but the following members " - + "are sent as part of the payload of the request: %s. These members are sent as part " - + "of the payload because they are not explicitly configured to be sent in headers, in the " - + "query string, or in a URI segment. Many HTTP clients do not support payloads with %1$s " - + "requests. Consider binding these members to other parts of the HTTP request such as " - + "query string parameters using the `httpQuery` trait, headers using the `httpHeader` " - + "trait, or URI segments using the `httpLabel` trait.", - method, ValidationUtils.tickedList(documentBindings.stream().map(HttpBinding::getMemberName)) - ))); + "This operation uses the `%s` method in the `http` trait, but the following members " + + "are sent as part of the payload of the request: %s. These members are sent as part " + + "of the payload because they are not explicitly configured to be sent in headers, in the " + + "query string, or in a URI segment. Many HTTP clients do not support payloads with %1$s " + + "requests. Consider binding these members to other parts of the HTTP request such as " + + "query string parameters using the `httpQuery` trait, headers using the `httpHeader` " + + "trait, or URI segments using the `httpLabel` trait.", + method, ValidationUtils.tickedList(documentBindings.stream().map(HttpBinding::getMemberName))), + UNEXPECTED_PAYLOAD) + ); } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/OperationValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/OperationValidator.java index 1ac72a3d4ae..42e48cd01fa 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/OperationValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/OperationValidator.java @@ -141,7 +141,7 @@ private ValidationEvent emitBadInputOutputName(Shape operation, String property, return ValidationEvent.builder() .severity(Severity.WARNING) .shape(operation) - .id(OPERATION_INPUT_OUTPUT_NAME) + .id(OPERATION_INPUT_OUTPUT_NAME + "." + property) .message(String.format( "The %s of this operation should target a shape that starts with the operation's name, '%s', " + "but the targeted shape is `%s`", property, operation.getId().getName(), target)) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java index 9f778b83b52..08dbdf56ec3 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PaginatedTraitValidator.java @@ -61,6 +61,9 @@ public final class PaginatedTraitValidator extends AbstractValidator { private static final Set TOKEN_SHAPES = SetUtils.of(ShapeType.STRING, ShapeType.MAP); private static final Set DANGER_TOKEN_SHAPES = SetUtils.of(ShapeType.MAP); private static final Pattern PATH_PATTERN = Pattern.compile("\\."); + private static final String DEEPLY_NESTED = "DeeplyNested"; + private static final String SHOULD_NOT_BE_REQUIRED = "ShouldNotBeRequired"; + private static final String WRONG_SHAPE_TYPE = "WrongShapeType"; @Override public List validate(Model model) { @@ -91,9 +94,14 @@ private List validateOperation( events.addAll(validateMember(opIndex, model, null, operation, trait, pageSizeValidator)); pageSizeValidator.getMember(model, opIndex, operation, trait) .filter(MemberShape::isRequired) - .ifPresent(member -> events.add(warning(operation, trait, String.format( - "paginated trait `%s` member `%s` should not be required", - pageSizeValidator.propertyName(), member.getMemberName())))); + .ifPresent(member -> events.add(warning( + operation, + trait, + String.format( + "paginated trait `%s` member `%s` should not be required", + pageSizeValidator.propertyName(), member.getMemberName()), + SHOULD_NOT_BE_REQUIRED, pageSizeValidator.propertyName()) + )); // Validate output. events.addAll(validateMember(opIndex, model, null, operation, trait, new OutputTokenValidator())); @@ -165,20 +173,25 @@ private List validateMember( if (validator.dangerTargets().contains(target.getType())) { Set preferredTargets = new TreeSet<>(validator.validTargets()); preferredTargets.removeAll(validator.dangerTargets()); + String traitName = validator.propertyName(); + String memberName = member.getId().getName(); + String targetType = target.getType().toString(); events.add(danger(operation, trait, String.format( - "%spaginated trait `%s` member `%s` targets a %s shape, but this is not recommended. " - + "One of [%s] SHOULD be targeted.", - prefix, validator.propertyName(), member.getId().getName(), target.getType(), - ValidationUtils.tickedList(preferredTargets)))); + "%spaginated trait `%s` member `%s` targets a %s shape, but this is not recommended. " + + "One of [%s] SHOULD be targeted.", + prefix, traitName, memberName, targetType, ValidationUtils.tickedList(preferredTargets)), + WRONG_SHAPE_TYPE, traitName + )); } } if (validator.pathsAllowed() && PATH_PATTERN.split(memberPath).length > 2) { events.add(warning(operation, trait, String.format( - "%spaginated trait `%s` contains a path with more than two parts, which can make your API " - + "cumbersome to use", - prefix, validator.propertyName() - ))); + "%spaginated trait `%s` contains a path with more than two parts, which can make your API " + + "cumbersome to use", + prefix, validator.propertyName()), + DEEPLY_NESTED, validator.propertyName() + )); } return events; diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java index 26229574074..206dd408dd0 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/TargetValidator.java @@ -204,7 +204,7 @@ private void validateDeprecatedTargets( deprecatedTrait.getMessage().ifPresent(message -> builder.append(". ").append(message)); deprecatedTrait.getSince().ifPresent(since -> builder.append(" (since ").append(since).append(')')); events.add(ValidationEvent.builder() - .id("DeprecatedShape") + .id("DeprecatedShape." + target.getId()) .severity(Severity.WARNING) .shape(shape) .message(builder.toString()) diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/UnstableTraitValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/UnstableTraitValidator.java index bdac51a51fb..e17526ca4fc 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/UnstableTraitValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/UnstableTraitValidator.java @@ -35,8 +35,12 @@ public List validate(Model model) { for (Shape trait : model.getShapesWithTrait(UnstableTrait.class)) { for (Shape appliedTo : model.getShapesWithTrait(trait)) { - events.add(warning(appliedTo, trait, format( - "This shape applies a trait that is unstable: %s", trait.toShapeId()))); + events.add(warning( + appliedTo, + trait, + format("This shape applies a trait that is unstable: %s", trait.toShapeId()), + trait.toShapeId().toString()) + ); } } diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/ShapeMatcher.java b/smithy-model/src/test/java/software/amazon/smithy/model/ShapeMatcher.java index 3f98741b430..f7531792164 100644 --- a/smithy-model/src/test/java/software/amazon/smithy/model/ShapeMatcher.java +++ b/smithy-model/src/test/java/software/amazon/smithy/model/ShapeMatcher.java @@ -117,7 +117,7 @@ private static boolean findEvent( for (ValidationEvent event : result.getValidationEvents()) { if (event.getShapeId().filter(sid -> sid.equals(id.toShapeId())).isPresent() && event.getSeverity() == severity - && event.getId().equals(eventId) + && event.containsId(eventId) && event.getMessage().contains(contains)) { return true; } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/annotation-trait-extra-properties.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/annotation-trait-extra-properties.errors index 3c01e7d2fdf..7192f56e252 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/annotation-trait-extra-properties.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/annotation-trait-extra-properties.errors @@ -1 +1 @@ -[WARNING] ns.foo#Baz: Error validating trait `sensitive`: Invalid structure member `test` found for `smithy.api#sensitive` | TraitValue +[WARNING] ns.foo#Baz: Error validating trait `sensitive`: Invalid structure member `test` found for `smithy.api#sensitive` | TraitValue.smithy.api#sensitive.test diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/deprecated-shape/deprecated-shape.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/deprecated-shape/deprecated-shape.errors index 892a168ed4c..1d897b87456 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/deprecated-shape/deprecated-shape.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/deprecated-shape/deprecated-shape.errors @@ -1,4 +1,4 @@ -[WARNING] smithy.example#Test$foo1: Member targets a deprecated shape, smithy.example#Foo1 | DeprecatedShape -[WARNING] smithy.example#Test$foo2: Member targets a deprecated shape, smithy.example#Foo2 (since 2.0) | DeprecatedShape -[WARNING] smithy.example#Test$foo3: Member targets a deprecated shape, smithy.example#Foo3. hello (since 2.0) | DeprecatedShape -[WARNING] smithy.example#Test: Applies a deprecated mixin, smithy.example#DeprecatedMixin | DeprecatedShape +[WARNING] smithy.example#Test$foo1: Member targets a deprecated shape, smithy.example#Foo1 | DeprecatedShape.smithy.example#Foo1 +[WARNING] smithy.example#Test$foo2: Member targets a deprecated shape, smithy.example#Foo2 (since 2.0) | DeprecatedShape.smithy.example#Foo2 +[WARNING] smithy.example#Test$foo3: Member targets a deprecated shape, smithy.example#Foo3. hello (since 2.0) | DeprecatedShape.smithy.example#Foo3 +[WARNING] smithy.example#Test: Applies a deprecated mixin, smithy.example#DeprecatedMixin | DeprecatedShape.smithy.example#DeprecatedMixin diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/enums/enum-trait-validation.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/enums/enum-trait-validation.errors index b6bf1525082..e0c11741acc 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/enums/enum-trait-validation.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/enums/enum-trait-validation.errors @@ -1,9 +1,9 @@ [WARNING] ns.foo#ValidNoName: Enums should define the `name` property to allow rich types to be generated in code generators. | EnumNamesPresent -[WARNING] ns.foo#Warn1: The name `_bar` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait -[WARNING] ns.foo#Warn1: The name `baz` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait -[WARNING] ns.foo#Invalid2: The name `invalid!` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait -[WARNING] ns.foo#Invalid2: The name `invalid2!` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait -[WARNING] ns.foo#Invalid3: The name `a` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait +[WARNING] ns.foo#Warn1: The name `_bar` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait._bar +[WARNING] ns.foo#Warn1: The name `baz` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait.baz +[WARNING] ns.foo#Invalid2: The name `invalid!` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait.invalid! +[WARNING] ns.foo#Invalid2: The name `invalid2!` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait.invalid2! +[WARNING] ns.foo#Invalid3: The name `a` does not match the recommended enum name format of beginning with an uppercase letter, followed by any number of uppercase letters, numbers, or underscores. | EnumTrait.a [ERROR] ns.foo#Invalid1: `foo` enum value body is missing the `name` property; if any enum trait value contains a `name` property, then all values must contain the `name` property. | EnumTrait [ERROR] ns.foo#Invalid2: Error validating trait `enum`.0.name: String value provided for `smithy.api#EnumConstantBodyName` must match regular expression: ^[a-zA-Z_]+[a-zA-Z_0-9]*$ | TraitValue [ERROR] ns.foo#Invalid2: Error validating trait `enum`.1.name: String value provided for `smithy.api#EnumConstantBodyName` must match regular expression: ^[a-zA-Z_]+[a-zA-Z_0-9]*$ | TraitValue diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors index b8a1123b292..e63b8308ae7 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.errors @@ -1,7 +1,7 @@ -[DANGER] ns.foo#K: This operation uses the `GET` method in the `http` trait, but the `payload` member is sent as the payload of the request because it is marked with the `httpPayload` trait. Many HTTP clients do not support payloads with GET requests. Consider binding this member to other parts of the HTTP request such as a query string parameter using the `httpQuery` trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` trait. | HttpMethodSemantics -[DANGER] ns.foo#L: This operation uses the `GET` method in the `http` trait, but the following members are sent as part of the payload of the request: `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with GET requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics -[DANGER] ns.foo#M: This operation uses the `DELETE` method in the `http` trait, but the following members are sent as part of the payload of the request: `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with DELETE requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics -[WARNING] ns.foo#G: This operation uses the `POST` method in the `http` trait, but is marked with the readonly trait | HttpMethodSemantics -[WARNING] ns.foo#H: This operation uses the `DELETE` method in the `http` trait, but is not marked with the idempotent trait | HttpMethodSemantics -[WARNING] ns.foo#I: This operation uses the `GET` method in the `http` trait, but is not marked with the readonly trait | HttpMethodSemantics -[WARNING] ns.foo#Options: OPTIONS requests are typically used as part of CORS and should not be modeled explicitly. They are an implementation detail that should not appear in generated client or server code. Instead, tooling should use the `cors` trait on a service to automatically configure CORS on clients and servers. It is the responsibility of service frameworks and API gateways to automatically manage OPTIONS requests. For example, OPTIONS requests are automatically created when using Smithy models with Amazon API Gateway. | HttpMethodSemantics +[DANGER] ns.foo#K: This operation uses the `GET` method in the `http` trait, but the `payload` member is sent as the payload of the request because it is marked with the `httpPayload` trait. Many HTTP clients do not support payloads with GET requests. Consider binding this member to other parts of the HTTP request such as a query string parameter using the `httpQuery` trait, a header using the `httpHeader` trait, or a path segment using the `httpLabel` trait. | HttpMethodSemantics.UnexpectedPayload +[DANGER] ns.foo#L: This operation uses the `GET` method in the `http` trait, but the following members are sent as part of the payload of the request: `morePayload`, `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with GET requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics.UnexpectedPayload +[DANGER] ns.foo#M: This operation uses the `DELETE` method in the `http` trait, but the following members are sent as part of the payload of the request: `payload`. These members are sent as part of the payload because they are not explicitly configured to be sent in headers, in the query string, or in a URI segment. Many HTTP clients do not support payloads with DELETE requests. Consider binding these members to other parts of the HTTP request such as query string parameters using the `httpQuery` trait, headers using the `httpHeader` trait, or URI segments using the `httpLabel` trait. | HttpMethodSemantics.UnexpectedPayload +[WARNING] ns.foo#G: This operation uses the `POST` method in the `http` trait, but is marked with the readonly trait | HttpMethodSemantics.UnnecessaryReadonlyTrait +[WARNING] ns.foo#H: This operation uses the `DELETE` method in the `http` trait, but is not marked with the idempotent trait | HttpMethodSemantics.MissingIdempotentTrait +[WARNING] ns.foo#I: This operation uses the `GET` method in the `http` trait, but is not marked with the readonly trait | HttpMethodSemantics.MissingReadonlyTrait +[WARNING] ns.foo#Options: OPTIONS requests are typically used as part of CORS and should not be modeled explicitly. They are an implementation detail that should not appear in generated client or server code. Instead, tooling should use the `cors` trait on a service to automatically configure CORS on clients and servers. It is the responsibility of service frameworks and API gateways to automatically manage OPTIONS requests. For example, OPTIONS requests are automatically created when using Smithy models with Amazon API Gateway. | HttpMethodSemantics.OPTIONS diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.json index 8722f7743b4..d55c1449aa7 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/http-method-semantics-validator.json @@ -152,6 +152,12 @@ "traits": { "smithy.api#required": {} } + }, + "morePayload": { + "target": "smithy.api#String", + "traits": { + "smithy.api#required": {} + } } }, "traits": { diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/operation/input-output-traits/invalid-multiple-operations-same-shape.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/operation/input-output-traits/invalid-multiple-operations-same-shape.errors index b34f96ffa50..07f41cbe0bf 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/operation/input-output-traits/invalid-multiple-operations-same-shape.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/operation/input-output-traits/invalid-multiple-operations-same-shape.errors @@ -1,4 +1,4 @@ -[WARNING] smithy.example#GetFoo2: The input of this operation should target a shape that starts with the operation's name, 'GetFoo2', but the targeted shape is `smithy.example#GetFooInput` | OperationInputOutputName -[WARNING] smithy.example#GetFoo2: The output of this operation should target a shape that starts with the operation's name, 'GetFoo2', but the targeted shape is `smithy.example#GetFooOutput` | OperationInputOutputName +[WARNING] smithy.example#GetFoo2: The input of this operation should target a shape that starts with the operation's name, 'GetFoo2', but the targeted shape is `smithy.example#GetFooInput` | OperationInputOutputName.input +[WARNING] smithy.example#GetFoo2: The output of this operation should target a shape that starts with the operation's name, 'GetFoo2', but the targeted shape is `smithy.example#GetFooOutput` | OperationInputOutputName.output [ERROR] smithy.example#GetFooInput: Shapes marked with the @input trait cannot be used as input by multiple operations: `smithy.example#GetFoo`, `smithy.example#GetFoo2` | OperationInputOutputMisuse [ERROR] smithy.example#GetFooOutput: Shapes marked with the @output trait cannot be used as output by multiple operations: `smithy.example#GetFoo`, `smithy.example#GetFoo2` | OperationInputOutputMisuse diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-deeply-nested.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-deeply-nested.errors index 4e034982a89..a48cb05bb15 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-deeply-nested.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-deeply-nested.errors @@ -1,2 +1,2 @@ -[WARNING] ns.foo#DeeplyNested: paginated trait `items` contains a path with more than two parts, which can make your API cumbersome to use | PaginatedTrait -[WARNING] ns.foo#DeeplyNested: paginated trait `outputToken` contains a path with more than two parts, which can make your API cumbersome to use | PaginatedTrait +[WARNING] ns.foo#DeeplyNested: paginated trait `items` contains a path with more than two parts, which can make your API cumbersome to use | PaginatedTrait.DeeplyNested.items +[WARNING] ns.foo#DeeplyNested: paginated trait `outputToken` contains a path with more than two parts, which can make your API cumbersome to use | PaginatedTrait.DeeplyNested.outputToken diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors index 3a9ce8f2a3f..90123a12a99 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-invalid.errors @@ -5,7 +5,7 @@ [ERROR] ns.foo#Invalid3: paginated trait `outputToken` member `OutputNotString` targets a integer shape, but must target one of the following: [`map`, `string`] | PaginatedTrait [ERROR] ns.foo#Invalid4: paginated trait `inputToken` member `nextToken` must not be required | PaginatedTrait [ERROR] ns.foo#Invalid4: paginated trait `outputToken` member `nextToken` must not be required | PaginatedTrait -[WARNING] ns.foo#Invalid4: paginated trait `pageSize` member `pageSize` should not be required | PaginatedTrait +[WARNING] ns.foo#Invalid4: paginated trait `pageSize` member `pageSize` should not be required | PaginatedTrait.ShouldNotBeRequired.pageSize [ERROR] ns.foo#Invalid5: paginated trait `pageSize` member `Invalid5Input` targets a string shape, but must target one of the following: [`integer`] | PaginatedTrait [ERROR] ns.foo#Invalid6: paginated trait `items` member `Invalid6Output` targets a string shape, but must target one of the following: [`list`, `map`] | PaginatedTrait [ERROR] ns.foo#Invalid7: paginated trait `items` targets a member `items` that does not exist | PaginatedTrait diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors index 4a47be1d7c5..9f6bc968c2f 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-map-tokens.errors @@ -1,2 +1,2 @@ -[DANGER] ns.foo#MapTokens: paginated trait `inputToken` member `MapTokensInput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait -[DANGER] ns.foo#MapTokens: paginated trait `outputToken` member `MapTokensOutput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait +[DANGER] ns.foo#MapTokens: paginated trait `inputToken` member `MapTokensInput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.inputToken +[DANGER] ns.foo#MapTokens: paginated trait `outputToken` member `MapTokensOutput` targets a map shape, but this is not recommended. One of [`string`] SHOULD be targeted. | PaginatedTrait.WrongShapeType.outputToken diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/trait-value/extra-trait-property.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/trait-value/extra-trait-property.errors index c8614d5eef9..b6e1dacbe9c 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/trait-value/extra-trait-property.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/trait-value/extra-trait-property.errors @@ -1 +1 @@ -[WARNING] smithy.example#MyInteger: Error validating trait `range`: Invalid structure member `maxx` found for `smithy.api#range` | TraitValue +[WARNING] smithy.example#MyInteger: Error validating trait `range`: Invalid structure member `maxx` found for `smithy.api#range` | TraitValue.smithy.api#range.maxx diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/unstable-trait-validator.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/unstable-trait-validator.errors index e8527d0a44f..e4b1405b9a1 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/unstable-trait-validator.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/unstable-trait-validator.errors @@ -1 +1 @@ -[WARNING] ns.foo#MyString: This shape applies a trait that is unstable: ns.foo#fooTrait | UnstableTrait +[WARNING] ns.foo#MyString: This shape applies a trait that is unstable: ns.foo#fooTrait | UnstableTrait.ns.foo#fooTrait diff --git a/smithy-waiters/src/main/java/software/amazon/smithy/waiters/WaiterMatcherValidator.java b/smithy-waiters/src/main/java/software/amazon/smithy/waiters/WaiterMatcherValidator.java index 59cdc3f68e6..b618f6ac00d 100644 --- a/smithy-waiters/src/main/java/software/amazon/smithy/waiters/WaiterMatcherValidator.java +++ b/smithy-waiters/src/main/java/software/amazon/smithy/waiters/WaiterMatcherValidator.java @@ -40,6 +40,9 @@ final class WaiterMatcherValidator implements Matcher.Visitor visitErrorType(Matcher.ErrorTypeMember errorType) { } } - addEvent(Severity.WARNING, INVALID_ERROR_TYPE, String.format( + addEvent(Severity.WARNING, String.format( "errorType '%s' not found on operation. This operation defines the following errors: %s", - error, operation.getErrors())); + error, operation.getErrors()), + INVALID_ERROR_TYPE, waiterName, String.valueOf(acceptorIndex)); return events; } @@ -114,10 +118,11 @@ private void validatePathMatcher(LiteralExpression input, PathMatcher pathMatche case BOOLEAN_EQUALS: // A booleanEquals comparator requires an `expected` value of "true" or "false". if (!pathMatcher.getExpected().equals("true") && !pathMatcher.getExpected().equals("false")) { - addEvent(Severity.ERROR, NON_SUPPRESSABLE_ERROR, String.format( + addEvent(Severity.ERROR, String.format( "Waiter acceptors with a %s comparator must set their `expected` value to 'true' or " - + "'false', but found '%s'.", - PathComparator.BOOLEAN_EQUALS, pathMatcher.getExpected())); + + "'false', but found '%s'.", + PathComparator.BOOLEAN_EQUALS, pathMatcher.getExpected()), + NON_SUPPRESSABLE_ERROR); } validateReturnType(pathMatcher.getComparator(), RuntimeType.BOOLEAN, returnType); break; @@ -138,18 +143,20 @@ private RuntimeType validatePath(LiteralExpression input, String path) { } return result.getReturnType(); } catch (JmespathException e) { - addEvent(Severity.ERROR, NON_SUPPRESSABLE_ERROR, String.format( - "Invalid JMESPath expression (%s): %s", path, e.getMessage())); + addEvent(Severity.ERROR, String.format( + "Invalid JMESPath expression (%s): %s", path, e.getMessage()), + NON_SUPPRESSABLE_ERROR); return RuntimeType.ANY; } } private void validateReturnType(PathComparator comparator, RuntimeType expected, RuntimeType actual) { if (actual != RuntimeType.ANY && actual != expected) { - addEvent(Severity.DANGER, JMESPATH_PROBLEM, String.format( + addEvent(Severity.DANGER, String.format( "Waiter acceptors with a %s comparator must return a `%s` type, but this acceptor was " - + "statically determined to return a `%s` type.", - comparator, expected, actual)); + + "statically determined to return a `%s` type.", + comparator, expected, actual), + JMESPATH_PROBLEM, RETURN_TYPE_MISMATCH, waiterName, String.valueOf(acceptorIndex)); } } @@ -162,26 +169,30 @@ private LiteralExpression createCurrentNodeFromShape(Shape shape) { private void addJmespathEvent(String path, ExpressionProblem problem) { Severity severity; + String eventId; switch (problem.severity) { case ERROR: severity = Severity.ERROR; + eventId = NON_SUPPRESSABLE_ERROR; break; case DANGER: severity = Severity.DANGER; + eventId = JMESPATH_PROBLEM + "." + JMES_PATH_DANGER + "." + waiterName + "." + acceptorIndex; break; default: severity = Severity.WARNING; + eventId = JMESPATH_PROBLEM + "." + JMES_PATH_WARNING + "." + waiterName + "." + acceptorIndex; break; } String problemMessage = problem.message + " (" + problem.line + ":" + problem.column + ")"; - addEvent(severity, severity == Severity.ERROR ? NON_SUPPRESSABLE_ERROR : JMESPATH_PROBLEM, String.format( - "Problem found in JMESPath expression (%s): %s", path, problemMessage)); + addEvent(severity, String.format("Problem found in JMESPath expression (%s): %s", path, problemMessage), + eventId); } - private void addEvent(Severity severity, String id, String message) { + private void addEvent(Severity severity, String message, String... eventIdParts) { events.add(ValidationEvent.builder() - .id(id) + .id(String.join(".", eventIdParts)) .shape(operation) .sourceLocation(waitable) .severity(severity) diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/emits-danger-and-warning-typechecks.errors b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/emits-danger-and-warning-typechecks.errors index 218f8ee06a7..0fe3ffcec12 100644 --- a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/emits-danger-and-warning-typechecks.errors +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/emits-danger-and-warning-typechecks.errors @@ -1,3 +1,3 @@ -[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 0: Problem found in JMESPath expression (`10`.foo): Object field 'foo' extraction performed on number (1:6) | WaitableTraitJmespathProblem -[DANGER] smithy.example#A: Waiter `Invalid2`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `null` type. | WaitableTraitJmespathProblem -[WARNING] smithy.example#A: Waiter `Invalid2`, acceptor 0: Problem found in JMESPath expression (`true` < `false`): Invalid comparator '<' for boolean (1:10) | WaitableTraitJmespathProblem +[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 0: Problem found in JMESPath expression (`10`.foo): Object field 'foo' extraction performed on number (1:6) | WaitableTraitJmespathProblem.JmespathEventDanger.Invalid1.0 +[DANGER] smithy.example#A: Waiter `Invalid2`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `null` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid2.0 +[WARNING] smithy.example#A: Waiter `Invalid2`, acceptor 0: Problem found in JMESPath expression (`true` < `false`): Invalid comparator '<' for boolean (1:10) | WaitableTraitJmespathProblem.JmespathEventWarning.Invalid2.0 diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-errorType.errors b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-errorType.errors index 8e64d042b84..d817a554d40 100644 --- a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-errorType.errors +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-errorType.errors @@ -1 +1 @@ -[WARNING] smithy.example#A: Waiter `A`, acceptor 0: errorType 'Nope' not found on operation. This operation defines the following errors: [smithy.example#OhNo] | WaitableTraitInvalidErrorType +[WARNING] smithy.example#A: Waiter `A`, acceptor 0: errorType 'Nope' not found on operation. This operation defines the following errors: [smithy.example#OhNo] | WaitableTraitInvalidErrorType.A.0 diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-jmespath-syntax.errors b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-jmespath-syntax.errors index 763428c7f91..939dae48fa1 100644 --- a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-jmespath-syntax.errors +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-jmespath-syntax.errors @@ -1,3 +1,3 @@ [ERROR] smithy.example#A: Waiter `Invalid1`, acceptor 0: Invalid JMESPath expression (||): Syntax error | WaitableTrait [ERROR] smithy.example#A: Waiter `Invalid2`, acceptor 0: Problem found in JMESPath expression (length(`10`)): length function argument 0 error | WaitableTrait -[DANGER] smithy.example#A: Waiter `Invalid2`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem +[DANGER] smithy.example#A: Waiter `Invalid2`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid2.0 diff --git a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-return-types.errors b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-return-types.errors index 41be4b1669d..de03fe56c60 100644 --- a/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-return-types.errors +++ b/smithy-waiters/src/test/resources/software/amazon/smithy/waiters/errorfiles/invalid-return-types.errors @@ -1,4 +1,4 @@ -[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem -[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 1: Waiter acceptors with a stringEquals comparator must return a `string` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem -[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 2: Waiter acceptors with a allStringEquals comparator must return a `array` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem -[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 3: Waiter acceptors with a anyStringEquals comparator must return a `array` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem +[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 0: Waiter acceptors with a booleanEquals comparator must return a `boolean` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid1.0 +[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 1: Waiter acceptors with a stringEquals comparator must return a `string` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid1.1 +[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 2: Waiter acceptors with a allStringEquals comparator must return a `array` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid1.2 +[DANGER] smithy.example#A: Waiter `Invalid1`, acceptor 3: Waiter acceptors with a anyStringEquals comparator must return a `array` type, but this acceptor was statically determined to return a `number` type. | WaitableTraitJmespathProblem.ReturnTypeMismatch.Invalid1.3