Reduce false positives of ShouldHaveUsedTimestampValidator (#2480)

Reduce false positives of ShouldHaveUsedTimestampValidator
smithy-lang · Dec 10, 2024 · b855e2b · b855e2b
1 parent f8ac947
commit b855e2b
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 22 deletions.
diff --git a/...inters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java b/...inters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java
@@ -24,6 +24,7 @@
 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,20 +113,25 @@ public List<ValidationEvent> validate(Model model) {
             @Override
             protected List<ValidationEvent> getDefault(Shape shape) {
                 if (shape.isStringShape() || shape instanceof NumberShape) {
-                    return validateSimpleShape(shape, patterns);
+                    return validateSimpleShape(shape);
                 } else {
                     return Collections.emptyList();
                 }
             }
 
             @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 List<ValidationEvent> unionShape(UnionShape shape) {
 
     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 @@ private Optional<ValidationEvent> validateName(
                 .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)

diff --git a/...esources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json b/...esources/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": {