Skip to content

Commit

Permalink
Correct smithy-diff nullability error messages
Browse files Browse the repository at this point in the history
These messages reflect a previous design in Smithy that was refactored
to rename @nullable to @clientOptional, but these error messages and
IDs were missed in Smithy-Diff. This change also simplifies the error
message for adding a required trait to remove extra detail (this
message is only presented when relevant so the detail isn't needed).
  • Loading branch information
mtdowling committed Sep 6, 2023
1 parent b18d3e7 commit 0a67811
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,15 @@ private void createErrors(
eventsToAdd.add(emit(Severity.DANGER, "AddedInputTrait", shape, message,
"The @input trait was added to " + newMember.getContainer()));
} else if (!newHasInput) {
// Can't add nullable to a preexisting required member.
// Can't add clientOptional to a preexisting required member.
if (change.isTraitAdded(ClientOptionalTrait.ID) && change.isTraitInBoth(RequiredTrait.ID)) {
eventsToAdd.add(emit(Severity.ERROR, "AddedNullableTrait", shape, message,
"The @nullable trait was added to a @required member."));
eventsToAdd.add(emit(Severity.ERROR, "AddedClientOptionalTrait", shape, message,
"The @clientOptional trait was added to a @required member."));
}
// Can't add required to a member unless the member is marked as nullable.
// Can't add required to a member unless the member is marked as @clientOptional or part of @input.
if (change.isTraitAdded(RequiredTrait.ID) && !newMember.hasTrait(ClientOptionalTrait.ID)) {
eventsToAdd.add(emit(Severity.ERROR, "AddedRequiredTrait", shape, message,
"The @required trait was added to a member that is not marked as @nullable."));
"The @required trait was added to a member."));
}
// Can't add the default trait to a member unless the member was previously required.
if (change.isTraitAdded(DefaultTrait.ID) && !change.isTraitRemoved(RequiredTrait.ID)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,12 @@ public void detectAdditionOfRequiredTrait() {
assertThat(events.stream()
.filter(event -> event.getSeverity() == Severity.ERROR)
.filter(event -> event.getId().equals("ChangedNullability.AddedRequiredTrait"))
.filter(event -> event.getMessage().contains("The @required trait was added to a member "
+ "that is not marked as @nullable"))
.filter(event -> event.getMessage().contains("The @required trait was added to a member"))
.count(), equalTo(1L));
}

@Test
public void detectAdditionOfNullableTrait() {
public void detectAdditionOfClientOptionalTrait() {
MemberShape member1 = MemberShape.builder()
.id("foo.baz#Baz$bam")
.target("foo.baz#String")
Expand All @@ -181,8 +180,8 @@ public void detectAdditionOfNullableTrait() {

assertThat(events.stream()
.filter(event -> event.getSeverity() == Severity.ERROR)
.filter(event -> event.getId().equals("ChangedNullability.AddedNullableTrait"))
.filter(event -> event.getMessage().contains("The @nullable trait was added to a "
.filter(event -> event.getId().equals("ChangedNullability.AddedClientOptionalTrait"))
.filter(event -> event.getMessage().contains("The @clientOptional trait was added to a "
+ "@required member"))
.count(), equalTo(1L));
}
Expand Down

0 comments on commit 0a67811

Please sign in to comment.