Repository: smithy-lang/smithy
Pull request #2480 (2024-11-27 18:56:16)

Reduce false positives of ShouldHaveUsedTimestampValidator
#### Background
* Reduce false positives of ShouldHaveUsedTimestampValidator, when a member shape targets a list of Timestamps or structure of solely Timestamps
* Also allows timey members to target enums without emitting events
* Improves the CX by reducing unhelpful noise in smithy validator

#### Testing
* added new unit tests

#### Links

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/.changes/next-release/bugfix-a4eb4361f200aabca46d06a5985bbd4ccc1fc60c.json b/.changes/next-release/bugfix-a4eb4361f200aabca46d06a5985bbd4ccc1fc60c.json
new file mode 100644
index 000000000..9a229a99d
--- /dev/null
+++ b/.changes/next-release/bugfix-a4eb4361f200aabca46d06a5985bbd4ccc1fc60c.json
@@ -0,0 +1,7 @@
+{
+  "type": "bugfix",
+  "description": "Fixed a bug in smoke test validation where test case ids weren't checked for uniqueness if their operations were bound to a resource shape",
+  "pull_requests": [
+    "[#2482](https://github.com/smithy-lang/smithy/issues/2482)"
+  ]
+}
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java
index 0ec02cb01..8ed708a77 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/AwsTagIndex.java
@@ -83,7 +83,7 @@ public final class AwsTagIndex implements KnowledgeIndex {
     }
 
     public static AwsTagIndex of(Model model) {
-        return new AwsTagIndex(model);
+        return model.getKnowledge(AwsTagIndex.class, AwsTagIndex::new);
     }
 
     /**
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java
index 9d08b7373..abcddb549 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/ServiceTaggingValidator.java
@@ -19,7 +19,7 @@ import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.LIST_T
 import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.TAG_RESOURCE_OPNAME;
 import static software.amazon.smithy.aws.traits.tagging.TaggingShapeUtils.UNTAG_RESOURCE_OPNAME;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import software.amazon.smithy.model.FromSourceLocation;
@@ -36,7 +36,7 @@ public final class ServiceTaggingValidator extends AbstractValidator {
     @Override
     public List<ValidationEvent> validate(Model model) {
         AwsTagIndex awsTagIndex = AwsTagIndex.of(model);
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) {
             events.addAll(validateService(service, awsTagIndex));
         }
@@ -44,7 +44,7 @@ public final class ServiceTaggingValidator extends AbstractValidator {
     }
 
     private List<ValidationEvent> validateService(ServiceShape service, AwsTagIndex awsTagIndex) {
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         TagEnabledTrait trait = service.expectTrait(TagEnabledTrait.class);
 
         Optional<ShapeId> tagResourceId = awsTagIndex.getTagResourceOperation(service.getId());
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java
index 5984d14e4..e29059ed0 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TagEnabledServiceValidator.java
@@ -15,7 +15,7 @@
 
 package software.amazon.smithy.aws.traits.tagging;
 
-import java.util.LinkedList;
+import java.util.ArrayList;
 import java.util.List;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.knowledge.TopDownIndex;
@@ -30,7 +30,7 @@ import software.amazon.smithy.model.validation.ValidationEvent;
 public final class TagEnabledServiceValidator extends AbstractValidator {
     @Override
     public List<ValidationEvent> validate(Model model) {
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         TopDownIndex topDownIndex = TopDownIndex.of(model);
         AwsTagIndex tagIndex = AwsTagIndex.of(model);
         for (ServiceShape service : model.getServiceShapesWithTrait(TagEnabledTrait.class)) {
@@ -44,7 +44,7 @@ public final class TagEnabledServiceValidator extends AbstractValidator {
             AwsTagIndex tagIndex,
             TopDownIndex topDownIndex
     ) {
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         TagEnabledTrait trait = service.expectTrait(TagEnabledTrait.class);
 
         int taggableResourceCount = 0;
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java
index 2dfa663ad..edbab2e2c 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggableResourceValidator.java
@@ -16,8 +16,8 @@
 package software.amazon.smithy.aws.traits.tagging;
 
 import java.util.AbstractMap;
+import java.util.ArrayList;
 import java.util.Collection;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -40,7 +40,7 @@ import software.amazon.smithy.model.validation.ValidationEvent;
 public final class TaggableResourceValidator extends AbstractValidator {
     @Override
     public List<ValidationEvent> validate(Model model) {
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         TopDownIndex topDownIndex = TopDownIndex.of(model);
         AwsTagIndex tagIndex = AwsTagIndex.of(model);
         for (ServiceShape service : model.getServiceShapes()) {
@@ -72,7 +72,7 @@ public final class TaggableResourceValidator extends AbstractValidator {
             ServiceShape service,
             AwsTagIndex awsTagIndex
     ) {
-        List<ValidationEvent> events = new LinkedList<>();
+        List<ValidationEvent> events = new ArrayList<>();
         // Generate danger if resource has tag property in update API.
         if (awsTagIndex.isResourceTagOnUpdate(resource.getId())) {
             Shape operation = resource.getUpdate().isPresent()
@@ -87,7 +87,6 @@ public final class TaggableResourceValidator extends AbstractValidator {
         //    list or tag keys input or output member
         // 2. Tagging via APIs specified in the @taggable trait which are validated
         //    through the tag property, and must be resource instance operations
-        //Caution: avoid short circuiting behavior.
         boolean isServiceWideTaggable = awsTagIndex.serviceHasTagApis(service.getId());
         boolean isInstanceOpTaggable = isTaggableViaInstanceOperations(model, resource);
 
@@ -96,9 +95,11 @@ public final class TaggableResourceValidator extends AbstractValidator {
                     + " It must use the `aws.api@arn` trait."));
         }
 
-        if (!(isServiceWideTaggable || isInstanceOpTaggable)) {
-            events.add(error(resource, "Resource does not have tagging CRUD operations and is not compatible"
-                    + " with service-wide tagging operations."));
+        if (!isServiceWideTaggable && !isInstanceOpTaggable) {
+            events.add(error(resource, String.format("Resource does not have tagging CRUD operations and is not"
+                                                     + " compatible with service-wide tagging operations"
+                                                     + " for service `%s`.",
+                                                     service.getId())));
         }
 
         return events;
@@ -174,7 +175,7 @@ public final class TaggableResourceValidator extends AbstractValidator {
     }
 
     private Collection<Map.Entry<MemberShape, Shape>> collectMemberTargetShapes(ShapeId ioShapeId, Model model) {
-        Collection<Map.Entry<MemberShape, Shape>> collection = new LinkedList<>();
+        Collection<Map.Entry<MemberShape, Shape>> collection = new ArrayList<>();
         for (MemberShape memberShape : model.expectShape(ioShapeId).members()) {
             collection.add(new AbstractMap.SimpleImmutableEntry<>(
                     memberShape, model.expectShape(memberShape.getTarget())));
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java
index 5c2d3857d..4fb4d62a6 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/tagging/TaggingShapeUtils.java
@@ -190,10 +190,10 @@ final class TaggingShapeUtils {
         Model model,
         ResourceShape resource
     ) {
-        PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model);
-        Optional<String> property = resource.expectTrait(TaggableTrait.class).getProperty();
-        if (property.isPresent()) {
-            if (operationId.isPresent()) {
+        if (operationId.isPresent()) {
+            PropertyBindingIndex propertyBindingIndex = PropertyBindingIndex.of(model);
+            Optional<String> property = resource.expectTrait(TaggableTrait.class).getProperty();
+            if (property.isPresent()) {
                 OperationShape operation = model.expectShape(operationId.get()).asOperationShape().get();
                 Shape inputShape = model.expectShape(operation.getInputShape());
                 return isTagPropertyInShape(property.get(), inputShape, propertyBindingIndex);
diff --git a/smithy-linters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java b/smithy-linters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java
index c8062aad0..13d23c2f8 100644
--- a/smithy-linters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java
+++ b/smithy-linters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java
@@ -24,6 +24,7 @@ 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.EnumShape;
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.NumberShape;
 import software.amazon.smithy.model.shapes.Shape;
@@ -112,7 +113,7 @@ public final class ShouldHaveUsedTimestampValidator extends AbstractValidator {
             @Override
             protected List<ValidationEvent> getDefault(Shape shape) {
                 if (shape.isStringShape() || shape instanceof NumberShape) {
-                    return validateSimpleShape(shape, patterns);
+                    return validateSimpleShape(shape);
                 } else {
                     return Collections.emptyList();
                 }
@@ -120,12 +121,17 @@ public final class ShouldHaveUsedTimestampValidator extends AbstractValidator {
 
             @Override
             public List<ValidationEvent> structureShape(StructureShape shape) {
-                return validateStructure(shape, model, patterns);
+                return validateStructure(shape, model);
             }
 
             @Override
             public List<ValidationEvent> unionShape(UnionShape shape) {
-                return validateUnion(shape, model, patterns);
+                return validateUnion(shape, model);
+            }
+
+            @Override
+            public List<ValidationEvent> enumShape(EnumShape shape) {
+                return Collections.emptyList();
             }
         };
 
@@ -134,57 +140,63 @@ public final class ShouldHaveUsedTimestampValidator extends AbstractValidator {
 
     private List<ValidationEvent> validateStructure(
             StructureShape structure,
-            Model model,
-            List<Pattern> patterns
+            Model model
     ) {
         return structure
                 .getAllMembers()
                 .entrySet()
                 .stream()
-                .flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model, patterns))
+                .flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model))
                 .collect(Collectors.toList());
     }
 
     private List<ValidationEvent> validateUnion(
             UnionShape union,
-            Model model,
-            List<Pattern> patterns
+            Model model
     ) {
         return union
                 .getAllMembers()
                 .entrySet()
                 .stream()
-                .flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model, patterns))
+                .flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model))
                 .collect(Collectors.toList());
     }
 
     private Stream<ValidationEvent> validateTargetShape(
             String name,
-            MemberShape target,
-            Model model,
-            List<Pattern> patterns
+            MemberShape memberShape,
+            Model model
     ) {
-        return OptionalUtils.stream(model.getShape(target.getTarget())
-                .flatMap(shape -> validateName(name, shape.getType(), target, patterns)));
+        return OptionalUtils.stream(model.getShape(memberShape.getTarget())
+                .flatMap(targetShape -> validateName(name, targetShape, memberShape, patterns, model)));
     }
 
     private List<ValidationEvent> validateSimpleShape(
-            Shape shape,
-            List<Pattern> patterns
+            Shape shape
     ) {
-        return validateName(shape.getId().getName(), shape.getType(), shape, patterns)
-                .map(ListUtils::of)
-                .orElse(ListUtils.of());
+        String name = shape.getId().getName();
+
+        return patterns
+                .stream()
+                .filter(pattern -> pattern.matcher(name).matches())
+                .map(matcher -> buildEvent(shape, name, shape.getType()))
+                .collect(Collectors.toList());
     }
 
     private Optional<ValidationEvent> validateName(
             String name,
-            ShapeType type,
+            Shape targetShape,
             Shape context,
-            List<Pattern> patterns
+            List<Pattern> patterns,
+            Model model
     ) {
-        if (type == ShapeType.TIMESTAMP) {
+        ShapeType type = targetShape.getType();
+        if (type == ShapeType.TIMESTAMP || type == ShapeType.ENUM) {
             return Optional.empty();
+        } else if (type == ShapeType.STRUCTURE || type == ShapeType.LIST) {
+            if (this.onlyContainsTimestamps(targetShape, model)) {
+                return Optional.empty();
+            }
         }
         return patterns
                 .stream()
@@ -193,6 +205,16 @@ public final class ShouldHaveUsedTimestampValidator extends AbstractValidator {
                 .findAny();
     }
 
+    private boolean onlyContainsTimestamps(Shape shape, Model model) {
+        return shape.getAllMembers()
+                .values()
+                .stream()
+                .map(memberShape -> model.getShape(memberShape.getTarget()))
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .allMatch(Shape::isTimestampShape);
+    }
+
     private ValidationEvent buildEvent(Shape context, String name, ShapeType type) {
         return danger(context, context.isMemberShape()
                 ? String.format("Member `%s` is named like a timestamp but references a `%s` shape", name, type)


Test patch:
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-enabled-service-list-broken.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-enabled-service-list-broken.errors
index 61ce2317f..8de5f15ce 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-enabled-service-list-broken.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-enabled-service-list-broken.errors
@@ -1,3 +1,3 @@
 [DANGER] example.weather#Weather: Shape `example.weather#ListTagsForResource` does not satisfy 'ListTagsForResource' operation requirements. | ServiceTagging
-[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations. | TaggableResource
+[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations for service `example.weather#Weather`. | TaggableResource
 [WARNING] example.weather#Weather: Service marked `aws.api#tagEnabled` trait does not have consistent tagging operations implemented: {TagResource, UntagResource, and ListTagsForResource}. | TagEnabledService
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-on-create.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-on-create.errors
index c11ce595a..67922eecc 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-on-create.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-on-create.errors
@@ -2,4 +2,4 @@
 [WARNING] example.weather#Weather: Service marked `aws.api#TagEnabled` is missing an operation named 'TagResource.' | ServiceTagging
 [WARNING] example.weather#Weather: Service marked `aws.api#TagEnabled` is missing an operation named 'UntagResource.' | ServiceTagging
 [WARNING] example.weather#Weather: Service marked `aws.api#tagEnabled` trait does not have consistent tagging operations implemented: {TagResource, UntagResource, and ListTagsForResource}. | TagEnabledService
-[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations. | TaggableResource
+[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations for service `example.weather#Weather`. | TaggableResource
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.errors
new file mode 100644
index 000000000..18a45045f
--- /dev/null
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.errors
@@ -0,0 +1,3 @@
+[WARNING] example.weather#Forecast: Resource is likely missing `aws.api#taggable` trait. | TaggableResource
+[WARNING] example.weather#AnotherService: Service has resources with `aws.api#taggable` applied but does not have the `aws.api#tagEnabled` trait. | TaggableResource
+[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations for service `example.weather#AnotherService`. | TaggableResource
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.smithy
new file mode 100644
index 000000000..73d034571
--- /dev/null
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-service-wide-two-services.smithy
@@ -0,0 +1,168 @@
+$version: "2.0"
+
+metadata suppressions = [
+    {
+        id: "UnstableTrait",
+        namespace: "example.weather"
+    }
+]
+
+namespace example.weather
+
+use aws.api#arn
+use aws.api#taggable
+use aws.api#tagEnabled
+
+@tagEnabled
+service Weather {
+    version: "2006-03-01",
+    resources: [City]
+    operations: [GetCurrentTime, TagResource, UntagResource, ListTagsForResource]
+}
+
+@internal
+service AnotherService {
+    version: "2006-03-01"
+    resources: [AnotherResouce]
+}
+
+@arn(template: "resource/{resourceId}/another")
+resource AnotherResouce {
+    identifiers: {
+        cityId: CityId
+    }
+    resources: [
+        City
+    ]
+}
+
+structure Tag {
+    key: String
+    value: String
+}
+
+list TagList {
+    member: Tag
+}
+
+list TagKeys {
+    member: String
+}
+
+operation TagResource {
+    input := {
+        @required
+        arn: String
+        @length(max: 128)
+        tags: TagList
+    }
+    output := { }
+}
+
+operation UntagResource {
+    input := {
+        @required
+        arn: String
+        @required
+        tagKeys: TagKeys
+    }
+    output := { }
+}
+
+operation ListTagsForResource {
+    input := {
+        @required
+        arn: String
+    }
+    output := {
+        @length(max: 128)
+        tags: TagList
+    }
+}
+
+@arn(
+    template: "city/{cityId}/forecast/{forecastId}"
+)
+resource Forecast {
+    identifiers: { 
+        cityId: CityId
+        forecastId: ForecastId
+    }
+}
+
+@taggable(property: "tags")
+@arn(template: "city/{CityId}")
+resource City {
+    identifiers: { cityId: CityId }
+    properties: {
+        name: String
+        coordinates: CityCoordinates
+    }
+    read: GetCity
+    resources: [Forecast]
+}
+
+@pattern("^[A-Za-z0-9 ]+$")
+string ForecastId
+
+@pattern("^[A-Za-z0-9 ]+$")
+string CityId
+
+@readonly
+operation GetCity {
+    input: GetCityInput
+    output: GetCityOutput
+    errors: [NoSuchResource]
+}
+
+@input
+structure GetCityInput {
+    // "cityId" provides the identifier for the resource and
+    // has to be marked as required.
+    @required
+    cityId: CityId
+}
+
+@output
+structure GetCityOutput {
+    // "required" is used on output to indicate if the service
+    // will always provide a value for the member.
+    @required
+    name: String,
+
+    @required
+    coordinates: CityCoordinates
+}
+
+// This structure is nested within GetCityOutput.
+structure CityCoordinates {
+    @required
+    latitude: Float,
+
+    @required
+    longitude: Float,
+}
+
+// "error" is a trait that is used to specialize
+// a structure as an error.
+@error("client")
+structure NoSuchResource {
+    @required
+    resourceType: String
+}
+
+@readonly
+operation GetCurrentTime {
+    input: GetCurrentTimeInput
+    output: GetCurrentTimeOutput
+}
+
+@input
+structure GetCurrentTimeInput {}
+
+@output
+structure GetCurrentTimeOutput {
+    @required
+    time: Timestamp
+}
+
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-types.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-types.errors
index 400362b8d..5f8ee4696 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-types.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/tagging/invalid-tag-types.errors
@@ -3,4 +3,4 @@
 [WARNING] example.weather#Weather: Service marked `aws.api#TagEnabled` is missing an operation named 'UntagResource.' | ServiceTagging
 [WARNING] example.weather#Weather: Service marked `aws.api#tagEnabled` trait does not have consistent tagging operations implemented: {TagResource, UntagResource, and ListTagsForResource}. | TagEnabledService
 [ERROR] example.weather#City: Tag property must be a list shape targeting a member containing a pair of strings, or a Map shape targeting a string member. | TagResourcePropertyType
-[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations. | TaggableResource
+[ERROR] example.weather#City: Resource does not have tagging CRUD operations and is not compatible with service-wide tagging operations for service `example.weather#Weather`. | TaggableResource
diff --git a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json
index 962f9ea81..32605bd94 100644
--- a/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json
+++ b/smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json
@@ -24,6 +24,48 @@
         },
         "example.namespace#CreatedOn": {
             "type": "timestamp"
+        },
+        "example.namespace#DateEnum": {
+            "type": "enum",
+            "members": {
+                "ANOMALOUS": {
+                    "target": "smithy.api#Unit",
+                    "traits": {
+                        "smithy.api#enumValue": "anomalous"
+                    }
+                },
+                "NORMAL": {
+                    "target": "smithy.api#Unit",
+                    "traits": {
+                        "smithy.api#enumValue": "normal"
+                    }
+                }
+            }
+        },
+        "example.namespace#DateStructureOuter": {
+            "type": "structure",
+            "members": {
+                "TimestampStructure": {
+                    "target": "example.namespace#DateStructure"
+                },
+                "ListOfTimestamp": {
+                    "target": "example.namespace#DateList"
+                }
+            }
+        },
+        "example.namespace#DateStructure": {
+            "type": "structure",
+            "members": {
+                "TimeOn": {
+                    "target": "example.namespace#CreatedOn"
+                }
+            }
+        },
+        "example.namespace#DateList": {
+            "type": "list",
+            "member": {
+                "target": "example.namespace#CreatedOn"
+            }
         }
     },
     "metadata": {
diff --git a/smithy-smoke-test-traits/src/main/java/software/amazon/smithy/smoketests/traits/UniqueSmokeTestCaseIdValidator.java b/smithy-smoke-test-traits/src/main/java/software/amazon/smithy/smoketests/traits/UniqueSmokeTestCaseIdValidator.java
index ed8d16684..5040d203b 100644
--- a/smithy-smoke-test-traits/src/main/java/software/amazon/smithy/smoketests/traits/UniqueSmokeTestCaseIdValidator.java
+++ b/smithy-smoke-test-traits/src/main/java/software/amazon/smithy/smoketests/traits/UniqueSmokeTestCaseIdValidator.java
@@ -20,18 +20,17 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.knowledge.TopDownIndex;
 import software.amazon.smithy.model.shapes.OperationShape;
 import software.amazon.smithy.model.shapes.ServiceShape;
 import software.amazon.smithy.model.shapes.Shape;
-import software.amazon.smithy.model.shapes.ShapeId;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationUtils;
-import software.amazon.smithy.utils.ListUtils;
+import software.amazon.smithy.utils.SetUtils;
 
 /**
  * Validates that smoke test cases have unique IDs within a service closure,
@@ -43,30 +42,27 @@ public class UniqueSmokeTestCaseIdValidator extends AbstractValidator {
     public List<ValidationEvent> validate(Model model) {
         List<ValidationEvent> events = new ArrayList<>();
         Set<ServiceShape> serviceShapes = model.getServiceShapes();
-        Set<ShapeId> serviceBoundOperationIds = new HashSet<>();
+        Set<OperationShape> serviceBoundOperations = new HashSet<>();
+        TopDownIndex index = new TopDownIndex(model);
         // Validate test case ids within each service closure
         for (ServiceShape service : serviceShapes) {
-            Set<ShapeId> operationIds = service.getAllOperations();
-            serviceBoundOperationIds.addAll(operationIds);
-            List<Shape> shapes = operationIds.stream()
-                    .map(model::getShape)
-                    .filter(Optional::isPresent)
-                    .map(Optional::get)
-                    .collect(Collectors.toList());
-            addValidationEventsForShapes(shapes, events);
+            Set<OperationShape> boundOperations = index.getContainedOperations(service);
+            addValidationEventsForShapes(boundOperations, events);
+            serviceBoundOperations.addAll(boundOperations);
         }
 
         // Also validate ids are unique within each non-service bound operation
         List<OperationShape> shapes = model.getOperationShapesWithTrait(SmokeTestsTrait.class).stream()
-                .filter(shape -> !serviceBoundOperationIds.contains(shape.getId()))
+                .filter(shape -> !serviceBoundOperations.contains(shape))
                 .collect(Collectors.toList());
+
         for (OperationShape shape : shapes) {
-            addValidationEventsForShapes(ListUtils.of(shape), events);
+            addValidationEventsForShapes(SetUtils.of(shape), events);
         }
         return events;
     }
 
-    private void addValidationEventsForShapes(List<? extends Shape> shapes, List<ValidationEvent> events) {
+    private void addValidationEventsForShapes(Set<OperationShape> shapes, List<ValidationEvent> events) {
         Map<String, List<Shape>> testCaseIdsToOperations = new HashMap<>();
         for (Shape shape : shapes) {
             if (!shape.isOperationShape() || !shape.hasTrait(SmokeTestsTrait.class)) {
@@ -91,6 +87,4 @@ public class UniqueSmokeTestCaseIdValidator extends AbstractValidator {
             }
         }
     }
-
-
 }
diff --git a/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.errors b/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.errors
index a24d3d91d..8b9fb608e 100644
--- a/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.errors
+++ b/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.errors
@@ -1,3 +1,5 @@
 [ERROR] smithy.example#SayHello: Conflicting `smithy.test#smokeTests` test case IDs found for ID `say_hello`: `smithy.example#SayHello`, `smithy.example#SayHello`, `smithy.example#SayHello2` | UniqueSmokeTestCaseId
 [ERROR] smithy.example#SayHello2: Conflicting `smithy.test#smokeTests` test case IDs found for ID `say_hello`: `smithy.example#SayHello`, `smithy.example#SayHello`, `smithy.example#SayHello2` | UniqueSmokeTestCaseId
 [ERROR] smithy.example#SayHello3: Conflicting `smithy.test#smokeTests` test case IDs found for ID `say_hello`: `smithy.example#SayHello3`, `smithy.example#SayHello3` | UniqueSmokeTestCaseId
+[ERROR] smithy.example#SayHello6: Conflicting `smithy.test#smokeTests` test case IDs found for ID `not_say_hello`: `smithy.example#SayHello6`, `smithy.example#SayHello7` | UniqueSmokeTestCaseId
+[ERROR] smithy.example#SayHello7: Conflicting `smithy.test#smokeTests` test case IDs found for ID `not_say_hello`: `smithy.example#SayHello6`, `smithy.example#SayHello7` | UniqueSmokeTestCaseId
diff --git a/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.smithy b/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.smithy
index 2b316c032..15a93c0b3 100644
--- a/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.smithy
+++ b/smithy-smoke-test-traits/src/test/resources/software/amazon/smithy/smoketests/traits/errorfiles/duplicate-ids.smithy
@@ -94,7 +94,8 @@ operation SayHello4 {
 
 service OtherSayStuff {
     version: "2023-10-11"
-    operations: [SayHello5]
+    operations: [SayHello5, SayHello6]
+    resources: [SayHelloResource]
 }
 
 // Shouldn't conflict between services
@@ -110,4 +111,36 @@ service OtherSayStuff {
 operation SayHello5 {
    input := {}
    output := {}
-}
\ No newline at end of file
+}
+
+@smokeTests([
+    {
+        id: "not_say_hello" // Conflicts with resource bound operation
+        params: {}
+        expect: {
+            success: {}
+        }
+    }
+])
+operation SayHello6 {
+    input := {}
+    output := {}
+}
+
+resource SayHelloResource {
+    operations: [SayHello7]
+}
+
+@smokeTests([
+    {
+        id: "not_say_hello"
+        params: {}
+        expect: {
+            success: {}
+        }
+    }
+])
+operation SayHello7 {
+    input := {}
+    output := {}
+}