Skip to content

Commit

Permalink
Add parse failure tests (#1820)
Browse files Browse the repository at this point in the history
* Add parse failure tests

Adds test cases that validate the parser fails when expected by
asserting that any descendant of the root is an error tree. There
is a test per-production, if that production has terminal tokens,
or represents an alternation of productions that are distinguished
by terminal tokens. For example, SHAPE_OR_APPLY_STATEMENT chooses
between shape and apply statements based on whether the current
lexemme is "apply".

Only cases where the terminal tokens are invalid are tested because
if a non-terminal (ie. another production) is invalid, the error tree
is a child of the tree for that production. Since we also have tests
that make sure the correct children are present for each production,
we should have enough coverage to only test invalid non-terminals.
For example, when parsing this shape statement:
```
structure Foo.Bar {}
```
we know the `Foo.Bar` is part of the IDENTIFIER production, we have
tests that validate the the IDENTIFIER production is present, and
we have tests that check for errors in IDENTIFIERs, so we don't
need to specifically test this kind of error for shape statements.

One caveat is that the existing tests I'm referring to only ensure
all expected children are present for valid trees, so there is a
logical inconsistency in the argument that these tests provide full
coverage, but I can't think of a case that could cause a problem.
Either way, these tests should provide _good enough_ coverage.

Some bug fixes:
- Fixed an issue where the error caused by not finding a "namespace"
in NAMESPACE_STATEMENT was recovered by the parent tree.
- Expect IDENTIFIER for "with" token in MIXINS production.

* Add cases for productions with loops

Adds more test cases for `*Statement` productions to ensure loops
are broken out of properly when parsing fails.
  • Loading branch information
milesziemer authored and mtdowling committed Jun 19, 2023
1 parent e0d5e85 commit 78bd805
Show file tree
Hide file tree
Showing 2 changed files with 547 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,19 @@ void parse(CapturingTokenizer tokenizer) {
NAMESPACE_STATEMENT {
@Override
void parse(CapturingTokenizer tokenizer) {
if (tokenizer.isCurrentLexeme("namespace")) {
tokenizer.withState(this, () -> {
tokenizer.withState(this, () -> {
if (tokenizer.isCurrentLexeme("namespace")) {
tokenizer.next(); // skip "namespace"
SP.parse(tokenizer);
NAMESPACE.parse(tokenizer);
BR.parse(tokenizer);
});
} else if (tokenizer.hasNext()) {
throw new ModelSyntaxException(
"Expected a namespace definition but found "
+ tokenizer.getCurrentToken().getDebug(tokenizer.getCurrentTokenLexeme()),
tokenizer.getCurrentTokenLocation());
}
} else if (tokenizer.hasNext()) {
throw new ModelSyntaxException(
"Expected a namespace definition but found "
+ tokenizer.getCurrentToken().getDebug(tokenizer.getCurrentTokenLexeme()),
tokenizer.getCurrentTokenLocation());
}
});
}
},

Expand Down
Loading

0 comments on commit 78bd805

Please sign in to comment.