Skip to content

Commit 7848124

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Fix error recovery for : super.<keyword>() / : this.<keyword>()
Previously, if a keyword was used in place of a constructor name in a constructor initializer list, it caused an analyzer crash, because as part of error recovery, the AST builder would try to create a SuperConstructorInvocation or a RedirectingConstructorInvocation using a null argument list. This CL fixes the problem by creating a synthetic argument list. It re-uses logic that previously existed for creating a synthetic argument list. To avoid a crash happening later in analysis, it was also necessary to modify the AST cloner so that when it encounters the `(` and `)` tokens in the synthetic argument list, it is able to clone them even though they are not in the token stream. (Note that this is not an ideal solution; I would have rather inserted the `(` and `)` into the token stream, but with the current parser architecture there's no good way to do this without the parser then trying to interpret those tokens). Change-Id: Ibbdcdd956d80c16d427ba1ba7a9cd7ce374e941b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/201282 Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 50640bd commit 7848124

File tree

5 files changed

+83
-13
lines changed

5 files changed

+83
-13
lines changed

pkg/analyzer/lib/src/dart/ast/utilities.dart

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class AstCloner implements AstVisitor<AstNode> {
9292
if (_lastClonedOffset <= token.offset) {
9393
_cloneTokens(_nextToClone ?? token, token.offset);
9494
}
95-
return _clonedTokens[token]!;
95+
return _clonedTokens[token] ?? _cloneSyntheticToken(token);
9696
} else {
9797
return token;
9898
}
@@ -1090,6 +1090,15 @@ class AstCloner implements AstVisitor<AstNode> {
10901090
cloneNode(node.expression),
10911091
cloneToken(node.semicolon));
10921092

1093+
/// Clones a synthetic token that isn't linked up to the rest of the token
1094+
/// list.
1095+
Token _cloneSyntheticToken(Token token) {
1096+
assert(token.isSynthetic);
1097+
assert(token.next == null);
1098+
assert(token.previous == null);
1099+
return token.copy();
1100+
}
1101+
10931102
/// Clone all token starting from the given [token] up to a token that has
10941103
/// offset greater then [stopAfter], and put mapping from originals to clones
10951104
/// into [_clonedTokens].

pkg/analyzer/lib/src/fasta/ast_builder.dart

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -485,15 +485,15 @@ class AstBuilder extends StackListener {
485485
// This error is also reported in the body builder
486486
handleRecoverableError(messageInvalidSuperInInitializer,
487487
target.superKeyword, target.superKeyword);
488-
return ast.superConstructorInvocation(
489-
target.superKeyword, null, null, argumentList!);
488+
return ast.superConstructorInvocation(target.superKeyword, null, null,
489+
argumentList ?? _syntheticArgumentList(target.superKeyword));
490490
} else if (target is ThisExpression) {
491491
// TODO(danrubel): Consider generating this error in the parser
492492
// This error is also reported in the body builder
493493
handleRecoverableError(messageInvalidThisInInitializer,
494494
target.thisKeyword, target.thisKeyword);
495-
return ast.redirectingConstructorInvocation(
496-
target.thisKeyword, null, null, argumentList!);
495+
return ast.redirectingConstructorInvocation(target.thisKeyword, null,
496+
null, argumentList ?? _syntheticArgumentList(target.thisKeyword));
497497
}
498498
return null;
499499
}
@@ -3537,14 +3537,8 @@ class AstBuilder extends StackListener {
35373537
// TODO(paulberry): once we have visitor support for constructor
35383538
// tear-offs, fall through and return a FunctionReference instead since
35393539
// that should lead to better quality error recovery.
3540-
var syntheticOffset = typeArguments.rightBracket.end;
3541-
push(ast.functionExpressionInvocation(
3542-
receiver,
3543-
typeArguments,
3544-
ast.argumentList(
3545-
SyntheticToken(TokenType.OPEN_PAREN, syntheticOffset),
3546-
[],
3547-
SyntheticToken(TokenType.CLOSE_PAREN, syntheticOffset))));
3540+
push(ast.functionExpressionInvocation(receiver, typeArguments,
3541+
_syntheticArgumentList(typeArguments.rightBracket)));
35483542
return;
35493543
}
35503544
push(ast.functionReference(
@@ -3871,6 +3865,14 @@ class AstBuilder extends StackListener {
38713865
return ast.variableDeclaration(name, equals, initializer);
38723866
}
38733867

3868+
ArgumentList _syntheticArgumentList(Token precedingToken) {
3869+
int syntheticOffset = precedingToken.end;
3870+
return ast.argumentList(
3871+
SyntheticToken(TokenType.OPEN_PAREN, syntheticOffset),
3872+
[],
3873+
SyntheticToken(TokenType.CLOSE_PAREN, syntheticOffset));
3874+
}
3875+
38743876
SimpleIdentifier _tmpSimpleIdentifier() {
38753877
return ast.simpleIdentifier(
38763878
StringToken(TokenType.STRING, '__tmp', -1),

pkg/analyzer/test/generated/simple_parser_test.dart

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,33 @@ class C<@Foo.bar(const [], const [1], const{"":r""}, 0xFF + 2, .3, 4.5) T> {}
117117
expect(metadata.name.name, 'Foo.bar');
118118
}
119119

120+
void test_classDeclaration_invalid_super() {
121+
parseCompilationUnit('''
122+
class C {
123+
C() : super.const();
124+
}
125+
''', errors: [
126+
expectedError(ParserErrorCode.INVALID_SUPER_IN_INITIALIZER, 18, 5),
127+
expectedError(ParserErrorCode.EXPECTED_IDENTIFIER_BUT_GOT_KEYWORD, 24, 5),
128+
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 24, 5),
129+
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 29, 1),
130+
]);
131+
}
132+
133+
void test_classDeclaration_invalid_this() {
134+
parseCompilationUnit('''
135+
class C {
136+
C() : this.const();
137+
}
138+
''', errors: [
139+
expectedError(ParserErrorCode.MISSING_ASSIGNMENT_IN_INITIALIZER, 18, 4),
140+
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 23, 5),
141+
expectedError(ParserErrorCode.MISSING_FUNCTION_BODY, 23, 5),
142+
expectedError(ParserErrorCode.CONST_METHOD, 23, 5),
143+
expectedError(ParserErrorCode.MISSING_IDENTIFIER, 28, 1),
144+
]);
145+
}
146+
120147
void test_method_name_notNull_37733() {
121148
// https://github.com/dart-lang/sdk/issues/37733
122149
var unit = parseCompilationUnit(r'class C { f(<T>()); }', errors: [
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/src/dart/error/syntactic_errors.dart';
6+
import 'package:test_reflective_loader/test_reflective_loader.dart';
7+
8+
import '../dart/resolution/context_collection_resolution.dart';
9+
10+
main() {
11+
defineReflectiveSuite(() {
12+
defineReflectiveTests(InvalidSuperInInitializerTest);
13+
});
14+
}
15+
16+
@reflectiveTest
17+
class InvalidSuperInInitializerTest extends PubPackageResolutionTest {
18+
test_constructor_name_is_keyword() async {
19+
await assertErrorsInCode('''
20+
class C {
21+
C() : super.const();
22+
}
23+
''', [
24+
error(ParserErrorCode.INVALID_SUPER_IN_INITIALIZER, 18, 5),
25+
error(ParserErrorCode.EXPECTED_IDENTIFIER_BUT_GOT_KEYWORD, 24, 5),
26+
error(ParserErrorCode.MISSING_IDENTIFIER, 24, 5),
27+
error(ParserErrorCode.MISSING_IDENTIFIER, 29, 1),
28+
]);
29+
}
30+
}

pkg/analyzer/test/src/diagnostics/test_all.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ import 'invalid_required_optional_positional_param_test.dart'
334334
import 'invalid_required_positional_param_test.dart'
335335
as invalid_required_positional_param;
336336
import 'invalid_sealed_annotation_test.dart' as invalid_sealed_annotation;
337+
import 'invalid_super_in_initializer_test.dart' as invalid_super_in_initializer;
337338
import 'invalid_super_invocation_test.dart' as invalid_super_invocation;
338339
import 'invalid_type_argument_in_const_list_test.dart'
339340
as invalid_type_argument_in_const_list;
@@ -918,6 +919,7 @@ main() {
918919
invalid_required_optional_positional_param.main();
919920
invalid_required_positional_param.main();
920921
invalid_sealed_annotation.main();
922+
invalid_super_in_initializer.main();
921923
invalid_super_invocation.main();
922924
invalid_type_argument_in_const_list.main();
923925
invalid_type_argument_in_const_map.main();

0 commit comments

Comments
 (0)