Skip to content

Commit 3c55850

Browse files
rchacheSteven Yuan
rchache
authored and
Steven Yuan
committed
New diff evaluator for added required members (smithy-lang#1781)
1 parent f7fa4b3 commit 3c55850

File tree

4 files changed

+148
-1
lines changed

4 files changed

+148
-1
lines changed

‎smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/AddedRequiredMember.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.smithy.diff.evaluators;
17+
18+
import java.util.List;
19+
import java.util.stream.Collectors;
20+
import java.util.stream.Stream;
21+
import software.amazon.smithy.diff.Differences;
22+
import software.amazon.smithy.model.shapes.MemberShape;
23+
import software.amazon.smithy.model.shapes.StructureShape;
24+
import software.amazon.smithy.model.traits.DefaultTrait;
25+
import software.amazon.smithy.model.traits.RequiredTrait;
26+
import software.amazon.smithy.model.validation.Severity;
27+
import software.amazon.smithy.model.validation.ValidationEvent;
28+
29+
/**
30+
* Validates that no members are newly created with the required trait
31+
* (but no default trait) in existing structures.
32+
*/
33+
public class AddedRequiredMember extends AbstractDiffEvaluator {
34+
@Override
35+
public List<ValidationEvent> evaluate(Differences differences) {
36+
List<ValidationEvent> events = newRequiredMembers(differences)
37+
.map(this::emit)
38+
.collect(Collectors.toList());
39+
40+
return events;
41+
}
42+
43+
private Stream<MemberShape> newRequiredMembers(Differences differences) {
44+
return differences.changedShapes(StructureShape.class)
45+
.flatMap(change -> change.getNewShape().members().stream()
46+
.filter(newMember -> newMember.hasTrait(RequiredTrait.ID)
47+
&& !newMember.hasTrait(DefaultTrait.ID)
48+
// Members that did not exist before
49+
&& change.getOldShape().getAllMembers().get(newMember.getMemberName()) == null));
50+
}
51+
52+
private ValidationEvent emit(MemberShape memberShape) {
53+
return ValidationEvent.builder()
54+
.id(getEventId())
55+
.shapeId(memberShape.getId())
56+
.message("Adding a new member with the `required` trait "
57+
+ "but not the `default` trait is backwards-incompatible.")
58+
.severity(Severity.ERROR)
59+
.build();
60+
}
61+
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ private ValidationEvent emit(
163163
.shapeId(shape)
164164
.message(actualMessage)
165165
.severity(severity)
166-
.message(message)
167166
.build();
168167
}
169168
}

‎smithy-diff/src/main/resources/META-INF/services/software.amazon.smithy.diff.DiffEvaluator‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
software.amazon.smithy.diff.evaluators.AddedEntityBinding
22
software.amazon.smithy.diff.evaluators.AddedMetadata
33
software.amazon.smithy.diff.evaluators.AddedOperationError
4+
software.amazon.smithy.diff.evaluators.AddedRequiredMember
45
software.amazon.smithy.diff.evaluators.AddedServiceError
56
software.amazon.smithy.diff.evaluators.AddedShape
67
software.amazon.smithy.diff.evaluators.AddedTraitDefinition

‎smithy-diff/src/test/java/software/amazon/smithy/diff/evaluators/AddedRequiredMemberTest.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
package software.amazon.smithy.diff.evaluators;
2+
3+
import org.junit.jupiter.api.Test;
4+
import software.amazon.smithy.diff.ModelDiff;
5+
import software.amazon.smithy.model.Model;
6+
import software.amazon.smithy.model.SourceLocation;
7+
import software.amazon.smithy.model.node.StringNode;
8+
import software.amazon.smithy.model.shapes.StringShape;
9+
import software.amazon.smithy.model.shapes.StructureShape;
10+
import software.amazon.smithy.model.traits.DefaultTrait;
11+
import software.amazon.smithy.model.traits.RequiredTrait;
12+
import software.amazon.smithy.model.validation.Severity;
13+
14+
import static org.hamcrest.MatcherAssert.assertThat;
15+
import static org.hamcrest.Matchers.equalTo;
16+
17+
public class AddedRequiredMemberTest {
18+
@Test
19+
public void addingRequiredTraitWithoutDefaultIsAnError() {
20+
StringShape s = StringShape.builder().id("smithy.example#Str").build();
21+
StructureShape a = StructureShape.builder().id("smithy.example#A")
22+
.build();
23+
StructureShape b = StructureShape.builder().id("smithy.example#A")
24+
.addMember("foo", s.getId(), b2 -> b2.addTrait(new RequiredTrait()))
25+
.build();
26+
Model model1 = Model.builder().addShapes(s, a).build();
27+
Model model2 = Model.builder().addShapes(s, b).build();
28+
ModelDiff.Result result = ModelDiff.builder().oldModel(model1).newModel(model2).compare();
29+
30+
assertThat(TestHelper.findEvents(result.getDiffEvents(), Severity.ERROR).size(), equalTo(1));
31+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").size(), equalTo(1));
32+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").get(0).getShapeId().get().toString(),
33+
equalTo("smithy.example#A$foo"));
34+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").get(0).getMessage(),
35+
equalTo("Adding a new member with the `required` trait " +
36+
"but not the `default` trait is backwards-incompatible."));
37+
}
38+
39+
@Test
40+
public void addingRequiredTraitWithDefaultIsOk() {
41+
StringShape s = StringShape.builder().id("smithy.example#Str").build();
42+
StructureShape a = StructureShape.builder().id("smithy.example#A")
43+
.build();
44+
StructureShape b = StructureShape.builder().id("smithy.example#A")
45+
.addMember("foo", s.getId(), b2 -> {
46+
b2.addTrait(new RequiredTrait());
47+
b2.addTrait(new DefaultTrait(new StringNode("default", SourceLocation.NONE)));
48+
})
49+
.build();
50+
Model model1 = Model.builder().addShapes(s, a).build();
51+
Model model2 = Model.builder().addShapes(s, b).build();
52+
ModelDiff.Result result = ModelDiff.builder().oldModel(model1).newModel(model2).compare();
53+
54+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").size(), equalTo(0));
55+
}
56+
57+
@Test
58+
public void addingRequiredTraitToExistingMember() {
59+
StringShape s = StringShape.builder().id("smithy.example#Str").build();
60+
StructureShape a = StructureShape.builder().id("smithy.example#A")
61+
.addMember("foo", s.getId())
62+
.build();
63+
StructureShape b = StructureShape.builder().id("smithy.example#A")
64+
.addMember("foo", s.getId(),
65+
b2 -> b2.addTrait(new RequiredTrait()))
66+
.build();
67+
Model model1 = Model.builder().addShapes(s, a).build();
68+
Model model2 = Model.builder().addShapes(s, b).build();
69+
ModelDiff.Result result = ModelDiff.builder().oldModel(model1).newModel(model2).compare();
70+
71+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").size(), equalTo(0));
72+
}
73+
74+
@Test
75+
public void addingNewStructureWithRequiredMemberIsOk() {
76+
StringShape s = StringShape.builder().id("smithy.example#Str").build();
77+
StructureShape b = StructureShape.builder().id("smithy.example#A")
78+
.addMember("foo", s.getId(), b2 -> b2.addTrait(new RequiredTrait()))
79+
.build();
80+
Model model1 = Model.builder().addShapes(s).build();
81+
Model model2 = Model.builder().addShapes(s, b).build();
82+
ModelDiff.Result result = ModelDiff.builder().oldModel(model1).newModel(model2).compare();
83+
84+
assertThat(TestHelper.findEvents(result.getDiffEvents(), "AddedRequiredMember").size(), equalTo(0));
85+
}
86+
}

0 commit comments

Comments
 (0)