Extension types. Report conflicts for members from superinterfaces.

Change-Id: I4bc513102f734a98a263dffd97c5d93874ef3ae1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/318803
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -644,6 +644,8 @@ CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_MEMBER_OF_OBJECT:
     Offer to delete the invalid member.
 CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_DISALLOWED_TYPE:
   status: noFix
+CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT:
+  status: noFix
 CompileTimeErrorCode.EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF:
   status: noFix
 CompileTimeErrorCode.EXTERNAL_FIELD_CONSTRUCTOR_INITIALIZER:

pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart

@@ -44,6 +44,19 @@ class GetterMethodConflict extends Conflict {
   });
 }
 
+/// The extension type has both an extension and non-extension member
+/// signature with the same name.
+class HasNonExtensionAndExtensionMemberConflict extends Conflict {
+  final List<ExecutableElement> nonExtension;
+  final List<ExecutableElement> extension;
+
+  HasNonExtensionAndExtensionMemberConflict({
+    required super.name,
+    required this.nonExtension,
+    required this.extension,
+  });
+}
+
 /// Manages knowledge about interface types and their members.
 class InheritanceManager3 {
   static final _noSuchMethodName =
@@ -681,10 +694,11 @@ class InheritanceManager3 {
     for (final interface in augmented.interfaces) {
       for (final entry in getInterface(interface.element).map.entries) {
         final name = entry.key;
-        if (interface.element is ExtensionTypeElement) {
-          (extensionCandidates[name] ??= []).add(entry.value);
+        final executable = entry.value;
+        if (executable.enclosingElement is ExtensionTypeElement) {
+          (extensionCandidates[name] ??= []).add(executable);
         } else {
-          (notExtensionCandidates[name] ??= []).add(entry.value);
+          (notExtensionCandidates[name] ??= []).add(executable);
         }
       }
     }
@@ -696,13 +710,26 @@ class InheritanceManager3 {
     for (final entry in extensionCandidates.entries) {
       final name = entry.key;
       final candidates = entry.value;
-      redeclared[name] = candidates;
+      (redeclared[name] ??= []).addAll(candidates);
 
       // Stop if redeclared.
       if (implemented.containsKey(name)) {
         continue;
       }
 
+      // If not redeclared, can have either non-extension, or extension.
+      final nonExtensionSignatures = notExtensionCandidates[name];
+      if (nonExtensionSignatures != null) {
+        conflicts.add(
+          HasNonExtensionAndExtensionMemberConflict(
+            name: name,
+            nonExtension: nonExtensionSignatures,
+            extension: candidates,
+          ),
+        );
+        continue;
+      }
+
       // The inherited member must be unique.
       ExecutableElement? uniqueElement;
       for (final candidate in candidates) {
@@ -737,6 +764,12 @@ class InheritanceManager3 {
         continue;
       }
 
+      // Skip, if also has extension candidates.
+      // The conflict is already reported.
+      if (extensionCandidates.containsKey(name)) {
+        continue;
+      }
+
       final combinedSignature = combineSignatures(
         targetClass: element,
         candidates: candidates,

pkg/analyzer/lib/src/error/codes.g.dart

@@ -1623,6 +1623,18 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
         "Try specifying a different type, or remove the type from the list.",
   );
 
+  ///  Parameters:
+  ///  0: the name of the extension type
+  ///  1: the name of the conflicting member
+  static const CompileTimeErrorCode EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT =
+      CompileTimeErrorCode(
+    'EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT',
+    "The extension type '{0}' has more than one distinct member named '{1}' "
+        "from implemented types.",
+    correctionMessage:
+        "Try redeclaring the corresponding member in this extension type.",
+  );
+
   ///  No parameters.
   static const CompileTimeErrorCode
       EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF = CompileTimeErrorCode(

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -194,6 +194,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_INSTANCE_FIELD,
   CompileTimeErrorCode.EXTENSION_TYPE_DECLARES_MEMBER_OF_OBJECT,
   CompileTimeErrorCode.EXTENSION_TYPE_IMPLEMENTS_DISALLOWED_TYPE,
+  CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT,
   CompileTimeErrorCode.EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF,
   CompileTimeErrorCode.EXTERNAL_FIELD_CONSTRUCTOR_INITIALIZER,
   CompileTimeErrorCode.EXTERNAL_FIELD_INITIALIZER,

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

@@ -29,6 +29,7 @@ import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:analyzer/src/dart/element/well_bounded.dart';
 import 'package:analyzer/src/dart/resolver/scope.dart';
 import 'package:analyzer/src/dart/resolver/variance.dart';
+import 'package:analyzer/src/diagnostic/diagnostic.dart';
 import 'package:analyzer/src/diagnostic/diagnostic_factory.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/error/constructor_fields_verifier.dart';
@@ -710,7 +711,10 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
       _checkForNonCovariantTypeParameterPositionInRepresentationType(
           node, element);
       _checkForExtensionTypeRepresentationDependsOnItself(node, element);
-      // TODO(scheglov) Add checks.
+      _checkForExtensionTypeMemberConflicts(
+        node: node,
+        element: element,
+      );
 
       super.visitExtensionTypeDeclaration(node);
     } finally {
@@ -2890,6 +2894,45 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
     }
   }
 
+  void _checkForExtensionTypeMemberConflicts({
+    required ExtensionTypeDeclaration node,
+    required ExtensionTypeElement element,
+  }) {
+    void report(String memberName, List<ExecutableElement> candidates) {
+      final contextMessages = candidates.map<DiagnosticMessage>((executable) {
+        final container = executable.enclosingElement as InterfaceElement;
+        return DiagnosticMessageImpl(
+          filePath: executable.source.fullName,
+          offset: executable.nameOffset,
+          length: executable.nameLength,
+          message: "Inherited from '${container.name}'",
+          url: null,
+        );
+      }).toList();
+      errorReporter.reportErrorForToken(
+        CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT,
+        node.name,
+        [node.name.lexeme, memberName],
+        contextMessages,
+      );
+    }
+
+    final interface = _inheritanceManager.getInterface(element);
+    for (final conflict in interface.conflicts) {
+      switch (conflict) {
+        case CandidatesConflict _:
+          report(conflict.name.name, conflict.candidates);
+        case HasNonExtensionAndExtensionMemberConflict _:
+          report(conflict.name.name, [
+            ...conflict.nonExtension,
+            ...conflict.extension,
+          ]);
+        case NotUniqueExtensionMemberConflict _:
+          report(conflict.name.name, conflict.candidates);
+      }
+    }
+  }
+
   void _checkForExtensionTypeRepresentationDependsOnItself(
     ExtensionTypeDeclarationImpl node,
     ExtensionTypeElementImpl element,

pkg/analyzer/messages.yaml

@@ -5069,6 +5069,13 @@ CompileTimeErrorCode:
     comment: |-
       Parameters:
       0: the display string of the disallowed type
+  EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT:
+    problemMessage: "The extension type '{0}' has more than one distinct member named '{1}' from implemented types."
+    correctionMessage: Try redeclaring the corresponding member in this extension type.
+    comment: |-
+      Parameters:
+      0: the name of the extension type
+      1: the name of the conflicting member
   EXTENSION_TYPE_REPRESENTATION_DEPENDS_ON_ITSELF:
     problemMessage: "The extension type representation can't depend on itself."
     correctionMessage: Try specifying a different type.

pkg/analyzer/test/src/dart/element/inheritance_manager3_test.dart

@@ -450,6 +450,22 @@ declared
 ''');
   }
 
+  test_declareGetter_static() async {
+    final library = await buildLibrary(r'''
+extension type A(int it) {
+  static int get foo => 0;
+}
+''');
+
+    final element = library.extensionType('A');
+    assertInterfaceText(element, r'''
+map
+  it: self::@extensionType::A::@getter::it
+declared
+  it: self::@extensionType::A::@getter::it
+''');
+  }
+
   test_declareMethod() async {
     final library = await buildLibrary(r'''
 extension type A(int it) {
@@ -468,6 +484,38 @@ declared
 ''');
   }
 
+  test_declareMethod_implementClass_implementExtensionType_wouldConflict() async {
+    final library = await buildLibrary(r'''
+class A {
+  void foo() {}
+}
+
+extension type B(A it) {
+  void foo() {}
+}
+
+extension type C(A it) implements A, B {
+  void foo() {}
+}
+''');
+
+    final element = library.extensionType('C');
+    assertInterfaceText(element, r'''
+map
+  foo: self::@extensionType::C::@method::foo
+  it: self::@extensionType::C::@getter::it
+declared
+  foo: self::@extensionType::C::@method::foo
+  it: self::@extensionType::C::@getter::it
+redeclared
+  foo
+    self::@extensionType::B::@method::foo
+    self::@class::A::@method::foo
+  it
+    self::@extensionType::B::@getter::it
+''');
+  }
+
   test_declareMethod_implementClass_method2_wouldConflict() async {
     final library = await buildLibrary(r'''
 class A {
@@ -683,34 +731,71 @@ declared
 ''');
   }
 
-  test_noDeclaration_implementClass_implementExtensionType() async {
+  test_declareSetter() async {
     final library = await buildLibrary(r'''
 extension type A(int it) {
-  void foo() {}
+  set foo(int _) {}
 }
-
-class B {
-  void foo() {}
-}
-
-class C extends B {}
-
-extension type D(C it) implements B, A {}
 ''');
 
-    final element = library.extensionType('D');
+    final element = library.extensionType('A');
     assertInterfaceText(element, r'''
 map
-  foo: self::@extensionType::A::@method::foo
-  it: self::@extensionType::D::@getter::it
+  foo=: self::@extensionType::A::@setter::foo
+  it: self::@extensionType::A::@getter::it
 declared
-  it: self::@extensionType::D::@getter::it
+  foo=: self::@extensionType::A::@setter::foo
+  it: self::@extensionType::A::@getter::it
+''');
+  }
+
+  test_declareSetter_static() async {
+    final library = await buildLibrary(r'''
+extension type A(int it) {
+  static set foo(int _) {}
+}
+''');
+
+    final element = library.extensionType('A');
+    assertInterfaceText(element, r'''
+map
+  it: self::@extensionType::A::@getter::it
+declared
+  it: self::@extensionType::A::@getter::it
+''');
+  }
+
+  test_noDeclaration_implementClass_implementExtensionType_hasConflict() async {
+    final library = await buildLibrary(r'''
+class A {
+  void foo() {}
+}
+
+extension type B(A it) {
+  void foo() {}
+}
+
+extension type C(A it) implements A, B {}
+''');
+
+    final element = library.extensionType('C');
+    assertInterfaceText(element, r'''
+map
+  it: self::@extensionType::C::@getter::it
+declared
+  it: self::@extensionType::C::@getter::it
 redeclared
   foo
-    self::@extensionType::A::@method::foo
-    self::@class::B::@method::foo
+    self::@extensionType::B::@method::foo
+    self::@class::A::@method::foo
   it
-    self::@extensionType::A::@getter::it
+    self::@extensionType::B::@getter::it
+conflicts
+  HasExtensionAndNotExtensionMemberConflict
+    nonExtension
+      self::@class::A::@method::foo
+    extension
+      self::@extensionType::B::@method::foo
 ''');
   }
 
@@ -805,16 +890,18 @@ class B1 extends A {}
 
 class B2 extends A {}
 
-extension type C(Object it) implements B1, B2 {}
+abstract class C implements B1, B2 {}
+
+extension type D(C it) implements B1, B2 {}
 ''');
 
-    final element = library.extensionType('C');
+    final element = library.extensionType('D');
     assertInterfaceText(element, r'''
 map
   foo: self::@class::A::@method::foo
-  it: self::@extensionType::C::@getter::it
+  it: self::@extensionType::D::@getter::it
 declared
-  it: self::@extensionType::C::@getter::it
+  it: self::@extensionType::D::@getter::it
 redeclared
   foo
     self::@class::A::@method::foo
@@ -2282,6 +2369,20 @@ class _InterfacePrinter {
               'CandidatesConflict',
               conflict.candidates,
             );
+          case HasNonExtensionAndExtensionMemberConflict _:
+            _sink.writelnWithIndent(
+              'HasExtensionAndNotExtensionMemberConflict',
+            );
+            _sink.withIndent(() {
+              _elementPrinter.writeElementList(
+                'nonExtension',
+                conflict.nonExtension,
+              );
+              _elementPrinter.writeElementList(
+                'extension',
+                conflict.extension,
+              );
+            });
           case NotUniqueExtensionMemberConflict _:
             _elementPrinter.writeElementList(
               'NotUniqueExtensionMemberConflict',

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

@@ -0,0 +1,184 @@
+// Copyright (c) 2023, 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/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(
+      ExtensionTypeInheritedMemberConflictTest_extension2,
+    );
+    defineReflectiveTests(
+      ExtensionTypeInheritedMemberConflictTest_extensionAndNot,
+    );
+    defineReflectiveTests(
+      ExtensionTypeInheritedMemberConflictTest_notExtension,
+    );
+  });
+}
+
+@reflectiveTest
+class ExtensionTypeInheritedMemberConflictTest_extension2
+    extends PubPackageResolutionTest {
+  test_conflict() async {
+    await assertErrorsInCode('''
+extension type A1(int it) {
+  void foo() {}
+}
+
+extension type A2(int it) {
+  void foo() {}
+}
+
+extension type B(int it) implements A1, A2 {}
+''', [
+      error(
+        CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT,
+        109,
+        1,
+        contextMessages: [
+          message('/home/test/lib/test.dart', 35, 3),
+          message('/home/test/lib/test.dart', 82, 3),
+        ],
+      ),
+    ]);
+  }
+
+  test_noConflict_redeclared() async {
+    await assertNoErrorsInCode('''
+extension type A1(int it) {
+  void foo() {}
+}
+
+extension type A2(int it) {
+  void foo() {}
+}
+
+extension type B(int it) implements A1, A2 {
+  void foo() {}
+}
+''');
+  }
+
+  test_noConflict_sameDeclaration() async {
+    await assertNoErrorsInCode('''
+extension type A(int it) {
+  void foo() {}
+}
+
+extension type B1(int it) implements A {}
+
+extension type B2(int it) implements A {}
+
+extension type C(int it) implements B1, B2 {}
+''');
+  }
+}
+
+@reflectiveTest
+class ExtensionTypeInheritedMemberConflictTest_extensionAndNot
+    extends PubPackageResolutionTest {
+  test_conflict() async {
+    await assertErrorsInCode('''
+class A {
+  void foo() {}
+}
+
+extension type B(A it) {
+  void foo() {}
+}
+
+extension type C(A it) implements A, B {}
+''', [
+      error(
+        CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT,
+        88,
+        1,
+        contextMessages: [
+          message('/home/test/lib/test.dart', 17, 3),
+          message('/home/test/lib/test.dart', 61, 3),
+        ],
+      ),
+    ]);
+  }
+
+  test_redeclared() async {
+    await assertNoErrorsInCode('''
+class A {
+  void foo() {}
+}
+
+extension type B(A it) {
+  void foo() {}
+}
+
+extension type C(A it) implements A, B {
+  void foo() {}
+}
+''');
+  }
+}
+
+@reflectiveTest
+class ExtensionTypeInheritedMemberConflictTest_notExtension
+    extends PubPackageResolutionTest {
+  test_conflict() async {
+    await assertErrorsInCode('''
+class A {
+  int foo() => 0;
+}
+
+class B {
+  String foo() => '0';
+}
+
+extension type C(Object it) implements A, B {}
+''', [
+      error(
+        CompileTimeErrorCode.EXTENSION_TYPE_INHERITED_MEMBER_CONFLICT,
+        82,
+        1,
+        contextMessages: [
+          message('/home/test/lib/test.dart', 16, 3),
+          message('/home/test/lib/test.dart', 50, 3),
+        ],
+      ),
+    ]);
+  }
+
+  test_noConflict_redeclared() async {
+    await assertNoErrorsInCode('''
+class A {
+  int foo() => 0;
+}
+
+class B {
+  String foo() => '0';
+}
+
+extension type C(Object it) implements A, B {
+  void foo() {}
+}
+''');
+  }
+
+  test_noConflict_sameDeclaration() async {
+    await assertNoErrorsInCode('''
+class A {
+  int foo() => 0;
+}
+
+class B1 extends A {}
+
+class B2 extends A {}
+
+abstract class C implements B1, B2 {}
+
+extension type D(C it) implements B1, B2 {}
+''');
+  }
+}

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

@@ -244,6 +244,8 @@ import 'extension_type_declares_member_of_object_test.dart'
     as extension_type_declares_member_of_object;
 import 'extension_type_implements_disallowed_type_test.dart'
     as extension_type_implements_disallowed_type;
+import 'extension_type_inherited_member_conflict_test.dart'
+    as extension_type_inherited_member_conflict;
 import 'extension_type_representation_depends_on_itself_test.dart'
     as extension_type_representation_depends_on_itself;
 import 'external_field_constructor_initializer_test.dart'
@@ -1047,6 +1049,7 @@ main() {
     extension_type_declares_instance_field.main();
     extension_type_declares_member_of_object.main();
     extension_type_implements_disallowed_type.main();
+    extension_type_inherited_member_conflict.main();
     extension_type_representation_depends_on_itself.main();
     external_field_constructor_initializer.main();
     external_field_initializer.main();