Fix identical event ids for Noninclusive events

rchache · Sep 14, 2023 · 5fd9a29 · 5fd9a29
1 parent 5b8717c
commit 5fd9a29
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 12 deletions.
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
@@ -19,11 +19,9 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.SourceLocation;
@@ -118,11 +116,11 @@ private NoninclusiveTermsValidator(Config config) {
     @Override
     public List<ValidationEvent> validate(Model model) {
         TextIndex textIndex = TextIndex.of(model);
-        Set<ValidationEvent> validationEvents = new HashSet<>();
+        List<ValidationEvent> validationEvents = new ArrayList<>();
         for (TextInstance text : textIndex.getTextInstances()) {
             validationEvents.addAll(getValidationEvents(text));
         }
-        return new ArrayList<>(validationEvents);
+        return validationEvents;
     }
 
     /**
@@ -174,10 +172,7 @@ private ValidationEvent constructValidationEvent(TextInstance instance,
                     return validationEvent.toBuilder()
                             .message(String.format(
                                     "'%s' trait value at path {%s} contains a non-inclusive term `%s`.%s",
-                                    idiomaticTraitName,
-                                    valuePropertyPathFormatted,
-                                    matchedText.toLowerCase(),
-                                    getReplacementAddendum(matchedText.toLowerCase(), replacements)))
+                                    idiomaticTraitName, valuePropertyPathFormatted, matchedText, replacementAddendum))
                             .id(getName() + "." + TRAIT + "." + matchedText.toLowerCase(Locale.US)
                                     + "." + idiomaticTraitName + "." + valuePropertyPathFormatted)
                             .build();

diff --git a/.../test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors b/.../test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.errors
@@ -7,9 +7,10 @@
 [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 {document/whitelist_doc_key_violation_1/key} 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/key
 [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#AOutput: 'ns.foo#MyWhitelistTrait' trait value at path {collection/1/blacklist_key/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/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
-[WARNING] ns.foo#A: 'examples' trait value at path {0/MasterUsername} contains a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms.Trait.master.examples.0/MasterUsername
 [WARNING] ns.foo#A: Error validating trait `examples`.0: Invalid structure member `MasterUsername` found for `smithy.api#Example` | TraitValue.smithy.api#Example.MasterUsername
+[WARNING] ns.foo#A: 'examples' trait value at path {0/MasterUsername/key} contains a non-inclusive term `Master`. Consider using one of the following terms instead: `Main`, `Parent`, `Primary` | NoninclusiveTerms.Trait.master.examples.0/MasterUsername/key
+[WARNING] ns.foo#A: 'examples' trait value at path {0/MasterUsername} contains a non-inclusive term `master`. Consider using one of the following terms instead: `main`, `parent`, `primary` | NoninclusiveTerms.Trait.master.examples.0/MasterUsername
diff --git a/...rc/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.json b/...rc/test/resources/software/amazon/smithy/linters/errorfiles/noninclusive-term-filter.json
@@ -79,7 +79,7 @@
                 "smithy.api#readonly": {},
                 "smithy.api#examples": [
                     {
-                        "title": "testing multiple noninclusive matches",
+                        "title": "testing noninclusive key and value",
                         "MasterUsername": "master"
                     }
                 ]

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/TextIndex.java b/smithy-model/src/main/java/software/amazon/smithy/model/knowledge/TextIndex.java
@@ -105,8 +105,10 @@ private static void computeTextInstancesForAppliedTrait(
                 if (memberTypeShape == null) {
                     //This means the "property" key value isn't modeled in the trait's structure/shape definition
                     //and this text instance is unique
+                    propertyPath.offerLast("key");
                     textInstances.add(TextInstance.createTraitInstance(
                         memberEntry.getKey(), parentShape, trait, propertyPath));
+                    propertyPath.removeLast();
                 }
                 computeTextInstancesForAppliedTrait(memberEntry.getValue(), trait, parentShape, textInstances,
                         propertyPath, model, memberTypeShape);