Skip to content

Commit a9f127d

Browse files
stereotype441Commit Queue
authored andcommitted
[analyzer] Rewrite ResultComparator.
Previously, this class was based on `AstComparator` (and was the sole user of `AstComparator`). The new implementation is based on `AstNodeImpl.namedChildEntities`. In a follow-up CL I'll remove `AstComparator`, which should make future modifications to the analyzer AST classes just a little bit easier and less error-prone. I've gone ahead and made some improvements to `ResultComparator` in the process: - I've improved the failure messages when there is a token mismatch, so that the token details (synthetic/non-synthetic, token type, lexeme, and length) are shown, as well as the path to the mismatched token in the AST. - I've improved the syntax for showing the path to the mismatch. Previously all that was shown was the types of the ancestor AST nodes, which was useful but didn't give complete information. Now the types are shown as well as the getter names and indices necessary to reach the mismatch. For example, a mismatch between `class X {}` and `class C {}` is now shown with the path `((root as CompilationUnit).declarations[0] as ClassDeclaration).name`. - I've tightened up the rules for token matching. An "actual" token that is synthetic no longer matches just any old "expected" token; the expected token must be `_s_` for a synthetic identifier, or an empty string for a synthetic string. This required some minor fixes to be made to `try_statement_test.dart`. - Documentation comments are now fully checked; previously only comment references were checked. Note that the tests `ModifiersTest.test_classDeclaration_static` and `MissingCodeTest.test_stringInterpolation_unclosed` now pass. These tests exercise parser recovery scenarios where a token is either ignored or synthetically inserted at the very beginning or end of a compilation unit. These tests were failing under the old `AstComparator`-based implementation of `ResultComparator`, because in addition to recursively comparing the AST structures of a compilation unit, `AstComparator` compares `CompilationUnit.beginToken` and `CompilationUnit.endToken`. These fields store the true starting and ending tokens of the compilation unit (not the results of parser error recovery), so as a result, the old `AstComparator`-based implementation of `ResultComparator` was reporting that parser error recovery had failed when it had in fact succeeded. The new implementation of `ResultComparator` only compares the AST structures, so it correctly concludes that parser error recovery has succeeded. Change-Id: Ifac2065b823e604e07a64500eb2b6b2be125802d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/419521 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent d7e0c3c commit a9f127d

File tree

5 files changed

+386
-120
lines changed

5 files changed

+386
-120
lines changed

pkg/analyzer/test/src/fasta/recovery/extra_code_test.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,7 @@ final Map v = {
163163
/// Test how well the parser recovers when extra modifiers are provided.
164164
@reflectiveTest
165165
class ModifiersTest extends AbstractRecoveryTest {
166-
@failingTest
167166
void test_classDeclaration_static() {
168-
// TODO(danrubel): Fails because compilation unit begin token is `static`
169-
// even after recovery.
170167
testRecovery('''
171168
static class A {}
172169
''', [ParserErrorCode.EXTRANEOUS_MODIFIER], '''

pkg/analyzer/test/src/fasta/recovery/missing_code_test.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,6 @@ f() {
453453
testUserDefinableOperatorWithSuper('*');
454454
}
455455

456-
@failingTest
457456
void test_stringInterpolation_unclosed() {
458457
// https://github.com/dart-lang/sdk/issues/946
459458
// TODO(brianwilkerson): Try to recover better. Ideally there would be a

pkg/analyzer/test/src/fasta/recovery/partial_code/try_statement_test.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class TryStatementTest extends PartialCodeTest {
6060
ParserErrorCode.CATCH_SYNTAX,
6161
ParserErrorCode.EXPECTED_CATCH_CLAUSE_BODY
6262
],
63-
"try {} catch (e) {}",
63+
"try {} catch (_s_) {}",
6464
failing: ['block']),
6565
TestDescriptor(
6666
'catch_leftParen',
@@ -70,7 +70,7 @@ class TryStatementTest extends PartialCodeTest {
7070
ParserErrorCode.CATCH_SYNTAX,
7171
ParserErrorCode.EXPECTED_CATCH_CLAUSE_BODY
7272
],
73-
"try {} catch (e) {}",
73+
"try {} catch (_s_) {}",
7474
failing: ['block', 'labeled', 'localFunctionNonVoid']),
7575
TestDescriptor(
7676
'catch_identifier',
@@ -120,7 +120,7 @@ class TryStatementTest extends PartialCodeTest {
120120
ParserErrorCode.CATCH_SYNTAX,
121121
ParserErrorCode.EXPECTED_CATCH_CLAUSE_BODY
122122
],
123-
"try {} on A catch (e) {}",
123+
"try {} on A catch (_s_) {}",
124124
failing: ['block']),
125125
TestDescriptor(
126126
'on_catch_leftParen',
@@ -130,7 +130,7 @@ class TryStatementTest extends PartialCodeTest {
130130
ParserErrorCode.EXPECTED_CATCH_CLAUSE_BODY,
131131
ScannerErrorCode.EXPECTED_TOKEN
132132
],
133-
"try {} on A catch (e) {}",
133+
"try {} on A catch (_s_) {}",
134134
failing: ['block', 'labeled', 'localFunctionNonVoid']),
135135
TestDescriptor(
136136
'on_catch_identifier',

0 commit comments

Comments
 (0)