Skip to content

Commit 12494a4

Browse files
committed
Warn don't fail on additional properties
This PR updates Smithy's loaders to not fail on additional properties, but rather, warn. This allows new features to be added in the future that might not materially change the model but do need new properties added to shapes (like a new resource method, structure parent, etc). Further, various traits were emitting warnings both to a logger and to the validator because they were manually asserting that traits don't have additional properties. This is unnecessary since this is already handled automatically for every trait. This commit removes those redundant checks.
1 parent 32408d1 commit 12494a4

30 files changed

+69
-91
lines changed

smithy-aws-iam-traits/src/main/java/software/amazon/smithy/aws/iam/traits/ConditionKeyDefinition.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,19 @@
1515

1616
package software.amazon.smithy.aws.iam.traits;
1717

18-
import java.util.List;
1918
import java.util.Objects;
2019
import java.util.Optional;
2120
import software.amazon.smithy.model.node.Node;
2221
import software.amazon.smithy.model.node.ObjectNode;
2322
import software.amazon.smithy.model.node.StringNode;
2423
import software.amazon.smithy.model.node.ToNode;
25-
import software.amazon.smithy.utils.ListUtils;
2624
import software.amazon.smithy.utils.SmithyBuilder;
2725
import software.amazon.smithy.utils.ToSmithyBuilder;
2826

2927
public final class ConditionKeyDefinition implements ToNode, ToSmithyBuilder<ConditionKeyDefinition> {
3028
private static final String TYPE = "type";
3129
private static final String DOCUMENTATION = "documentation";
3230
private static final String EXTERNAL_DOCUMENTATION = "externalDocumentation";
33-
private static final List<String> SUPPORTED_PROPERTIES = ListUtils.of(TYPE, DOCUMENTATION, EXTERNAL_DOCUMENTATION);
3431

3532
private final String type;
3633
private final String documentation;
@@ -48,7 +45,6 @@ public static Builder builder() {
4845

4946
public static ConditionKeyDefinition fromNode(Node value) {
5047
ObjectNode objectNode = value.expectObjectNode();
51-
objectNode.warnIfAdditionalProperties(SUPPORTED_PROPERTIES);
5248
Builder builder = builder()
5349
.type(objectNode.expectStringMember(TYPE).getValue());
5450
objectNode.getStringMember(DOCUMENTATION).map(StringNode::getValue)

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ArnReferenceTrait.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
package software.amazon.smithy.aws.traits;
1717

18-
import java.util.List;
1918
import java.util.Optional;
2019
import software.amazon.smithy.model.node.Node;
2120
import software.amazon.smithy.model.node.ObjectNode;
@@ -24,7 +23,6 @@
2423
import software.amazon.smithy.model.traits.AbstractTrait;
2524
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
2625
import software.amazon.smithy.model.traits.Trait;
27-
import software.amazon.smithy.utils.ListUtils;
2826
import software.amazon.smithy.utils.ToSmithyBuilder;
2927

3028
/**
@@ -36,7 +34,6 @@ public final class ArnReferenceTrait extends AbstractTrait implements ToSmithyBu
3634
private static final String TYPE = "type";
3735
private static final String SERVICE = "service";
3836
private static final String RESOURCE = "resource";
39-
private static final List<String> PROPERTIES = ListUtils.of(TYPE, SERVICE, RESOURCE);
4037

4138
private String type;
4239
private ShapeId service;
@@ -58,7 +55,6 @@ public Provider() {
5855
public Trait createTrait(ShapeId target, Node value) {
5956
ObjectNode objectNode = value.expectObjectNode();
6057
Builder builder = builder();
61-
objectNode.warnIfAdditionalProperties(PROPERTIES);
6258
objectNode.getStringMember(TYPE)
6359
.map(StringNode::getValue)
6460
.ifPresent(builder::type);

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ArnTrait.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import software.amazon.smithy.model.traits.AbstractTrait;
2828
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
2929
import software.amazon.smithy.model.traits.Trait;
30-
import software.amazon.smithy.utils.ListUtils;
3130
import software.amazon.smithy.utils.SmithyBuilder;
3231
import software.amazon.smithy.utils.ToSmithyBuilder;
3332

@@ -42,7 +41,6 @@ public final class ArnTrait extends AbstractTrait implements ToSmithyBuilder<Arn
4241
private static final String ABSOLUTE = "absolute";
4342
private static final String NO_REGION = "noRegion";
4443
private static final String NO_ACCOUNT = "noAccount";
45-
private static final List<String> PROPERTIES = ListUtils.of(TEMPLATE, ABSOLUTE, NO_REGION, NO_ACCOUNT);
4644
private static final Pattern PATTERN = Pattern.compile("\\{([^}]+)}");
4745

4846
private final boolean noRegion;
@@ -74,7 +72,6 @@ public Provider() {
7472
public Trait createTrait(ShapeId target, Node value) {
7573
Builder builder = builder();
7674
ObjectNode objectNode = value.expectObjectNode();
77-
objectNode.warnIfAdditionalProperties(PROPERTIES);
7875
builder.template(objectNode.expectStringMember(TEMPLATE).getValue());
7976
builder.absolute(objectNode.getBooleanMemberOrDefault(ABSOLUTE));
8077
builder.noRegion(objectNode.getBooleanMemberOrDefault(NO_REGION));

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/ServiceTrait.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
package software.amazon.smithy.aws.traits;
1717

18-
import java.util.Arrays;
1918
import java.util.Locale;
2019
import java.util.Optional;
2120
import software.amazon.smithy.model.SourceException;
@@ -60,9 +59,6 @@ public Provider() {
6059
@Override
6160
public Trait createTrait(ShapeId target, Node value) {
6261
ObjectNode objectNode = value.expectObjectNode();
63-
objectNode.warnIfAdditionalProperties(Arrays.asList(
64-
"sdkId", "arnNamespace", "cloudFormationName", "cloudTrailEventSource", "abbreviation"));
65-
6662
Builder builder = builder();
6763
String sdkId = objectNode.getStringMember("sdkId")
6864
.map(StringNode::getValue)

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/CognitoUserPoolsTrait.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public Provider() {
5151

5252
@Override
5353
public Trait createTrait(ShapeId target, Node value) {
54-
ObjectNode objectNode = value.expectObjectNode().warnIfAdditionalProperties(ListUtils.of(PROVIDER_ARNS));
54+
ObjectNode objectNode = value.expectObjectNode();
5555
return builder()
5656
.providerArns(objectNode.expectArrayMember(PROVIDER_ARNS).getElementsAs(StringNode::getValue))
5757
.build();

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/auth/SigV4Trait.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,12 @@
1515

1616
package software.amazon.smithy.aws.traits.auth;
1717

18-
import java.util.Set;
1918
import software.amazon.smithy.model.node.Node;
2019
import software.amazon.smithy.model.node.ObjectNode;
2120
import software.amazon.smithy.model.shapes.ShapeId;
2221
import software.amazon.smithy.model.traits.AbstractTrait;
2322
import software.amazon.smithy.model.traits.AbstractTraitBuilder;
2423
import software.amazon.smithy.model.traits.Trait;
25-
import software.amazon.smithy.utils.SetUtils;
2624
import software.amazon.smithy.utils.SmithyBuilder;
2725
import software.amazon.smithy.utils.ToSmithyBuilder;
2826

@@ -33,7 +31,6 @@ public final class SigV4Trait extends AbstractTrait implements ToSmithyBuilder<S
3331

3432
public static final ShapeId ID = ShapeId.from("aws.auth#sigv4");
3533
private static final String NAME = "name";
36-
private static final Set<String> PROPERTIES = SetUtils.of(NAME);
3734

3835
private final String name;
3936

@@ -90,7 +87,6 @@ public Provider() {
9087
public Trait createTrait(ShapeId target, Node value) {
9188
Builder builder = builder().sourceLocation(value);
9289
ObjectNode objectNode = value.expectObjectNode();
93-
objectNode.warnIfAdditionalProperties(PROPERTIES);
9490
builder.name(objectNode.expectStringMember(NAME).getValue());
9591
return builder.build();
9692
}

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/clientendpointdiscovery/ClientDiscoveredEndpointTrait.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
package software.amazon.smithy.aws.traits.clientendpointdiscovery;
1717

18-
import java.util.Collections;
1918
import software.amazon.smithy.model.node.Node;
2019
import software.amazon.smithy.model.node.ObjectNode;
2120
import software.amazon.smithy.model.shapes.ShapeId;
@@ -90,8 +89,6 @@ public Provider() {
9089
@Override
9190
public ClientDiscoveredEndpointTrait createTrait(ShapeId target, Node value) {
9291
ObjectNode objectNode = value.expectObjectNode();
93-
objectNode.warnIfAdditionalProperties(Collections.singletonList(REQUIRED));
94-
9592
return builder()
9693
.required(objectNode.getBooleanMemberOrDefault(REQUIRED, true))
9794
.build();

smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/clientendpointdiscovery/ClientEndpointDiscoveryTrait.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515

1616
package software.amazon.smithy.aws.traits.clientendpointdiscovery;
1717

18-
import java.util.Arrays;
19-
import java.util.Collections;
20-
import java.util.List;
2118
import software.amazon.smithy.model.node.Node;
2219
import software.amazon.smithy.model.node.ObjectNode;
2320
import software.amazon.smithy.model.shapes.ShapeId;
@@ -34,7 +31,6 @@ public final class ClientEndpointDiscoveryTrait extends AbstractTrait
3431

3532
private static final String OPERATION = "operation";
3633
private static final String ERROR = "error";
37-
private static final List<String> PROPERTIES = Collections.unmodifiableList(Arrays.asList(OPERATION, ERROR));
3834

3935
private final ShapeId operation;
4036
private final ShapeId error;
@@ -139,8 +135,6 @@ public Provider() {
139135
@Override
140136
public ClientEndpointDiscoveryTrait createTrait(ShapeId target, Node value) {
141137
ObjectNode objectNode = value.expectObjectNode();
142-
objectNode.warnIfAdditionalProperties(PROPERTIES);
143-
144138
return builder()
145139
.operation(objectNode.expectStringMember(OPERATION).expectShapeId())
146140
.error(objectNode.expectStringMember(ERROR).expectShapeId())

smithy-model/src/main/java/software/amazon/smithy/model/loader/AstModelLoader.java

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ enum AstModelLoader {
8686
TYPE, "version", "operations", "resources", TRAITS);
8787

8888
void load(ObjectNode model, LoaderVisitor visitor) {
89-
model.expectNoAdditionalProperties(TOP_LEVEL_PROPERTIES);
89+
visitor.checkForAdditionalProperties(model, null, TOP_LEVEL_PROPERTIES);
9090
loadMetadata(model, visitor);
9191
loadShapes(model, visitor);
9292
}
@@ -185,7 +185,7 @@ private void loadShape(ShapeId id, String type, ObjectNode value, LoaderVisitor
185185
loadOperation(id, value, visitor);
186186
break;
187187
case "apply":
188-
value.expectNoAdditionalProperties(APPLY_PROPERTIES);
188+
visitor.checkForAdditionalProperties(value, id, APPLY_PROPERTIES);
189189
applyTraits(id, value.expectObjectMember(TRAITS), visitor);
190190
break;
191191
default:
@@ -205,7 +205,7 @@ private void applyShapeTraits(ShapeId id, ObjectNode node, LoaderVisitor visitor
205205
}
206206

207207
private void loadMember(LoaderVisitor visitor, ShapeId id, ObjectNode targetNode) {
208-
targetNode.expectNoAdditionalProperties(MEMBER_PROPERTIES);
208+
visitor.checkForAdditionalProperties(targetNode, id, MEMBER_PROPERTIES);
209209
MemberShape.Builder builder = MemberShape.builder().source(targetNode.getSourceLocation()).id(id);
210210
ShapeId target = targetNode.expectStringMember(TARGET).expectShapeId();
211211
builder.target(target);
@@ -219,52 +219,52 @@ private void loadCollection(
219219
CollectionShape.Builder builder,
220220
LoaderVisitor visitor
221221
) {
222-
node.expectNoAdditionalProperties(COLLECTION_PROPERTY_NAMES);
222+
visitor.checkForAdditionalProperties(node, id, COLLECTION_PROPERTY_NAMES);
223223
applyShapeTraits(id, node, visitor);
224224
loadMember(visitor, id.withMember("member"), node.expectObjectMember("member"));
225225
visitor.onShape(builder.id(id).source(node.getSourceLocation()));
226226
}
227227

228228
private void loadMap(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
229-
node.expectNoAdditionalProperties(MAP_PROPERTY_NAMES);
229+
visitor.checkForAdditionalProperties(node, id, MAP_PROPERTY_NAMES);
230230
loadMember(visitor, id.withMember("key"), node.expectObjectMember("key"));
231231
loadMember(visitor, id.withMember("value"), node.expectObjectMember("value"));
232232
applyShapeTraits(id, node, visitor);
233233
visitor.onShape(MapShape.builder().id(id).source(node.getSourceLocation()));
234234
}
235235

236-
private void loadOperation(ShapeId operationShapeId, ObjectNode node, LoaderVisitor visitor) {
237-
node.expectNoAdditionalProperties(OPERATION_PROPERTY_NAMES);
238-
applyShapeTraits(operationShapeId, node, visitor);
236+
private void loadOperation(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
237+
visitor.checkForAdditionalProperties(node, id, OPERATION_PROPERTY_NAMES);
238+
applyShapeTraits(id, node, visitor);
239239
OperationShape.Builder builder = OperationShape.builder()
240-
.id(operationShapeId)
240+
.id(id)
241241
.source(node.getSourceLocation())
242-
.addErrors(loadOptionalTargetList(node, "errors"));
242+
.addErrors(loadOptionalTargetList(visitor, id, node, "errors"));
243243

244-
loadOptionalTarget(node, "input").ifPresent(builder::input);
245-
loadOptionalTarget(node, "output").ifPresent(builder::output);
244+
loadOptionalTarget(visitor, id, node, "input").ifPresent(builder::input);
245+
loadOptionalTarget(visitor, id, node, "output").ifPresent(builder::output);
246246
visitor.onShape(builder);
247247
}
248248

249249
private void loadResource(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
250-
node.expectNoAdditionalProperties(RESOURCE_PROPERTIES);
250+
visitor.checkForAdditionalProperties(node, id, RESOURCE_PROPERTIES);
251251
applyShapeTraits(id, node, visitor);
252252
ResourceShape.Builder builder = ResourceShape.builder().id(id).source(node.getSourceLocation());
253-
loadOptionalTarget(node, "put").ifPresent(builder::put);
254-
loadOptionalTarget(node, "create").ifPresent(builder::create);
255-
loadOptionalTarget(node, "read").ifPresent(builder::read);
256-
loadOptionalTarget(node, "update").ifPresent(builder::update);
257-
loadOptionalTarget(node, "delete").ifPresent(builder::delete);
258-
loadOptionalTarget(node, "list").ifPresent(builder::list);
259-
builder.operations(loadOptionalTargetList(node, "operations"));
260-
builder.collectionOperations(loadOptionalTargetList(node, "collectionOperations"));
261-
builder.resources(loadOptionalTargetList(node, "resources"));
253+
loadOptionalTarget(visitor, id, node, "put").ifPresent(builder::put);
254+
loadOptionalTarget(visitor, id, node, "create").ifPresent(builder::create);
255+
loadOptionalTarget(visitor, id, node, "read").ifPresent(builder::read);
256+
loadOptionalTarget(visitor, id, node, "update").ifPresent(builder::update);
257+
loadOptionalTarget(visitor, id, node, "delete").ifPresent(builder::delete);
258+
loadOptionalTarget(visitor, id, node, "list").ifPresent(builder::list);
259+
builder.operations(loadOptionalTargetList(visitor, id, node, "operations"));
260+
builder.collectionOperations(loadOptionalTargetList(visitor, id, node, "collectionOperations"));
261+
builder.resources(loadOptionalTargetList(visitor, id, node, "resources"));
262262

263263
// Load identifiers and resolve forward references.
264264
node.getObjectMember("identifiers").ifPresent(ids -> {
265265
for (Map.Entry<StringNode, Node> entry : ids.getMembers().entrySet()) {
266266
String name = entry.getKey().getValue();
267-
ShapeId target = loadReferenceBody(entry.getValue());
267+
ShapeId target = loadReferenceBody(visitor, id, entry.getValue());
268268
builder.addIdentifier(name, target);
269269
}
270270
});
@@ -273,30 +273,30 @@ private void loadResource(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
273273
}
274274

275275
private void loadService(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
276-
node.expectNoAdditionalProperties(SERVICE_PROPERTIES);
276+
visitor.checkForAdditionalProperties(node, id, SERVICE_PROPERTIES);
277277
applyShapeTraits(id, node, visitor);
278278
ServiceShape.Builder builder = new ServiceShape.Builder().id(id).source(node.getSourceLocation());
279279
builder.version(node.expectStringMember("version").getValue());
280-
builder.operations(loadOptionalTargetList(node, "operations"));
281-
builder.resources(loadOptionalTargetList(node, "resources"));
280+
builder.operations(loadOptionalTargetList(visitor, id, node, "operations"));
281+
builder.resources(loadOptionalTargetList(visitor, id, node, "resources"));
282282
visitor.onShape(builder);
283283
}
284284

285285
private void loadSimpleShape(
286286
ShapeId id, ObjectNode node, AbstractShapeBuilder builder, LoaderVisitor visitor) {
287-
node.expectNoAdditionalProperties(SIMPLE_PROPERTY_NAMES);
287+
visitor.checkForAdditionalProperties(node, id, SIMPLE_PROPERTY_NAMES);
288288
applyShapeTraits(id, node, visitor);
289289
visitor.onShape(builder.id(id).source(node.getSourceLocation()));
290290
}
291291

292292
private void loadStructure(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
293-
node.expectNoAdditionalProperties(STRUCTURE_AND_UNION_PROPERTY_NAMES);
293+
visitor.checkForAdditionalProperties(node, id, STRUCTURE_AND_UNION_PROPERTY_NAMES);
294294
visitor.onShape(StructureShape.builder().id(id).source(node.getSourceLocation()));
295295
finishLoadingStructOrUnionMembers(id, node, visitor);
296296
}
297297

298298
private void loadUnion(ShapeId id, ObjectNode node, LoaderVisitor visitor) {
299-
node.expectNoAdditionalProperties(STRUCTURE_AND_UNION_PROPERTY_NAMES);
299+
visitor.checkForAdditionalProperties(node, id, STRUCTURE_AND_UNION_PROPERTY_NAMES);
300300
visitor.onShape(UnionShape.builder().id(id).source(node.getSourceLocation()));
301301
finishLoadingStructOrUnionMembers(id, node, visitor);
302302
}
@@ -309,20 +309,22 @@ private void finishLoadingStructOrUnionMembers(ShapeId id, ObjectNode node, Load
309309
}
310310
}
311311

312-
private Optional<ShapeId> loadOptionalTarget(ObjectNode node, String member) {
313-
return node.getObjectMember(member).map(this::loadReferenceBody);
312+
private Optional<ShapeId> loadOptionalTarget(
313+
LoaderVisitor visitor, ShapeId id, ObjectNode node, String member) {
314+
return node.getObjectMember(member).map(r -> loadReferenceBody(visitor, id, r));
314315
}
315316

316-
private ShapeId loadReferenceBody(Node reference) {
317+
private ShapeId loadReferenceBody(LoaderVisitor visitor, ShapeId id, Node reference) {
317318
ObjectNode referenceObject = reference.expectObjectNode();
318-
referenceObject.expectNoAdditionalProperties(REFERENCE_PROPERTIES);
319+
visitor.checkForAdditionalProperties(referenceObject, id, REFERENCE_PROPERTIES);
319320
return referenceObject.expectStringMember(TARGET).expectShapeId();
320321
}
321322

322-
private List<ShapeId> loadOptionalTargetList(ObjectNode node, String member) {
323+
private List<ShapeId> loadOptionalTargetList(
324+
LoaderVisitor visitor, ShapeId id, ObjectNode node, String member) {
323325
return node.getArrayMember(member)
324326
.map(array -> array.getElements().stream()
325-
.map(this::loadReferenceBody)
327+
.map(e -> loadReferenceBody(visitor, id, e))
326328
.collect(Collectors.toList()))
327329
.orElseGet(Collections::emptyList);
328330
}

smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelLoader.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ private void parseService() {
885885
.source(sourceLocation);
886886

887887
ObjectNode shapeNode = parseObjectNode(expect(LBRACE).getSourceLocation(), RBRACE);
888-
shapeNode.warnIfAdditionalProperties(SERVICE_PROPERTY_NAMES);
888+
visitor.checkForAdditionalProperties(shapeNode, shapeId, SERVICE_PROPERTY_NAMES);
889889
builder.version(shapeNode.expectStringMember(VERSION_KEY).getValue());
890890
optionalIdList(shapeNode, shapeId.getNamespace(), OPERATIONS_KEY).forEach(builder::addOperation);
891891
optionalIdList(shapeNode, shapeId.getNamespace(), RESOURCES_KEY).forEach(builder::addResource);
@@ -913,7 +913,7 @@ private void parseResource() {
913913
visitor.onShape(builder);
914914
ObjectNode shapeNode = parseObjectNode(expect(LBRACE).getSourceLocation(), RBRACE);
915915

916-
shapeNode.warnIfAdditionalProperties(RESOURCE_PROPERTY_NAMES);
916+
visitor.checkForAdditionalProperties(shapeNode, shapeId, RESOURCE_PROPERTY_NAMES);
917917
optionalId(shapeNode, shapeId.getNamespace(), PUT_KEY).ifPresent(builder::put);
918918
optionalId(shapeNode, shapeId.getNamespace(), CREATE_KEY).ifPresent(builder::create);
919919
optionalId(shapeNode, shapeId.getNamespace(), READ_KEY).ifPresent(builder::read);
@@ -945,7 +945,7 @@ private void parseOperation() {
945945

946946
Token opening = expect(LBRACE);
947947
ObjectNode node = parseObjectNode(opening.getSourceLocation(), RBRACE);
948-
node.expectNoAdditionalProperties(OPERATION_PROPERTY_NAMES);
948+
visitor.checkForAdditionalProperties(node, id, OPERATION_PROPERTY_NAMES);
949949
node.getStringMember("input").ifPresent(input -> {
950950
onShapeTarget(input.getValue(), input, builder::input);
951951
});

0 commit comments

Comments
 (0)