diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java index 495111f3972..0e5f5fa449c 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java @@ -458,9 +458,11 @@ private void parseCollection(ShapeId id, SourceLocation location, CollectionShap } private void parseMembers(ShapeId id, Set requiredMembers) { + Set definedMembers = new HashSet<>(); Set remaining = requiredMembers.isEmpty() ? requiredMembers : new HashSet<>(requiredMembers); + ws(); expect('{'); // Don't keep any previous state of captured doc comments when @@ -469,9 +471,7 @@ private void parseMembers(ShapeId id, Set requiredMembers) { ws(); if (peek() != '}') { - // Remove the parsed member from the remaining set to detect - // when duplicates are found, or when members are missing. - remaining.remove(parseMember(id, remaining)); + parseMember(id, requiredMembers, definedMembers, remaining); while (!eof()) { ws(); if (peek() == ',') { @@ -484,15 +484,7 @@ private void parseMembers(ShapeId id, Set requiredMembers) { // Trailing comma: "," "}" break; } - - // Special casing to detect invalid members early on, even - // after draining all the valid members. This keeps builders - // from raising confusing errors about invalid members. - if (remaining.isEmpty() && !requiredMembers.isEmpty()) { - parseMember(id, requiredMembers); - } else { - remaining.remove(parseMember(id, remaining)); - } + parseMember(id, requiredMembers, definedMembers, remaining); } else { // Assume '}'; break to enforce. break; @@ -508,14 +500,22 @@ private void parseMembers(ShapeId id, Set requiredMembers) { expect('}'); } - private String parseMember(ShapeId parent, Set requiredMembers) { + private String parseMember(ShapeId parent, Set required, Set defined, Set remaining) { // Parse optional member traits. List memberTraits = parseDocsAndTraits(); SourceLocation memberLocation = currentLocation(); String memberName = ParserUtils.parseIdentifier(this); + if (defined.contains(memberName)) { + // This is a duplicate member name. + throw syntax("Duplicate member of " + parent + ": '" + memberName + '\''); + } + + defined.add(memberName); + remaining.remove(memberName); + // Only enforce "allowedMembers" if it isn't empty. - if (!requiredMembers.isEmpty() && !requiredMembers.contains(memberName)) { + if (!required.isEmpty() && !required.contains(memberName)) { throw syntax("Unexpected member of " + parent + ": '" + memberName + '\''); } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-list-member-names.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-list-member-names.errors index 3fd8eeacc5e..64d55184264 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-list-member-names.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-list-member-names.errors @@ -1 +1 @@ -[ERROR] -: Duplicate shape definition for `com.foo#List$member` found at | Model +[ERROR] -: Parse error at line 5, column 11 near `: `: Duplicate member of com.foo#List: 'member' | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-map-member-names.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-map-member-names.errors index 1655889db59..36f760ab876 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-map-member-names.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-map-member-names.errors @@ -1 +1 @@ -[ERROR] -: Parse error at line 5, column 8 near `: `: Unexpected member of com.foo#Map: 'key' | Model +[ERROR] -: Parse error at line 5, column 8 near `: `: Duplicate member of com.foo#Map: 'key' | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-set-member-names.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-set-member-names.errors index a36d892598d..658912d2def 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-set-member-names.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-set-member-names.errors @@ -1 +1 @@ -[ERROR] -: Duplicate shape definition for `com.foo#Set$member` found at | Model +[ERROR] -: Parse error at line 5, column 11 near `: `: Duplicate member of com.foo#Set: 'member' | Model diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-structure-member-names.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-structure-member-names.errors index 6dd49e0e39d..b97278fd01d 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-structure-member-names.errors +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/loader/dupe-structure-member-names.errors @@ -1 +1 @@ -[ERROR] -: Duplicate shape definition for `com.foo#Struct$foo` found at | Model +[ERROR] -: Parse error at line 5, column 8 near `: `: Duplicate member of com.foo#Struct: 'foo' | Model