Improve fasta scanner error recovery

[srawlins]

We have a smoke test for a tool that uses analysis_server, that sends the following Dart content for analysis:

void main() {
  print((('hello world');
add()
}

(Note the syntax errors.) The analysis server used to return 3x errors, but now returns 8. I can't tell if this behavior is better, or expected :) It wasn't announced.

$ # OLD
$ dartanalyzer --version
dartanalyzer version 1.24.0-dev.6.8
$ dartanalyzer a.dart                                                                                              
Analyzing a.dart...
  error • Expected to find ')' at a.dart:2:25 • expected_token
  error • The function 'add' isn't defined at a.dart:3:1 • undefined_function
  error • Expected to find ';' at a.dart:3:5 • expected_token
3 errors found.
$
$ # NEW
$ dartanalyzer --version
dartanalyzer version 1.25.0-dev.7.0
$ dartanalyzer a.dart 
Analyzing a.dart...
  error • Expected to find ')' at a.dart:2:8 • expected_token
  error • Expected to find ')' at a.dart:2:9 • expected_token
  error • Expected to find ')' at a.dart:2:25 • expected_token
  error • The function 'add' isn't defined at a.dart:3:1 • undefined_function
  error • Expected to find ';' at a.dart:3:5 • expected_token
  error • Expected an identifier at a.dart:4:1 • missing_identifier
  error • Unexpected text ')' at a.dart:4:1 • unexpected_token
  error • Expected to find ';' at a.dart:4:1 • expected_token
8 errors found.

[danrubel]

Yes, that is expected, but not optimal in this particular example. The difference is in how the old scanner and new scanner recover given unbalanced curly braces, parenthesis, and square brackets.

Old scanner: When openers and closers are mismatched, consider the closer to be mismatched.
{ ( } ) parses as a pair of matched parenthesis with an unmatched open curly brace before them and an unmatched closing curly brace between them.

New scanner: When openers and closers are mismatched, consider the opener to be mismatched and insert synthetic closers as needed. { ( } ) is parsed as { ( ) } ) where the first ) is synthetic and the trailing ) is unmatched.

In this case, the new scanner is inserting two synthetic ) before the } and generating more errors as a result. The errors that the old scanner/parser produced are much better in this situation. I'll investigate ways to improve the new scanner/parser.

[srawlins]

Interesting, thanks for the info, Dan! I won't presume to know any priority for this, but thanks for triaging!

[danrubel]

Possible improvements:

• reword the error message to point to the opener so as not to mislead the developer. In this example, Expected to find ')' at a.dart:2:8 to Expected to find ')' for '(' at a.dart:2:7
• since ; denotes the end of an expression, insert missing ) before ; rather than before }

[bwilkerson]

reword the error message to point to the opener so as not to mislead the developer.

Or Missing closing paren but highlight the opening paren? If we really can't tell where the closing paren should be maybe we shouldn't pretend we can.

since ; denotes the end of an expression, insert missing ) before ;

Not sure whether it would fall into this trap or whether it has more context than I'm assuming, but...

for (int i = 0; i < count; i++

It would be unfortunate to insert the ) before either of the ;s.

[danrubel]

Good points. I like your rewording better than mine, and yes my suggestion would fail given yourfor loop example.

[bwilkerson]

Why was this issue closed? As far as I know the issue has not yet been addressed, and it's an important prerequisite to moving the analyzer onto the front end. If it has been addressed, please document what was done to address the issue.

[kmillikin]

It was closed in error because it somehow got moved into the wrong bucket while triaging issues. I apologize.

[danrubel]

At this point, recovery has improved enough that the parser again only reports 3 errors for the example above. Closing this as fixed for now. Feel free to reopen this or perhaps a new issue if there are additional recovery and/or error message improvements to be made.