From 8c5e45d913d752f9fb5913a1f40cbbe8e4741334 Mon Sep 17 00:00:00 2001 From: Richard Chen Date: Wed, 27 Nov 2024 13:54:17 -0500 Subject: [PATCH 1/2] Reduce false positives of ShouldHaveUsedTimestampValidator --- .../ShouldHaveUsedTimestampValidator.java | 50 ++++++++++++++++--- .../should-have-used-timestamp-ignore.json | 42 ++++++++++++++++ 2 files changed, 84 insertions(+), 8 deletions(-) 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 c8062aad0ef..dc9224f5953 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,12 +24,14 @@ 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; import software.amazon.smithy.model.shapes.ShapeType; import software.amazon.smithy.model.shapes.ShapeVisitor; import software.amazon.smithy.model.shapes.StructureShape; +import software.amazon.smithy.model.shapes.TimestampShape; import software.amazon.smithy.model.shapes.UnionShape; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.ValidationEvent; @@ -127,6 +129,16 @@ public List structureShape(StructureShape shape) { public List unionShape(UnionShape shape) { return validateUnion(shape, model, patterns); } + + @Override + public List timestampShape(TimestampShape shape) { + return Collections.emptyList(); + } + + @Override + public List enumShape(EnumShape shape) { + return Collections.emptyList(); + } }; return model.shapes().flatMap(shape -> shape.accept(visitor).stream()).collect(Collectors.toList()); @@ -160,31 +172,43 @@ private List validateUnion( private Stream validateTargetShape( String name, - MemberShape target, + MemberShape memberShape, Model model, List patterns ) { - 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 validateSimpleShape( Shape shape, List patterns ) { - 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 validateName( String name, - ShapeType type, + Shape targetShape, Shape context, - List patterns + List patterns, + Model model ) { + ShapeType type = targetShape.getType(); if (type == ShapeType.TIMESTAMP) { return Optional.empty(); + } else if (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 +217,16 @@ private Optional 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/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 962f9ea8109..32605bd945d 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": { From e70266fd38b6c3762cc4892a35e57e3477143212 Mon Sep 17 00:00:00 2001 From: Richard Chen Date: Wed, 27 Nov 2024 14:39:56 -0500 Subject: [PATCH 2/2] minor refactors --- .../ShouldHaveUsedTimestampValidator.java | 32 ++++++------------- 1 file changed, 10 insertions(+), 22 deletions(-) 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 dc9224f5953..13d23c2f884 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 @@ -31,7 +31,6 @@ import software.amazon.smithy.model.shapes.ShapeType; import software.amazon.smithy.model.shapes.ShapeVisitor; import software.amazon.smithy.model.shapes.StructureShape; -import software.amazon.smithy.model.shapes.TimestampShape; import software.amazon.smithy.model.shapes.UnionShape; import software.amazon.smithy.model.validation.AbstractValidator; import software.amazon.smithy.model.validation.ValidationEvent; @@ -114,7 +113,7 @@ public List validate(Model model) { @Override protected List getDefault(Shape shape) { if (shape.isStringShape() || shape instanceof NumberShape) { - return validateSimpleShape(shape, patterns); + return validateSimpleShape(shape); } else { return Collections.emptyList(); } @@ -122,17 +121,12 @@ protected List getDefault(Shape shape) { @Override public List structureShape(StructureShape shape) { - return validateStructure(shape, model, patterns); + return validateStructure(shape, model); } @Override public List unionShape(UnionShape shape) { - return validateUnion(shape, model, patterns); - } - - @Override - public List timestampShape(TimestampShape shape) { - return Collections.emptyList(); + return validateUnion(shape, model); } @Override @@ -146,43 +140,39 @@ public List enumShape(EnumShape shape) { private List validateStructure( StructureShape structure, - Model model, - List 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 validateUnion( UnionShape union, - Model model, - List 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 validateTargetShape( String name, MemberShape memberShape, - Model model, - List patterns + Model model ) { return OptionalUtils.stream(model.getShape(memberShape.getTarget()) .flatMap(targetShape -> validateName(name, targetShape, memberShape, patterns, model))); } private List validateSimpleShape( - Shape shape, - List patterns + Shape shape ) { String name = shape.getId().getName(); @@ -201,9 +191,7 @@ private Optional validateName( Model model ) { ShapeType type = targetShape.getType(); - if (type == ShapeType.TIMESTAMP) { - return Optional.empty(); - } else if (type == ShapeType.ENUM) { + if (type == ShapeType.TIMESTAMP || type == ShapeType.ENUM) { return Optional.empty(); } else if (type == ShapeType.STRUCTURE || type == ShapeType.LIST) { if (this.onlyContainsTimestamps(targetShape, model)) {