-
Notifications
You must be signed in to change notification settings - Fork 218
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use structure member parsing for lists and maps (#1782)
* Use structure member parsing for lists and maps
This commit moves the validation of list and map members out of the
parser. List and map members have special validation, because they
can only have specific members, and those members can't have default
values. Prior to this commit, we did this validation in the parser,
throwing a parse error if default value syntax sugar was used, or
incorrect member names were used. This meant we needed special
parsing for list and map shape members. It also lead to situations
like the following:
```
list Foo {
member: String = "foo"
}
```
fails to parse, saying it expected a "}" but got a "=", while
```
list Foo {
@default("foo")
member: String
}
```
fails model validation, explaining you can't assign a default to
a list member. Removing special handling of list and map members
consolidates this logic and simplifies the parser.
Because we previously threw parse errors when encountering unexpected
members, this PR had to update CollectionShape and MapShape to throw
SourceExceptions, which become validation events, when the incorrect
members were added to their respective builders. Since both shapes
constructors require the builder as an argument, you can't construct
the shapes with the wrong members.
For testing, cases were added for invalid map and list members, and
for default value syntax on list and map members. Some old test cases
that expected the parse error were removed or adjusted.
This also merges the grammar for list, union, and map shapes.
* Emit both wrong and missing member errors
This change makes the loader emit error messages for both
incorrect members and missing members in lists and maps.
Previously, a single error would stop the loader from continuing
to load the shape.
* Remove extra shape id checks in list/map ctor
Removes extraneous checks in the list/map constructor that
were validating the shape id for list/map members. This validation
is now performed when the members are collected, and errors are
emitted when the members are missing.
* Emit error for all missing members
Previously, an error was only emitted for the first member that
was found to be missing. So if you had an empty map shape, it would
just say you're missing the key. This commit makes the error message
say that both key and value are missing. To do this, Shape was
updated, adding a getRequiredMembers method which stores a list
of missing members, throwing an error after all required members
have been checked.
* Add test cases for invalid list/map mixins
Previously, we didn't have to give dedicated error messages when
you try to mixin a list/map with missing or incorrect members,
because parsing would fail. So we didn't have test cases verifying
this behavior.- Loading branch information
1 parent
5636d15
commit 9cb0328
Showing
34 changed files
with
448 additions
and
205 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.