Report EXTENSION_CONFLICTING_STATIC_AND_INSTANCE and DUPLICATE_DEFINI…

…TION for extensions. [email protected] Change-Id: I237ee3edc6a2196693638c12c59d1530a1f15152 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112605 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
dart-lang · Aug 9, 2019 · 9713ce1 · 9713ce1
1 parent a244295
commit 9713ce1
Show file tree

Hide file tree

Showing 7 changed files with 381 additions and 4 deletions.
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
@@ -137,6 +137,7 @@ const List<ErrorCode> errorCodeValues = const [
   CompileTimeErrorCode.EXTENDS_DEFERRED_CLASS,
   CompileTimeErrorCode.EXTENDS_DISALLOWED_CLASS,
   CompileTimeErrorCode.EXTENDS_NON_CLASS,
+  CompileTimeErrorCode.EXTENSION_CONFLICTING_STATIC_AND_INSTANCE,
   CompileTimeErrorCode.EXTENSION_DECLARES_ABSTRACT_MEMBER,
   CompileTimeErrorCode.EXTENSION_DECLARES_CONSTRUCTOR,
   CompileTimeErrorCode.EXTENSION_DECLARES_INSTANCE_FIELD,

diff --git a/pkg/analyzer/lib/src/error/codes.dart b/pkg/analyzer/lib/src/error/codes.dart
@@ -1268,6 +1268,22 @@ class CompileTimeErrorCode extends ErrorCode {
           correction: "Try specifying a different superclass, or "
               "removing the extends clause.");
 
+  /**
+   * It is for an extension to define a static member and an instance member
+   * with the same base name.
+   *
+   * Parameters:
+   * 0: the name of the extension defining the conflicting member
+   * 1: the name of the conflicting static member
+   */
+  static const CompileTimeErrorCode EXTENSION_CONFLICTING_STATIC_AND_INSTANCE =
+      const CompileTimeErrorCode(
+          'EXTENSION_CONFLICTING_STATIC_AND_INSTANCE',
+          "Extension '{0}' can't define static member '{1}' and instance "
+              "member with the same name.",
+          correction:
+              "Try renaming the member to a name that doesn't conflict.");
+
   /**
    * No parameters.
    */

diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -726,6 +726,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitExtensionDeclaration(ExtensionDeclaration node) {
     _enclosingExtension = node.declaredElement;
+    _checkDuplicateExtensionMembers(node.members);
     super.visitExtensionDeclaration(node);
     _enclosingExtension = null;
   }
@@ -1739,6 +1740,44 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  /**
+   * Check that there are no members with the same name.
+   */
+  void _checkDuplicateExtensionMembers(List<ClassMember> members) {
+    var instanceGetters = <String, Element>{};
+    var instanceSetters = <String, Element>{};
+    var staticGetters = <String, Element>{};
+    var staticSetters = <String, Element>{};
+
+    for (var member in members) {
+      if (member is MethodDeclaration) {
+        _checkDuplicateIdentifier(
+          member.isStatic ? staticGetters : instanceGetters,
+          member.name,
+          setterScope: member.isStatic ? staticSetters : instanceSetters,
+        );
+      }
+    }
+
+    // Check for local static members conflicting with local instance members.
+    for (var member in members) {
+      if (member is MethodDeclaration) {
+        if (member.isStatic) {
+          var identifier = member.name;
+          var name = identifier.name;
+          if (instanceGetters.containsKey(name) ||
+              instanceSetters.containsKey(name)) {
+            _errorReporter.reportErrorForNode(
+              CompileTimeErrorCode.EXTENSION_CONFLICTING_STATIC_AND_INSTANCE,
+              identifier,
+              [_enclosingExtension.name, name],
+            );
+          }
+        }
+      }
+    }
+  }
+
   /**
    * Check whether the given [element] defined by the [identifier] is already
    * in one of the scopes - [getterScope] or [setterScope], and produce an

diff --git a/pkg/analyzer/test/src/diagnostics/duplicate_definition_extension_test.dart b/pkg/analyzer/test/src/diagnostics/duplicate_definition_extension_test.dart
@@ -0,0 +1,143 @@
+// 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(DuplicateDefinitionExtensionTest);
+  });
+}
+
+@reflectiveTest
+class DuplicateDefinitionExtensionTest extends DriverResolutionTest {
+  @override
+  AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+    ..contextFeatures = new FeatureSet.forTesting(
+        sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
+
+  CompileTimeErrorCode get _errorCode =>
+      CompileTimeErrorCode.DUPLICATE_DEFINITION;
+
+  test_extendedType_instance() async {
+    await assertNoErrorsInCode('''
+class A {
+  int get foo => 0;
+  set foo(_) {}
+  void bar() {}
+}
+
+extension E on A {
+  int get foo => 0;
+  set foo(_) {}
+  void bar() {}
+}
+''');
+  }
+
+  test_extendedType_static() async {
+    await assertNoErrorsInCode('''
+class A {
+  static int get foo => 0;
+  static set foo(_) {}
+  static void bar() {}
+}
+
+extension E on A {
+  static int get foo => 0;
+  static set foo(_) {}
+  static void bar() {}
+}
+''');
+  }
+
+  test_instance_getter_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  int get foo => 0;
+  int get foo => 0;
+}
+''', [
+      error(_errorCode, 54, 3),
+    ]);
+  }
+
+  test_instance_getter_setter() async {
+    await assertNoErrorsInCode('''
+extension E on String {
+  int get foo => 0;
+  set foo(_) {}
+}
+''');
+  }
+
+  test_instance_method_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  void foo() {}
+  void foo() {}
+}
+''', [
+      error(_errorCode, 47, 3),
+    ]);
+  }
+
+  test_instance_setter_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  set foo(_) {}
+  set foo(_) {}
+}
+''', [
+      error(_errorCode, 46, 3),
+    ]);
+  }
+
+  test_static_getter_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static int get foo => 0;
+  static int get foo => 0;
+}
+''', [
+      error(_errorCode, 68, 3),
+    ]);
+  }
+
+  test_static_getter_setter() async {
+    await assertNoErrorsInCode('''
+extension E on String {
+  static int get foo => 0;
+  static set foo(_) {}
+}
+''');
+  }
+
+  test_static_method_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static void foo() {}
+  static void foo() {}
+}
+''', [
+      error(_errorCode, 61, 3),
+    ]);
+  }
+
+  test_static_setter_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static set foo(_) {}
+  static set foo(_) {}
+}
+''', [
+      error(_errorCode, 60, 3),
+    ]);
+  }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/extension_conflicting_static_and_instance_test.dart b/pkg/analyzer/test/src/diagnostics/extension_conflicting_static_and_instance_test.dart
@@ -0,0 +1,168 @@
+// 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(ExtensionConflictingStaticAndInstanceTest);
+  });
+}
+
+@reflectiveTest
+class ExtensionConflictingStaticAndInstanceTest extends DriverResolutionTest {
+  @override
+  AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+    ..contextFeatures = new FeatureSet.forTesting(
+        sdkVersion: '2.3.0', additionalFeatures: [Feature.extension_methods]);
+
+  CompileTimeErrorCode get _errorCode =>
+      CompileTimeErrorCode.EXTENSION_CONFLICTING_STATIC_AND_INSTANCE;
+
+  test_extendedType_getter() async {
+    await assertNoErrorsInCode('''
+class A {
+  static int get foo => 0;
+  int get bar => 0;
+}
+
+extension E on A {
+  int get foo => 0;
+  static int get bar => 0;
+}
+''');
+  }
+
+  test_extendedType_method() async {
+    await assertNoErrorsInCode('''
+class A {
+  static void foo() {}
+  void bar() {}
+}
+
+extension E on A {
+  void foo() {}
+  static void bar() {}
+}
+''');
+  }
+
+  test_extendedType_setter() async {
+    await assertNoErrorsInCode('''
+class A {
+  static set foo(_) {}
+  set bar(_) {}
+}
+
+extension E on A {
+  set foo(_) {}
+  static set bar(_) {}
+}
+''');
+  }
+
+  test_getter_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static int get foo => 0;
+  int get foo => 0;
+}
+''', [
+      error(_errorCode, 41, 3),
+    ]);
+  }
+
+  test_getter_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static int get foo => 0;
+  void foo() {}
+}
+''', [
+      error(_errorCode, 41, 3),
+    ]);
+  }
+
+  test_getter_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static int get foo => 0;
+  set foo(_) {}
+}
+''', [
+      error(_errorCode, 41, 3),
+    ]);
+  }
+
+  test_method_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static void foo() {}
+  int get foo => 0;
+}
+''', [
+      error(_errorCode, 38, 3),
+    ]);
+  }
+
+  test_method_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static void foo() {}
+  void foo() {}
+}
+''', [
+      error(_errorCode, 38, 3),
+    ]);
+  }
+
+  test_method_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static void foo() {}
+  set foo(_) {}
+}
+''', [
+      error(_errorCode, 38, 3),
+    ]);
+  }
+
+  test_setter_getter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static set foo(_) {}
+  int get foo => 0;
+}
+''', [
+      error(_errorCode, 37, 3),
+    ]);
+  }
+
+  test_setter_method() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static set foo(_) {}
+  void foo() {}
+}
+''', [
+      error(_errorCode, 37, 3),
+    ]);
+  }
+
+  test_setter_setter() async {
+    await assertErrorsInCode('''
+extension E on String {
+  static set foo(_) {}
+  set foo(_) {}
+}
+''', [
+      error(_errorCode, 37, 3),
+    ]);
+  }
+}