Add a diagnostic when an extension override is used to access a stati…

…c member. Also added missing awaits to several previously committed tests. Change-Id: Ide2d714b02e36f097f7c958b609dd780ff333253 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109420 Reviewed-by: Phil Quitslund <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
dart-lang · Jul 17, 2019 · 227ff83 · 227ff83
1 parent c304336
commit 227ff83
Show file tree

Hide file tree

Showing 14 changed files with 160 additions and 59 deletions.
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
@@ -138,6 +138,7 @@ const List<ErrorCode> errorCodeValues = const [
   CompileTimeErrorCode.EXTENSION_DECLARES_ABSTRACT_METHOD,
   CompileTimeErrorCode.EXTENSION_DECLARES_CONSTRUCTOR,
   CompileTimeErrorCode.EXTENSION_DECLARES_INSTANCE_FIELD,
+  CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER,
   CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS,
   CompileTimeErrorCode.EXTRA_POSITIONAL_ARGUMENTS_COULD_BE_NAMED,
   CompileTimeErrorCode.FIELD_INITIALIZED_BY_MULTIPLE_INITIALIZERS,

diff --git a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart
@@ -411,17 +411,24 @@ class MethodInvocationResolver {
   void _resolveExtensionOverride(MethodInvocation node,
       ExtensionOverride override, SimpleIdentifier nameNode, String name) {
     ExtensionElement element = override.extensionName.staticElement;
-    Element member = element.getMethod(name) ??
-        element.getGetter(name) ??
-        element.getSetter(name);
+    Element member = element.getMethod(name) ?? element.getGetter(name);
     if (member == null) {
       _resolver.errorReporter.reportErrorForNode(
           CompileTimeErrorCode.UNDEFINED_EXTENSION_METHOD,
           nameNode,
           [name, element.name]);
     } else if (member is ExecutableElement && member.isStatic) {
-      // TODO(brianwilkerson) Report this error.
+      _resolver.errorReporter.reportErrorForNode(
+          CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER,
+          nameNode);
+    }
+    if (node.isCascaded) {
+      // TODO(brianwilkerson) Report this error and decide how to recover.
+      throw new UnsupportedError('cascaded extension override');
     }
+    // TODO(brianwilkerson) Handle the case where the name resolved to a getter.
+    //  It might be that the getter returns a function that is being invoked, or
+    //  it might be an error to have an argument list.
     nameNode.staticElement = member;
   }
 

diff --git a/pkg/analyzer/lib/src/error/codes.dart b/pkg/analyzer/lib/src/error/codes.dart
@@ -1199,6 +1199,16 @@ class CompileTimeErrorCode extends ErrorCode {
           correction:
               "Try removing the field declaration or making it a static field.");
 
+  /**
+   * No parameters.
+   */
+  static const CompileTimeErrorCode EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER =
+      const CompileTimeErrorCode(
+          'EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER',
+          "An extension override can't be used to access a static member from an "
+              "extension.",
+          correction: "Try using just the name of the extension.");
+
   /**
    * 12.14.2 Binding Actuals to Formals: It is a static warning if <i>m &lt;
    * h</i> or if <i>m &gt; n</i>.

diff --git a/pkg/analyzer/lib/src/fasta/ast_builder.dart b/pkg/analyzer/lib/src/fasta/ast_builder.dart
@@ -272,7 +272,9 @@ class AstBuilder extends StackListener {
       assert(staticToken.isModifier);
       String className = classDeclaration != null
           ? classDeclaration.name.name
-          : mixinDeclaration.name.name;
+          : (mixinDeclaration != null
+              ? mixinDeclaration.name.name
+              : extensionDeclaration.name?.name);
       if (name?.lexeme == className && getOrSet == null) {
         // This error is also reported in OutlineBuilder.beginMethod
         handleRecoverableError(

diff --git a/pkg/analyzer/lib/src/generated/element_resolver.dart b/pkg/analyzer/lib/src/generated/element_resolver.dart
@@ -710,8 +710,9 @@ class ElementResolver extends SimpleAstVisitor<void> {
         }
       }
       if (member != null && member.isStatic) {
-        // TODO(brianwilkerson) Report this error.
-        throw new UnsupportedError('extension override of static member');
+        _resolver.errorReporter.reportErrorForNode(
+            CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER,
+            propertyName);
       }
       propertyName.staticElement = member;
       return;
@@ -1496,9 +1497,6 @@ class ElementResolver extends SimpleAstVisitor<void> {
               CompileTimeErrorCode.UNDEFINED_EXTENSION_METHOD,
               node.operator,
               [methodName, element.name]);
-        } else if (member.isStatic) {
-          // TODO(brianwilkerson) Report this error.
-          throw new UnsupportedError('extension override of static member');
         }
         node.staticElement = member;
         return;

diff --git a/pkg/analyzer/test/src/diagnostics/extension_declares_abstract_method_test.dart b/pkg/analyzer/test/src/diagnostics/extension_declares_abstract_method_test.dart
@@ -22,8 +22,8 @@ class ExtensionDeclaresAbstractMethodTest extends DriverResolutionTest {
     ..contextFeatures = new FeatureSet.forTesting(
         sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
 
-  test_getter() {
-    assertErrorsInCode('''
+  test_getter() async {
+    await assertErrorsInCode('''
 extension E on String {
   bool get isPalindrome;
 }
@@ -32,8 +32,8 @@ extension E on String {
     ]);
   }
 
-  test_method() {
-    assertErrorsInCode('''
+  test_method() async {
+    await assertErrorsInCode('''
 extension E on String {
   String reversed();
 }
@@ -42,14 +42,14 @@ extension E on String {
     ]);
   }
 
-  test_none() {
-    assertNoErrorsInCode('''
+  test_none() async {
+    await assertNoErrorsInCode('''
 extension E on String {}
 ''');
   }
 
-  test_operator() {
-    assertErrorsInCode('''
+  test_operator() async {
+    await assertErrorsInCode('''
 extension E on String {
   String operator -(String otherString);
 }
@@ -58,8 +58,8 @@ extension E on String {
     ]);
   }
 
-  test_setter() {
-    assertErrorsInCode('''
+  test_setter() async {
+    await assertErrorsInCode('''
 extension E on String {
   set length(int newLength);
 }

diff --git a/pkg/analyzer/test/src/diagnostics/extension_declares_constructor_test.dart b/pkg/analyzer/test/src/diagnostics/extension_declares_constructor_test.dart
@@ -22,22 +22,22 @@ class ExtensionDeclaresConstructorTest extends DriverResolutionTest {
     ..contextFeatures = new FeatureSet.forTesting(
         sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
 
-  test_named() {
-    assertErrorsInCode('''
+  test_named() async {
+    await assertErrorsInCode('''
 extension E on String {
   E.named() : super();
 }
 ''', [error(CompileTimeErrorCode.EXTENSION_DECLARES_CONSTRUCTOR, 28, 5)]);
   }
 
-  test_none() {
-    assertNoErrorsInCode('''
+  test_none() async {
+    await assertNoErrorsInCode('''
 extension E on String {}
 ''');
   }
 
-  test_unnamed() {
-    assertErrorsInCode('''
+  test_unnamed() async {
+    await assertErrorsInCode('''
 extension E on String {
   E() : super();
 }

diff --git a/pkg/analyzer/test/src/diagnostics/extension_declares_field_test.dart b/pkg/analyzer/test/src/diagnostics/extension_declares_field_test.dart
@@ -22,8 +22,8 @@ class ExtensionDeclaresFieldTest extends DriverResolutionTest {
     ..contextFeatures = new FeatureSet.forTesting(
         sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
 
-  test_multiple() {
-    assertErrorsInCode('''
+  test_multiple() async {
+    await assertErrorsInCode('''
 extension E on String {
   String one, two, three;
 }
@@ -34,22 +34,22 @@ extension E on String {
     ]);
   }
 
-  test_none() {
-    assertNoErrorsInCode('''
+  test_none() async {
+    await assertNoErrorsInCode('''
 extension E on String {}
 ''');
   }
 
-  test_one() {
-    assertErrorsInCode('''
+  test_one() async {
+    await assertErrorsInCode('''
 extension E on String {
   String s;
 }
 ''', [error(CompileTimeErrorCode.EXTENSION_DECLARES_INSTANCE_FIELD, 33, 1)]);
   }
 
-  test_static() {
-    assertNoErrorsInCode('''
+  test_static() async {
+    await assertNoErrorsInCode('''
 extension E on String {
   static String EMPTY = '';
 }

diff --git a/pkg/analyzer/test/src/diagnostics/extension_override_access_to_static_member_test.dart b/pkg/analyzer/test/src/diagnostics/extension_override_access_to_static_member_test.dart
@@ -0,0 +1,81 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(ExtensionOverrideAccessToStaticMemberTest);
+  });
+}
+
+@reflectiveTest
+class ExtensionOverrideAccessToStaticMemberTest extends DriverResolutionTest {
+  @override
+  AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+    ..contextFeatures = new FeatureSet.forTesting(
+        sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
+
+  test_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static String get empty => '';
+}
+void f() {
+  E('a').empty;
+}
+''', [
+      error(CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER, 79,
+          5),
+    ]);
+  }
+
+  test_getterAndSetter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static String get empty => '';
+  static void set empty(String s) {}
+}
+void f() {
+  E('a').empty += 'b';
+}
+''', [
+      error(CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER,
+          116, 5),
+    ]);
+  }
+
+  test_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static String empty() => '';
+}
+void f() {
+  E('a').empty();
+}
+''', [
+      error(CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER, 77,
+          5),
+    ]);
+  }
+
+  test_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static void set empty(String s) {}
+}
+void f() {
+  E('a').empty = 'b';
+}
+''', [
+      error(CompileTimeErrorCode.EXTENSION_OVERRIDE_ACCESS_TO_STATIC_MEMBER, 83,
+          5),
+    ]);
+  }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/invalid_extension_argument_count_test.dart b/pkg/analyzer/test/src/diagnostics/invalid_extension_argument_count_test.dart
@@ -22,8 +22,8 @@ class InvalidExtensionArgumentCountTest extends DriverResolutionTest {
     ..contextFeatures = new FeatureSet.forTesting(
         sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
 
-  test_many() {
-    assertErrorsInCode('''
+  test_many() async {
+    await assertErrorsInCode('''
 extension E on String {
   void m() {}
 }
@@ -35,8 +35,8 @@ f() {
     ]);
   }
 
-  test_one() {
-    assertNoErrorsInCode('''
+  test_one() async {
+    await assertNoErrorsInCode('''
 extension E on String {
   void m() {}
 }
@@ -46,8 +46,8 @@ f() {
 ''');
   }
 
-  test_zero() {
-    assertErrorsInCode('''
+  test_zero() async {
+    await assertErrorsInCode('''
 extension E on String {
   void m() {}
 }

diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -59,6 +59,8 @@ import 'extension_declares_abstract_method_test.dart'
 import 'extension_declares_constructor_test.dart'
     as extension_declares_constructor;
 import 'extension_declares_field_test.dart' as extension_declares_field;
+import 'extension_override_access_to_static_member_test.dart'
+    as extension_override_access_to_static_member;
 import 'final_not_initialized_test.dart' as final_not_initialized;
 import 'implements_non_class_test.dart' as implements_non_class;
 import 'implicit_this_reference_in_initializer_test.dart'
@@ -220,7 +222,6 @@ main() {
     ambiguous_import.main();
     ambiguous_set_or_map_literal.main();
     argument_type_not_assignable.main();
-
     assignment_to_const.main();
     assignment_to_final_local.main();
     assignment_to_final_no_setter.main();
@@ -260,6 +261,7 @@ main() {
     extension_declares_abstract_method.main();
     extension_declares_constructor.main();
     extension_declares_field.main();
+    extension_override_access_to_static_member.main();
     final_not_initialized.main();
     implements_non_class.main();
     implicit_this_reference_in_initializer.main();

diff --git a/pkg/analyzer/test/src/diagnostics/undefined_extension_getter_test.dart b/pkg/analyzer/test/src/diagnostics/undefined_extension_getter_test.dart
@@ -22,8 +22,8 @@ class UndefinedExtensionGetterTest extends DriverResolutionTest {
     ..contextFeatures = new FeatureSet.forTesting(
         sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
 
-  test_defined() {
-    assertNoErrorsInCode('''
+  test_defined() async {
+    await assertNoErrorsInCode('''
 extension E on String {
   int get g => 0;
 }
@@ -33,8 +33,8 @@ f() {
 ''');
   }
 
-  test_undefined() {
-    assertErrorsInCode('''
+  test_undefined() async {
+    await assertErrorsInCode('''
 extension E on String {}
 f() {
   E('a').g;
@@ -44,8 +44,8 @@ f() {
     ]);
   }
 
-  test_undefined_withSetter() {
-    assertErrorsInCode('''
+  test_undefined_withSetter() async {
+    await assertErrorsInCode('''
 extension E on String {
   void set s(int x) {}
 }