Skip to content

Commit 31bb4df

Browse files
scheglovcommit-bot@chromium.org
authored andcommitted
Move ExtensionOverride errors into resolveOverride().
R=brianwilkerson@google.com, pquitslund@google.com Change-Id: Ia9dae5b461210c6fae40e9cf2d5282aa4fea3b3b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112262 Reviewed-by: Phil Quitslund <pquitslund@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
1 parent 7b46191 commit 31bb4df

File tree

4 files changed

+71
-58
lines changed

4 files changed

+71
-58
lines changed

pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/element/element.dart';
77
import 'package:analyzer/dart/element/type.dart';
8+
import 'package:analyzer/error/listener.dart';
89
import 'package:analyzer/src/dart/ast/ast.dart';
910
import 'package:analyzer/src/dart/element/member.dart';
1011
import 'package:analyzer/src/dart/element/type_algebra.dart';
@@ -15,10 +16,13 @@ import 'package:analyzer/src/generated/type_system.dart';
1516

1617
class ExtensionMemberResolver {
1718
final ResolverVisitor _resolver;
19+
1820
ExtensionMemberResolver(this._resolver);
1921

2022
DartType get _dynamicType => _typeProvider.dynamicType;
2123

24+
ErrorReporter get _errorReporter => _resolver.errorReporter;
25+
2226
Scope get _nameScope => _resolver.nameScope;
2327

2428
TypeProvider get _typeProvider => _resolver.typeProvider;
@@ -48,7 +52,7 @@ class ExtensionMemberResolver {
4852
return extension;
4953
}
5054

51-
_resolver.errorReporter.reportErrorForNode(
55+
_errorReporter.reportErrorForNode(
5256
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_METHOD_ACCESS,
5357
target,
5458
[
@@ -61,24 +65,51 @@ class ExtensionMemberResolver {
6165
}
6266

6367
void resolveOverride(ExtensionOverride node) {
68+
var nodeImpl = node as ExtensionOverrideImpl;
6469
var element = node.staticElement;
6570
var typeParameters = element.typeParameters;
6671

67-
DartType receiverType;
72+
if (!_isValidContext(node)) {
73+
_errorReporter.reportErrorForNode(
74+
CompileTimeErrorCode.EXTENSION_OVERRIDE_WITHOUT_ACCESS,
75+
node,
76+
);
77+
nodeImpl.staticType = _dynamicType;
78+
}
79+
6880
var arguments = node.argumentList.arguments;
69-
if (arguments.length == 1) {
70-
receiverType = arguments[0].staticType;
81+
if (arguments.length != 1) {
82+
_errorReporter.reportErrorForNode(
83+
CompileTimeErrorCode.INVALID_EXTENSION_ARGUMENT_COUNT,
84+
node.argumentList,
85+
);
86+
nodeImpl.typeArgumentTypes = _listOfDynamic(typeParameters);
87+
nodeImpl.extendedType = _dynamicType;
88+
return;
7189
}
72-
// TODO(scheglov) Test when not exactly 1 argument.
7390

74-
var typeArgumentTypes =
75-
_inferTypeArguments(element, receiverType, node.typeArguments);
91+
var receiverExpression = arguments[0];
92+
var receiverType = receiverExpression.staticType;
93+
94+
var typeArgumentTypes = _inferTypeArguments(
95+
element,
96+
receiverType,
97+
node.typeArguments,
98+
);
7699

77-
var nodeImpl = node as ExtensionOverrideImpl;
78100
nodeImpl.typeArgumentTypes = typeArgumentTypes;
79-
nodeImpl.extendedType =
80-
Substitution.fromPairs(typeParameters, typeArgumentTypes)
81-
.substituteType(element.extendedType);
101+
nodeImpl.extendedType = Substitution.fromPairs(
102+
typeParameters,
103+
typeArgumentTypes,
104+
).substituteType(element.extendedType);
105+
106+
if (!_typeSystem.isAssignableTo(receiverType, node.extendedType)) {
107+
_errorReporter.reportErrorForNode(
108+
CompileTimeErrorCode.EXTENSION_OVERRIDE_ARGUMENT_NOT_ASSIGNABLE,
109+
receiverExpression,
110+
[receiverType, node.extendedType],
111+
);
112+
}
82113
}
83114

84115
/// Return the most specific extension or `null` if no single one can be
@@ -259,7 +290,7 @@ class ExtensionMemberResolver {
259290
return arguments.map((a) => a.type).toList();
260291
} else {
261292
// TODO(scheglov) Report an error.
262-
return List.filled(typeParameters.length, _dynamicType);
293+
return _listOfDynamic(typeParameters);
263294
}
264295
} else {
265296
if (receiverType != null) {
@@ -275,7 +306,7 @@ class ExtensionMemberResolver {
275306
);
276307
return inferrer.infer(typeParameters);
277308
} else {
278-
return List.filled(typeParameters.length, _dynamicType);
309+
return _listOfDynamic(typeParameters);
279310
}
280311
}
281312
}
@@ -341,6 +372,22 @@ class ExtensionMemberResolver {
341372
/// Ask the type system for a subtype check.
342373
bool _isSubtypeOf(DartType type1, DartType type2) =>
343374
_typeSystem.isSubtypeOf(type1, type2);
375+
376+
List<DartType> _listOfDynamic(List<TypeParameterElement> parameters) {
377+
return List<DartType>.filled(parameters.length, _dynamicType);
378+
}
379+
380+
/// Return `true` if the extension override [node] is being used as a target
381+
/// of an operation that might be accessing an instance member.
382+
static bool _isValidContext(ExtensionOverride node) {
383+
AstNode parent = node.parent;
384+
return parent is BinaryExpression && parent.leftOperand == node ||
385+
parent is FunctionExpressionInvocation && parent.function == node ||
386+
parent is IndexExpression && parent.target == node ||
387+
parent is MethodInvocation && parent.target == node ||
388+
parent is PrefixExpression ||
389+
parent is PropertyAccess && parent.target == node;
390+
}
344391
}
345392

346393
class InstantiatedExtension {

pkg/analyzer/lib/src/generated/error_verifier.dart

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -730,28 +730,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
730730
_enclosingExtension = null;
731731
}
732732

733-
@override
734-
void visitExtensionOverride(ExtensionOverride node) {
735-
NodeList<Expression> arguments = node.argumentList.arguments;
736-
int argCount = arguments.length;
737-
if (argCount == 1) {
738-
_checkForAssignableExpression(
739-
arguments[0],
740-
node.extendedType,
741-
CompileTimeErrorCode.EXTENSION_OVERRIDE_ARGUMENT_NOT_ASSIGNABLE,
742-
);
743-
} else {
744-
_errorReporter.reportErrorForNode(
745-
CompileTimeErrorCode.INVALID_EXTENSION_ARGUMENT_COUNT,
746-
node.argumentList);
747-
}
748-
if (!_isExtensionOverrideInValidContext(node)) {
749-
_errorReporter.reportErrorForNode(
750-
CompileTimeErrorCode.EXTENSION_OVERRIDE_WITHOUT_ACCESS, node);
751-
}
752-
super.visitExtensionOverride(node);
753-
}
754-
755733
@override
756734
void visitFieldDeclaration(FieldDeclaration node) {
757735
_isInStaticVariableDeclaration = node.isStatic;
@@ -6466,21 +6444,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
64666444
return element.name == "List" && element.library.isDartCore;
64676445
}
64686446

6469-
/// Return `true` if the extension override [node] is being used as a target
6470-
/// of an operation that might be accessing an instance member.
6471-
bool _isExtensionOverrideInValidContext(ExtensionOverride node) {
6472-
AstNode parent = node.parent;
6473-
if ((parent is PropertyAccess && parent.target == node) ||
6474-
(parent is MethodInvocation && parent.target == node) ||
6475-
(parent is FunctionExpressionInvocation && parent.function == node) ||
6476-
(parent is BinaryExpression && parent.leftOperand == node) ||
6477-
(parent is IndexExpression && parent.target == node) ||
6478-
parent is PrefixExpression) {
6479-
return true;
6480-
}
6481-
return false;
6482-
}
6483-
64846447
bool _isFunctionType(DartType type) {
64856448
if (type.isDynamic || type.isDartCoreNull) {
64866449
return true;

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class ExtensionOverrideWithoutAccessTest extends DriverResolutionTest {
2323
sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
2424

2525
test_binaryExpression() async {
26-
assertNoErrorsInCode('''
26+
await assertNoErrorsInCode('''
2727
class C {}
2828
extension E on C {
2929
int operator +(int x) => x;
@@ -49,7 +49,7 @@ f(C c) {
4949
}
5050

5151
test_expressionStatement() async {
52-
assertErrorsInCode('''
52+
await assertErrorsInCode('''
5353
class C {}
5454
extension E on C {
5555
void m() {}
@@ -58,10 +58,11 @@ f(C c) {
5858
E(c);
5959
}
6060
''', [error(CompileTimeErrorCode.EXTENSION_OVERRIDE_WITHOUT_ACCESS, 57, 4)]);
61+
assertTypeDynamic(findNode.extensionOverride('E(c)'));
6162
}
6263

6364
test_getter() async {
64-
assertNoErrorsInCode('''
65+
await assertNoErrorsInCode('''
6566
class C {}
6667
extension E on C {
6768
int get g => 0;
@@ -73,7 +74,7 @@ f(C c) {
7374
}
7475

7576
test_indexExpression_get() async {
76-
assertNoErrorsInCode('''
77+
await assertNoErrorsInCode('''
7778
class C {}
7879
extension E on C {
7980
int operator [](int i) => 4;
@@ -85,7 +86,7 @@ f(C c) {
8586
}
8687

8788
test_indexExpression_set() async {
88-
assertNoErrorsInCode('''
89+
await assertNoErrorsInCode('''
8990
class C {}
9091
extension E on C {
9192
void operator []=(int i, int v) {}
@@ -97,7 +98,7 @@ f(C c) {
9798
}
9899

99100
test_methodInvocation() async {
100-
assertNoErrorsInCode('''
101+
await assertNoErrorsInCode('''
101102
class C {}
102103
extension E on C {
103104
void m() {}
@@ -109,7 +110,7 @@ f(C c) {
109110
}
110111

111112
test_prefixExpression() async {
112-
assertNoErrorsInCode('''
113+
await assertNoErrorsInCode('''
113114
class C {}
114115
extension E on C {
115116
int operator -() => 7;
@@ -121,7 +122,7 @@ f(C c) {
121122
}
122123

123124
test_setter() async {
124-
assertNoErrorsInCode('''
125+
await assertNoErrorsInCode('''
125126
class C {}
126127
extension E on C {
127128
set s(int x) {}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ f() {
3333
''', [
3434
error(CompileTimeErrorCode.INVALID_EXTENSION_ARGUMENT_COUNT, 49, 15),
3535
]);
36+
assertElementTypeDynamic(findNode.extensionOverride('E(').extendedType);
3637
}
3738

3839
test_one() async {
@@ -57,5 +58,6 @@ f() {
5758
''', [
5859
error(CompileTimeErrorCode.INVALID_EXTENSION_ARGUMENT_COUNT, 49, 2),
5960
]);
61+
assertElementTypeDynamic(findNode.extensionOverride('E(').extendedType);
6062
}
6163
}

0 commit comments

Comments
 (0)