Change method override inference to use combined member signature.

dart-lang/language#975

Change-Id: I5cec14a384b124a29dcd5adac0f0be6e5214cc9e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151044
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Paul Berry <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: scheglov

pkg/_fe_analyzer_shared/test/inheritance/data/infer_parameter_opt_out.dart

@@ -66,7 +66,8 @@ class C1 extends A implements B {
 /*cfe|cfe:builder.member: C2._simpleInstanceOfTrue:bool* Function(dynamic)**/
 /*cfe|cfe:builder.member: C2.==:bool* Function(dynamic)**/
 class C2 extends B implements A {
-  /*member: C2.method:dynamic Function(dynamic, {dynamic named})**/
+  /*cfe|cfe:builder.member: C2.method:dynamic Function(dynamic, {dynamic named})**/
+  /*analyzer.member: C2.method:Object* Function(Object*, {Object* named})**/
   method(o, {named}) {}
 }
 
@@ -98,7 +99,8 @@ class C3 implements A, B {
 /*cfe|cfe:builder.member: C4._simpleInstanceOfTrue:bool* Function(dynamic)**/
 /*cfe|cfe:builder.member: C4.==:bool* Function(dynamic)**/
 class C4 implements B, A {
-  /*member: C4.method:dynamic Function(dynamic, {dynamic named})**/
+  /*cfe|cfe:builder.member: C4.method:dynamic Function(dynamic, {dynamic named})**/
+  /*analyzer.member: C4.method:Object* Function(Object*, {Object* named})**/
   method(o, {named}) {}
 }
 

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

@@ -54,6 +54,66 @@ class InheritanceManager3 {
   /// self-referencing cycles.
   final Set<ClassElement> _processingClasses = <ClassElement>{};
 
+  /// Combine [candidates] into a single signature in the [targetClass].
+  ///
+  /// If such signature does not exist, return `null`, and if [conflicts] is
+  /// not `null`, add a new [Conflict] to it.
+  ExecutableElement combineSignatures({
+    @required ClassElement targetClass,
+    @required List<ExecutableElement> candidates,
+    @required bool doTopMerge,
+    @required Name name,
+    List<Conflict> conflicts,
+  }) {
+    // If just one candidate, it is always valid.
+    if (candidates.length == 1) {
+      return candidates[0];
+    }
+
+    // Check for a getter/method conflict.
+    var conflict = _checkForGetterMethodConflict(name, candidates);
+    if (conflict != null) {
+      conflicts?.add(conflict);
+      return null;
+    }
+
+    var validOverrides = <ExecutableElement>[];
+    for (var i = 0; i < candidates.length; i++) {
+      var validOverride = candidates[i];
+      var overrideHelper = CorrectOverrideHelper(
+        library: targetClass.library,
+        thisMember: validOverride,
+      );
+      for (var j = 0; j < candidates.length; j++) {
+        var candidate = candidates[j];
+        if (!overrideHelper.isCorrectOverrideOf(superMember: candidate)) {
+          validOverride = null;
+          break;
+        }
+      }
+      if (validOverride != null) {
+        validOverrides.add(validOverride);
+      }
+    }
+
+    if (validOverrides.isEmpty) {
+      conflicts?.add(
+        CandidatesConflict(
+          name: name,
+          candidates: candidates,
+        ),
+      );
+      return null;
+    }
+
+    if (doTopMerge) {
+      var typeSystem = targetClass.library.typeSystem;
+      return _topMerge(typeSystem, targetClass, validOverrides);
+    } else {
+      return validOverrides.first;
+    }
+  }
+
   /// Return the result of [getInherited2] with [type] substitution.
   ExecutableElement getInherited(InterfaceType type, Name name) {
     var rawElement = getInherited2(type.element, name);
@@ -369,9 +429,7 @@ class InheritanceManager3 {
     Map<Name, List<ExecutableElement>> namedCandidates, {
     @required bool doTopMerge,
   }) {
-    TypeSystemImpl typeSystem = targetClass.library.typeSystem;
-
-    List<Conflict> conflicts;
+    var conflicts = <Conflict>[];
 
     for (var name in namedCandidates.keys) {
       if (map.containsKey(name)) {
@@ -380,54 +438,18 @@ class InheritanceManager3 {
 
       var candidates = namedCandidates[name];
 
-      // If just one candidate, it is always valid.
-      if (candidates.length == 1) {
-        map[name] = candidates[0];
-        continue;
-      }
-
-      // Check for a getter/method conflict.
-      var conflict = _checkForGetterMethodConflict(name, candidates);
-      if (conflict != null) {
-        conflicts ??= <Conflict>[];
-        conflicts.add(conflict);
-      }
-
-      var validOverrides = <ExecutableElement>[];
-      for (var i = 0; i < candidates.length; i++) {
-        var validOverride = candidates[i];
-        var overrideHelper = CorrectOverrideHelper(
-          library: targetClass.library,
-          thisMember: validOverride,
-        );
-        for (var j = 0; j < candidates.length; j++) {
-          var candidate = candidates[j];
-          if (!overrideHelper.isCorrectOverrideOf(superMember: candidate)) {
-            validOverride = null;
-            break;
-          }
-        }
-        if (validOverride != null) {
-          validOverrides.add(validOverride);
-        }
-      }
+      var combinedSignature = combineSignatures(
+        targetClass: targetClass,
+        candidates: candidates,
+        doTopMerge: doTopMerge,
+        name: name,
+        conflicts: conflicts,
+      );
 
-      if (validOverrides.isEmpty) {
-        conflicts ??= <Conflict>[];
-        conflicts.add(
-          CandidatesConflict(
-            name: name,
-            candidates: candidates,
-          ),
-        );
+      if (combinedSignature != null) {
+        map[name] = combinedSignature;
         continue;
       }
-
-      if (doTopMerge) {
-        map[name] = _topMerge(typeSystem, targetClass, validOverrides);
-      } else {
-        map[name] = validOverrides.first;
-      }
     }
 
     return conflicts;