Skip to content

Commit 54e6468

Browse files
authored
Add a sufix to distinguish added required traits to structures or unions (#2793)
1 parent b51e9fa commit 54e6468

File tree

3 files changed

+52
-9
lines changed

3 files changed

+52
-9
lines changed

gradle/libs.versions.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
[versions]
22
junit5 = "5.13.4"
3+
junit-platform = "1.13.4"
34
hamcrest = "3.0"
45
jmh = "0.7.2"
56
spotbugs = "6.2.4"
@@ -40,7 +41,7 @@ junit-bom = { module = "org.junit:junit-bom", version.ref = "junit5" }
4041
junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit5" }
4142
junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit5" }
4243
junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit5" }
43-
junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher" }
44+
junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher", version.ref = "junit-platform" }
4445
hamcrest = { module = "org.hamcrest:hamcrest", version.ref = "hamcrest" }
4546
assertj-core = { module = "org.assertj:assertj-core", version.ref = "assertj" }
4647
mockserver = { module = "org.mock-server:mockserver-netty", version.ref = "mockserver" }

smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedNullability.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,24 @@ private void createErrors(
124124
}
125125
// Can't add required to a member unless the member is marked as @clientOptional or part of @input.
126126
if (change.isTraitAdded(RequiredTrait.ID) && !newMember.hasTrait(ClientOptionalTrait.ID)) {
127-
eventsToAdd.add(emit(Severity.ERROR,
128-
"AddedRequiredTrait",
129-
shape,
130-
oldMemberSourceLocation,
131-
message,
132-
"The @required trait was added to a member."));
127+
if (newTarget.isStructureShape() || newTarget.isUnionShape()) {
128+
eventsToAdd.add(emit(Severity.WARNING,
129+
"AddedRequiredTrait.StructureOrUnion",
130+
shape,
131+
oldMemberSourceLocation,
132+
message,
133+
"The @required trait was added to a member "
134+
+ "that targets a " + newTarget.getType() + ". This is backward compatible in "
135+
+ "generators that always treat structures and unions as optional "
136+
+ "(e.g., AWS generators)"));
137+
} else {
138+
eventsToAdd.add(emit(Severity.ERROR,
139+
"AddedRequiredTrait",
140+
shape,
141+
oldMemberSourceLocation,
142+
message,
143+
"The @required trait was added to a member."));
144+
}
133145
}
134146
// Can't add the default trait to a member unless the member was previously required.
135147
if (change.isTraitAdded(DefaultTrait.ID) && !change.isTraitRemoved(RequiredTrait.ID)) {

smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/ChangedNullabilityTest.java

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ public void specialHandlingForRequiredStructureMembers() {
396396
}
397397

398398
@Test
399-
public void specialHandlingForRequiredUnionMembers() {
399+
public void specialHandlingForRemovedRequiredUnionMembers() {
400400
SourceLocation memberSource = new SourceLocation("b.smithy", 3, 4);
401401
MemberShape memberA = MemberShape.builder().id("smithy.example#Baz$a").target("smithy.api#String").build();
402402
MemberShape memberB = MemberShape.builder().id("smithy.example#Baz$B").target("smithy.api#String").build();
@@ -418,7 +418,37 @@ public void specialHandlingForRequiredUnionMembers() {
418418

419419
List<ValidationEvent> events = TestHelper.findEvents(
420420
ModelDiff.compare(oldModel, newModel),
421-
"ChangedNullability");
421+
"ChangedNullability.RemovedRequiredTrait.StructureOrUnion");
422+
423+
assertThat(events, hasSize(1));
424+
assertThat(events.get(0).getSeverity(), is(Severity.WARNING));
425+
assertThat(events.get(0).getSourceLocation(), equalTo(memberSource));
426+
}
427+
428+
@Test
429+
public void specialHandlingForAddedRequiredUnionMembers() {
430+
SourceLocation memberSource = new SourceLocation("b.smithy", 3, 4);
431+
MemberShape memberA = MemberShape.builder().id("smithy.example#Baz$a").target("smithy.api#String").build();
432+
MemberShape memberB = MemberShape.builder().id("smithy.example#Baz$B").target("smithy.api#String").build();
433+
UnionShape union = UnionShape.builder().id("smithy.example#Baz").addMember(memberA).addMember(memberB).build();
434+
MemberShape memberBaz = MemberShape.builder()
435+
.id("smithy.example#Foo$baz")
436+
.addTrait(new RequiredTrait())
437+
.target(union)
438+
.source(memberSource)
439+
.build();
440+
StructureShape struct = StructureShape.builder().id("smithy.example#Foo").addMember(memberBaz).build();
441+
Model newModel = Model.assembler().addShapes(union, struct, memberA, memberB, memberBaz).assemble().unwrap();
442+
Model oldModel = ModelTransformer.create()
443+
.replaceShapes(newModel,
444+
ListUtils.of(
445+
Shape.shapeToBuilder(newModel.expectShape(ShapeId.from("smithy.example#Foo$baz")))
446+
.removeTrait(RequiredTrait.ID)
447+
.build()));
448+
449+
List<ValidationEvent> events = TestHelper.findEvents(
450+
ModelDiff.compare(oldModel, newModel),
451+
"ChangedNullability.AddedRequiredTrait.StructureOrUnion");
422452

423453
assertThat(events, hasSize(1));
424454
assertThat(events.get(0).getSeverity(), is(Severity.WARNING));

0 commit comments

Comments
 (0)