Switch ErrorVerifier and OverrideVerifier to InheritanceManager2.

R=brianwilkerson@google.com

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

pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart

@@ -220,9 +220,10 @@ class LibraryAnalyzer {
         typeSystem: _context.typeSystem));
 
     unit.accept(new OverrideVerifier(
-        errorReporter,
-        new InheritanceManager(_libraryElement,
-            includeAbstractFromSuperclasses: true)));
+      _inheritance,
+      _libraryElement,
+      errorReporter,
+    ));
 
     new ToDoFinder(errorReporter).findIn(unit);
 

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

@@ -43,11 +43,18 @@ class InheritanceManager2 {
 
   InheritanceManager2(this._typeSystem);
 
+  /// Return the member with the given [name] that the class inherits from the
+  /// mixins, superclasses, or interfaces; or `null` if no member is inherited.
+  FunctionType getInherited(InterfaceType type, Name name) {
+    var interface = getInterface(type);
+    return interface._inherited[name];
+  }
+
   /// Return the interface of the given [type].  It might include private
   /// members, not necessary accessible in all libraries.
   Interface getInterface(InterfaceType type) {
     if (type == null) {
-      return const Interface._(const {}, const [{}], const []);
+      return const Interface._(const {}, const {}, const [{}], const []);
     }
 
     var result = _interfaces[type];
@@ -55,7 +62,12 @@ class InheritanceManager2 {
       return result;
     }
 
-    _interfaces[type] = const Interface._(const {}, const [{}], const []);
+    _interfaces[type] = const Interface._(
+      const {},
+      const {},
+      const [{}],
+      const [],
+    );
     Map<Name, FunctionType> map = {};
     List<Map<Name, FunctionType>> superImplemented = [];
     List<Conflict> conflicts = null;
@@ -109,8 +121,16 @@ class InheritanceManager2 {
     // signature becomes the signature of the class's interface.
     conflicts = _findMostSpecificFromNamedCandidates(map, namedCandidates);
 
+    // Get one candidate for each name.
+    Map<Name, FunctionType> inherited = {};
+    for (var name in namedCandidates.keys) {
+      var candidates = namedCandidates[name];
+      inherited[name] = candidates.last;
+    }
+
     var interface = new Interface._(
       map,
+      inherited,
       superImplemented,
       conflicts ?? const [],
     );
@@ -366,6 +386,10 @@ class Interface {
   /// The map of names to their signature in the interface.
   final Map<Name, FunctionType> map;
 
+  /// The map of names to their signature from the mixins, superclasses,
+  /// or interfaces.
+  final Map<Name, FunctionType> _inherited;
+
   /// Each item of this list maps names to their concrete implementations.
   /// The first item of the list is the nominal superclass, next the nominal
   /// superclass plus the first mixin, etc. So, for the class like
@@ -377,7 +401,12 @@ class Interface {
   /// members of the class.
   final List<Conflict> conflicts;
 
-  const Interface._(this.map, this._superImplemented, this.conflicts);
+  const Interface._(
+    this.map,
+    this._inherited,
+    this._superImplemented,
+    this.conflicts,
+  );
 }
 
 /// A public name, or a private name qualified by a library URI.

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

@@ -2441,16 +2441,20 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
     if (_enclosingClass == null) {
       return;
     }
+    InterfaceType enclosingType = _enclosingClass.type;
+    Uri libraryUri = _currentLibrary.source.uri;
 
     // method declared in the enclosing class vs. inherited getter/setter
     for (MethodElement method in _enclosingClass.methods) {
       String name = method.name;
 
       // find inherited property accessor
-      ExecutableElement inherited =
-          _inheritanceManager.lookupInheritance(_enclosingClass, name);
-      inherited ??=
-          _inheritanceManager.lookupInheritance(_enclosingClass, '$name=');
+      ExecutableElement inherited = _inheritanceManager2
+          .getInherited(enclosingType, new Name(libraryUri, name))
+          ?.element;
+      inherited ??= _inheritanceManager2
+          .getInherited(enclosingType, new Name(libraryUri, '$name='))
+          ?.element;
 
       if (method.isStatic && inherited != null) {
         _errorReporter.reportErrorForElement(
@@ -2474,10 +2478,12 @@ class ErrorVerifier extends RecursiveAstVisitor<Object> {
       String name = accessor.displayName;
 
       // find inherited method or property accessor
-      ExecutableElement inherited =
-          _inheritanceManager.lookupInheritance(_enclosingClass, name);
-      inherited ??=
-          _inheritanceManager.lookupInheritance(_enclosingClass, '$name=');
+      ExecutableElement inherited = _inheritanceManager2
+          .getInherited(enclosingType, new Name(libraryUri, name))
+          ?.element;
+      inherited ??= _inheritanceManager2
+          .getInherited(enclosingType, new Name(libraryUri, '$name='))
+          ?.element;
 
       if (accessor.isStatic && inherited != null) {
         _errorReporter.reportErrorForElement(

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

@@ -3898,27 +3898,27 @@ class InstanceFieldResolverVisitor extends ResolverVisitor {
 /// compilation unit to verify that if they have an override annotation it is
 /// being used correctly.
 class OverrideVerifier extends RecursiveAstVisitor {
+  /// The inheritance manager used to find overridden methods.
+  final InheritanceManager2 _inheritance;
+
+  /// The URI of the library being verified.
+  final Uri _libraryUri;
+
   /// The error reporter used to report errors.
   final ErrorReporter _errorReporter;
 
-  /// The inheritance manager used to find overridden methods.
-  final InheritanceManager _manager;
+  /// The current class or mixin.
+  InterfaceType _currentType;
 
-  /// The [ClassElement] of the current [ClassDeclaration].
-  ClassElement _currentClass;
-
-  /// Initialize a newly created verifier to look for inappropriate uses of the
-  /// override annotation.
-  ///
-  /// @param errorReporter the error reporter used to report errors
-  /// @param manager the inheritance manager used to find overridden methods
-  OverrideVerifier(this._errorReporter, this._manager);
+  OverrideVerifier(
+      this._inheritance, LibraryElement library, this._errorReporter)
+      : _libraryUri = library.source.uri;
 
   @override
   visitClassDeclaration(ClassDeclaration node) {
-    _currentClass = node.declaredElement;
+    _currentType = node.declaredElement.type;
     super.visitClassDeclaration(node);
-    _currentClass = null;
+    _currentType = null;
   }
 
   @override
@@ -3967,14 +3967,15 @@ class OverrideVerifier extends RecursiveAstVisitor {
 
   @override
   visitMixinDeclaration(MixinDeclaration node) {
-    _currentClass = node.declaredElement;
+    _currentType = node.declaredElement.type;
     super.visitMixinDeclaration(node);
-    _currentClass = null;
+    _currentType = null;
   }
 
   /// Return `true` if the [member] overrides a member from a superinterface.
   bool _isOverride(ExecutableElement member) {
-    return _manager.lookupInheritance(_currentClass, member.name) != null;
+    var name = new Name(_libraryUri, member.name);
+    return _inheritance.getInherited(_currentType, name) != null;
   }
 }
 

pkg/analyzer/lib/src/task/dart.dart

@@ -2855,15 +2855,17 @@ class GenerateHintsTask extends SourceBasedAnalysisTask {
       unit.accept(new Dart2JSVerifier(errorReporter));
     }
     // Dart best practices.
-    InheritanceManager inheritanceManager = new InheritanceManager(
-        libraryElement,
-        includeAbstractFromSuperclasses: true);
+    var inheritanceManager2 = new InheritanceManager2(context.typeSystem);
     TypeProvider typeProvider = getRequiredInput(TYPE_PROVIDER_INPUT);
 
     unit.accept(new BestPracticesVerifier(
         errorReporter, typeProvider, libraryElement,
         typeSystem: typeSystem));
-    unit.accept(new OverrideVerifier(errorReporter, inheritanceManager));
+    unit.accept(new OverrideVerifier(
+      inheritanceManager2,
+      libraryElement,
+      errorReporter,
+    ));
     // Find to-do comments.
     new ToDoFinder(errorReporter).findIn(unit);
     //

pkg/analyzer/test/generated/non_hint_code_test.dart

@@ -723,29 +723,6 @@ class B extends A {
     verify([source]);
   }
 
-  @FailingTest(
-      reason: 'We should not use types here. '
-          'There is a member in a superinterface, so we override something.')
-  test_overrideOnNonOverridingMethod_dontUseInterface() async {
-    Source source = addSource(r'''
-abstract class A {
-  void foo(int _);
-}
-
-abstract class B {
-  void foo(double _);
-}
-
-abstract class X implements A, B {
-  @override
-  void foo(Object _) {}
-}
-''');
-    await computeAnalysisResult(source);
-    assertNoErrors(source);
-    verify([source]);
-  }
-
   test_overrideOnNonOverridingMethod_inInterface() async {
     Source source = addSource(r'''
 class A {
@@ -760,6 +737,26 @@ class B implements A {
     verify([source]);
   }
 
+  test_overrideOnNonOverridingMethod_inInterfaces() async {
+    Source source = addSource(r'''
+abstract class I {
+  void foo(int _);
+}
+
+abstract class J {
+  void foo(String _);
+}
+
+class C implements I, J {
+  @override
+  void foo(Object _) {}
+}
+''');
+    await computeAnalysisResult(source);
+    assertNoErrors(source);
+    verify([source]);
+  }
+
   test_overrideOnNonOverridingMethod_inSuperclass() async {
     Source source = addSource(r'''
 class A {

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

@@ -2,6 +2,7 @@
 // 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/element/element.dart';
 import 'package:analyzer/src/dart/element/inheritance_manager2.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -26,6 +27,94 @@ class InheritanceManager2Test extends DriverResolutionTest {
     );
   }
 
+  test_getInherited_closestSuper() async {
+    addTestFile('''
+class A {
+  void foo() {}
+}
+
+class B extends A {
+  void foo() {}
+}
+
+class X extends B {
+  void foo() {}
+}
+''');
+    await resolveTestFile();
+
+    expect(
+      _getInherited('X', 'foo'),
+      same(findElement.method('foo', of: 'B')),
+    );
+  }
+
+  test_getInherited_interfaces() async {
+    addTestFile('''
+abstract class I {
+  void foo();
+}
+
+abstrac class J {
+  void foo();
+}
+
+class X implements I, J {
+  void foo() {}
+}
+''');
+    await resolveTestFile();
+
+    expect(
+      _getInherited('X', 'foo'),
+      same(findElement.method('foo', of: 'J')),
+    );
+  }
+
+  test_getInherited_mixin() async {
+    addTestFile('''
+class A {
+  void foo() {}
+}
+
+mixin M {
+  void foo() {}
+}
+
+class X extends A with M {
+  void foo() {}
+}
+''');
+    await resolveTestFile();
+
+    expect(
+      _getInherited('X', 'foo'),
+      same(findElement.method('foo', of: 'M')),
+    );
+  }
+
+  test_getInherited_preferImplemented() async {
+    addTestFile('''
+class A {
+  void foo() {}
+}
+
+class I {
+  void foo() {}
+}
+
+class X extends A implements I {
+  void foo() {}
+}
+''');
+    await resolveTestFile();
+
+    expect(
+      _getInherited('X', 'foo'),
+      same(findElement.method('foo', of: 'A')),
+    );
+  }
+
   void test_getMember_() async {
     addTestFile('''
 abstract class I1 {
@@ -123,4 +212,9 @@ class X extends A implements I {
     );
     expect(member.element, findElement.method('foo', of: 'X'));
   }
+
+  ExecutableElement _getInherited(String className, String name) {
+    var type = findElement.class_(className).type;
+    return manager.getInherited(type, new Name(null, name)).element;
+  }
 }