Skip to content

Commit b855e2b

Browse files
authored
Reduce false positives of ShouldHaveUsedTimestampValidator (#2480)
Reduce false positives of ShouldHaveUsedTimestampValidator
1 parent f8ac947 commit b855e2b

File tree

2 files changed

+86
-22
lines changed

2 files changed

+86
-22
lines changed

smithy-linters/src/main/java/software/amazon/smithy/linters/ShouldHaveUsedTimestampValidator.java

+44-22
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.stream.Stream;
2525
import software.amazon.smithy.model.Model;
2626
import software.amazon.smithy.model.node.NodeMapper;
27+
import software.amazon.smithy.model.shapes.EnumShape;
2728
import software.amazon.smithy.model.shapes.MemberShape;
2829
import software.amazon.smithy.model.shapes.NumberShape;
2930
import software.amazon.smithy.model.shapes.Shape;
@@ -112,20 +113,25 @@ public List<ValidationEvent> validate(Model model) {
112113
@Override
113114
protected List<ValidationEvent> getDefault(Shape shape) {
114115
if (shape.isStringShape() || shape instanceof NumberShape) {
115-
return validateSimpleShape(shape, patterns);
116+
return validateSimpleShape(shape);
116117
} else {
117118
return Collections.emptyList();
118119
}
119120
}
120121

121122
@Override
122123
public List<ValidationEvent> structureShape(StructureShape shape) {
123-
return validateStructure(shape, model, patterns);
124+
return validateStructure(shape, model);
124125
}
125126

126127
@Override
127128
public List<ValidationEvent> unionShape(UnionShape shape) {
128-
return validateUnion(shape, model, patterns);
129+
return validateUnion(shape, model);
130+
}
131+
132+
@Override
133+
public List<ValidationEvent> enumShape(EnumShape shape) {
134+
return Collections.emptyList();
129135
}
130136
};
131137

@@ -134,57 +140,63 @@ public List<ValidationEvent> unionShape(UnionShape shape) {
134140

135141
private List<ValidationEvent> validateStructure(
136142
StructureShape structure,
137-
Model model,
138-
List<Pattern> patterns
143+
Model model
139144
) {
140145
return structure
141146
.getAllMembers()
142147
.entrySet()
143148
.stream()
144-
.flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model, patterns))
149+
.flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model))
145150
.collect(Collectors.toList());
146151
}
147152

148153
private List<ValidationEvent> validateUnion(
149154
UnionShape union,
150-
Model model,
151-
List<Pattern> patterns
155+
Model model
152156
) {
153157
return union
154158
.getAllMembers()
155159
.entrySet()
156160
.stream()
157-
.flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model, patterns))
161+
.flatMap(entry -> validateTargetShape(entry.getKey(), entry.getValue(), model))
158162
.collect(Collectors.toList());
159163
}
160164

161165
private Stream<ValidationEvent> validateTargetShape(
162166
String name,
163-
MemberShape target,
164-
Model model,
165-
List<Pattern> patterns
167+
MemberShape memberShape,
168+
Model model
166169
) {
167-
return OptionalUtils.stream(model.getShape(target.getTarget())
168-
.flatMap(shape -> validateName(name, shape.getType(), target, patterns)));
170+
return OptionalUtils.stream(model.getShape(memberShape.getTarget())
171+
.flatMap(targetShape -> validateName(name, targetShape, memberShape, patterns, model)));
169172
}
170173

171174
private List<ValidationEvent> validateSimpleShape(
172-
Shape shape,
173-
List<Pattern> patterns
175+
Shape shape
174176
) {
175-
return validateName(shape.getId().getName(), shape.getType(), shape, patterns)
176-
.map(ListUtils::of)
177-
.orElse(ListUtils.of());
177+
String name = shape.getId().getName();
178+
179+
return patterns
180+
.stream()
181+
.filter(pattern -> pattern.matcher(name).matches())
182+
.map(matcher -> buildEvent(shape, name, shape.getType()))
183+
.collect(Collectors.toList());
178184
}
179185

180186
private Optional<ValidationEvent> validateName(
181187
String name,
182-
ShapeType type,
188+
Shape targetShape,
183189
Shape context,
184-
List<Pattern> patterns
190+
List<Pattern> patterns,
191+
Model model
185192
) {
186-
if (type == ShapeType.TIMESTAMP) {
193+
ShapeType type = targetShape.getType();
194+
if (type == ShapeType.TIMESTAMP || type == ShapeType.ENUM) {
187195
return Optional.empty();
196+
} else if (type == ShapeType.STRUCTURE || type == ShapeType.LIST) {
197+
if (this.onlyContainsTimestamps(targetShape, model)) {
198+
return Optional.empty();
199+
}
188200
}
189201
return patterns
190202
.stream()
@@ -193,6 +205,16 @@ private Optional<ValidationEvent> validateName(
193205
.findAny();
194206
}
195207

208+
private boolean onlyContainsTimestamps(Shape shape, Model model) {
209+
return shape.getAllMembers()
210+
.values()
211+
.stream()
212+
.map(memberShape -> model.getShape(memberShape.getTarget()))
213+
.filter(Optional::isPresent)
214+
.map(Optional::get)
215+
.allMatch(Shape::isTimestampShape);
216+
}
217+
196218
private ValidationEvent buildEvent(Shape context, String name, ShapeType type) {
197219
return danger(context, context.isMemberShape()
198220
? String.format("Member `%s` is named like a timestamp but references a `%s` shape", name, type)

smithy-linters/src/test/resources/software/amazon/smithy/linters/errorfiles/should-have-used-timestamp-ignore.json

+42
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,48 @@
2424
},
2525
"example.namespace#CreatedOn": {
2626
"type": "timestamp"
27+
},
28+
"example.namespace#DateEnum": {
29+
"type": "enum",
30+
"members": {
31+
"ANOMALOUS": {
32+
"target": "smithy.api#Unit",
33+
"traits": {
34+
"smithy.api#enumValue": "anomalous"
35+
}
36+
},
37+
"NORMAL": {
38+
"target": "smithy.api#Unit",
39+
"traits": {
40+
"smithy.api#enumValue": "normal"
41+
}
42+
}
43+
}
44+
},
45+
"example.namespace#DateStructureOuter": {
46+
"type": "structure",
47+
"members": {
48+
"TimestampStructure": {
49+
"target": "example.namespace#DateStructure"
50+
},
51+
"ListOfTimestamp": {
52+
"target": "example.namespace#DateList"
53+
}
54+
}
55+
},
56+
"example.namespace#DateStructure": {
57+
"type": "structure",
58+
"members": {
59+
"TimeOn": {
60+
"target": "example.namespace#CreatedOn"
61+
}
62+
}
63+
},
64+
"example.namespace#DateList": {
65+
"type": "list",
66+
"member": {
67+
"target": "example.namespace#CreatedOn"
68+
}
2769
}
2870
},
2971
"metadata": {

0 commit comments

Comments
 (0)