fix: suppress cascading syntax errors in method bodies (#422)#456
Merged
Conversation
When a method body fails to parse (e.g. an invalid escape '\s' in a
string), ANTLR's recovery produced a noisy cluster of diagnostics on
top of the original error:
- the lexer reported the bad escape twice (once at the backslash,
again at the closing quote when recovery re-entered the string)
- control-flow analysis ran on the partial AST and emitted a
spurious "Expected return statement" / "Method does not return"
Both are now suppressed:
- OuterBlock tracks whether its on-demand block parse produced
issues; verify() still runs per-statement verification (so
rename / navigation / references keep working on the salvaged
AST) but skips verifyControlPath on failure.
- A new statementsOrErrors() returns Either[ParseErrors, Seq[Statement]]
for callers that want to distinguish failure from an empty block;
statements() retains its best-effort Seq behaviour.
- CollectingErrorListener collapses adjacent "Invalid escape sequence"
diagnostics on the same line — the typical lexer-recovery cascade —
while leaving unrelated same-line errors (e.g. two genuine missing
';' errors) untouched.
nawforce
approved these changes
May 25, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #422.
When a method body fails to parse (e.g. an invalid escape
'\s'in a string), ANTLR's recovery produced a noisy cluster of diagnostics on top of the original error:Expected return statement/Method does not return a valueBoth are now suppressed. Example: a class with a single
'\s'invalid escape used to report 4 diagnostics; it now reports a singleInvalid escape sequence '\s' in string.Changes
OuterBlocktracks whether its on-demand block parse produced issues.verify()still runs per-statement verification on the salvaged AST so downstream features (rename, navigation, references) keep working — onlyverifyControlPathis skipped on parse failure.statementsOrErrors()is a new sibling tostatements()returningEither[ParseErrors, Seq[Statement]]for callers that want to distinguish a parse failure from a genuinely empty block.statements()retains its existing best-effortSeqbehaviour, so existing call-sites are unaffected.CollectingErrorListenercollapses adjacentInvalid escape sequencediagnostics on the same line — the typical lexer-recovery cascade — while leaving unrelated same-line errors (e.g. two genuine missing;errors) untouched.Test plan
BlockTestcases cover:Expected return statement/Code path does not return/Method does not returnstatementsOrErrorsreturnsLeft(ParseErrors)on failure,Right(stmts)on successInvalid escape sequencediagnostic is collapsedsbt scalafmtCheckAllclean