Enable strong mode in Dartdoc (#1611)

* First version of the strong mode change, before refactoring

* Basic refactor eliminating multiple inheritance manager builds

* dartfmt and test package rebuild

* pubspec change

* regen test package docs post merge

Author: jcollins-g

lib/src/model.dart

@@ -698,115 +698,50 @@ class Class extends ModelElement
   }
 
   List<Method> get inheritedMethods {
-    if (_inheritedMethods != null) return _inheritedMethods;
-
-    Map<String, ExecutableElement> cmap =
-        library.inheritanceManager.getMembersInheritedFromClasses(element);
-    Map<String, ExecutableElement> imap =
-        library.inheritanceManager.getMembersInheritedFromInterfaces(element);
-
-    // remove methods that exist on this class
-    _methods.forEach((method) {
-      cmap.remove(method.name);
-      imap.remove(method.name);
-    });
-
-    _inheritedMethods = [];
-    List<ExecutableElement> values = [];
-    Set<String> uniqueNames = new Set();
-
-    instanceProperties.forEach((f) {
-      if (f.setter != null) uniqueNames.add(f.setter.element.name);
-      if (f.getter != null) uniqueNames.add(f.getter.element.name);
-    });
-
-    for (String key in cmap.keys) {
-      // XXX: if we care about showing a hierarchy with our inherited methods,
-      // then don't do this
-      if (uniqueNames.contains(key)) continue;
-
-      uniqueNames.add(key);
-      values.add(cmap[key]);
-    }
-
-    for (String key in imap.keys) {
-      // XXX: if we care about showing a hierarchy with our inherited methods,
-      // then don't do this
-      if (uniqueNames.contains(key)) continue;
-
-      uniqueNames.add(key);
-      values.add(imap[key]);
-    }
-
-    for (ExecutableElement value in values) {
-      if (value != null &&
-          value is MethodElement &&
-          !value.isOperator &&
-          value.enclosingElement != null) {
-        Method m;
-        m = new ModelElement.from(value, library, enclosingClass: this);
+    if (_inheritedMethods == null) {
+      _inheritedMethods = new List<Method>();
+      Set<String> methodNames = _methods.map((m) => m.element.name).toSet();
+
+      Set<ExecutableElement> inheritedMethodElements =
+          _inheritedElements.where((e) {
+        return (e is MethodElement &&
+            !e.isOperator &&
+            e is! PropertyAccessorElement &&
+            !methodNames.contains(e.name));
+      }).toSet();
+
+      for (ExecutableElement e in inheritedMethodElements) {
+        Method m = new ModelElement.from(e, library, enclosingClass: this);
         _inheritedMethods.add(m);
         _genPageMethods.add(m);
       }
+      _inheritedMethods.sort(byName);
     }
-
-    _inheritedMethods.sort(byName);
-
     return _inheritedMethods;
   }
 
   Iterable get publicInheritedMethods => filterNonPublic(inheritedMethods);
 
   bool get hasPublicInheritedMethods => publicInheritedMethods.isNotEmpty;
 
-  // TODO(jcollins-g): this is very copy-paste from inheritedMethods now that the
-  // constructor is always [ModelElement.from].  Fix this.
   List<Operator> get inheritedOperators {
-    if (_inheritedOperators != null) return _inheritedOperators;
-    Map<String, ExecutableElement> cmap =
-        library.inheritanceManager.getMembersInheritedFromClasses(element);
-    Map<String, ExecutableElement> imap =
-        library.inheritanceManager.getMembersInheritedFromInterfaces(element);
-    operators.forEach((operator) {
-      cmap.remove(operator.element.name);
-      imap.remove(operator.element.name);
-    });
-    _inheritedOperators = [];
-    Map<String, ExecutableElement> values = {};
-
-    bool _isInheritedOperator(ExecutableElement value) {
-      if (value != null &&
-          value is MethodElement &&
-          !value.isPrivate &&
-          value.isOperator &&
-          value.enclosingElement != null) {
-        return true;
+    if (_inheritedOperators == null) {
+      _inheritedOperators = [];
+      Set<String> operatorNames = _operators.map((o) => o.element.name).toSet();
+
+      Set<ExecutableElement> inheritedOperatorElements =
+          _inheritedElements.where((e) {
+        return (e is MethodElement &&
+            e.isOperator &&
+            !operatorNames.contains(e.name));
+      }).toSet();
+      for (ExecutableElement e in inheritedOperatorElements) {
+        Operator o = new ModelElement.from(e, library, enclosingClass: this);
+        _inheritedOperators.add(o);
+        _genPageOperators.add(o);
       }
-      return false;
-    }
-
-    for (String key in imap.keys) {
-      ExecutableElement value = imap[key];
-      if (_isInheritedOperator(value)) {
-        values.putIfAbsent(value.name, () => value);
-      }
-    }
-
-    for (String key in cmap.keys) {
-      ExecutableElement value = cmap[key];
-      if (_isInheritedOperator(value)) {
-        values.putIfAbsent(value.name, () => value);
-      }
-    }
-
-    for (ExecutableElement value in values.values) {
-      Operator o = new ModelElement.from(value, library, enclosingClass: this);
-      _inheritedOperators.add(o);
-      _genPageOperators.add(o);
+      _inheritedOperators.sort(byName);
     }
-
-    _inheritedOperators.sort(byName);
-
     return _inheritedOperators;
   }
 
@@ -994,22 +929,26 @@ class Class extends ModelElement
 
   ElementType get supertype => _supertype;
 
+  List<ExecutableElement> __inheritedElements;
+  List<ExecutableElement> get _inheritedElements {
+    if (__inheritedElements == null) {
+      __inheritedElements = [];
+      Map<String, ExecutableElement> cmap =
+          library.inheritanceManager.getMembersInheritedFromClasses(element);
+      Map<String, ExecutableElement> imap =
+          library.inheritanceManager.getMembersInheritedFromInterfaces(element);
+      __inheritedElements.addAll(cmap.values);
+      __inheritedElements
+          .addAll(imap.values.where((e) => !cmap.containsKey(e.name)));
+    }
+    return __inheritedElements;
+  }
+
   List<Field> get allFields {
     if (_fields != null) return _fields;
     _fields = [];
-    Map<String, ExecutableElement> cmap =
-        library.inheritanceManager.getMembersInheritedFromClasses(element);
-    Map<String, ExecutableElement> imap =
-        library.inheritanceManager.getMembersInheritedFromInterfaces(element);
-
-    Set<PropertyAccessorElement> inheritedAccessors = new Set();
-    inheritedAccessors
-        .addAll(cmap.values.where((e) => e is PropertyAccessorElement));
-
-    // Interfaces are subordinate to members inherited from classes, so don't
-    // add this to our accessor set if we already have something inherited from classes.
-    inheritedAccessors.addAll(imap.values.where(
-        (e) => e is PropertyAccessorElement && !cmap.containsKey(e.name)));
+    Set<PropertyAccessorElement> inheritedAccessors = new Set()
+      ..addAll(_inheritedElements.where((e) => e is PropertyAccessorElement));
 
     // This structure keeps track of inherited accessors, allowing lookup
     // by field name (stripping the '=' from setters).
@@ -5091,6 +5030,7 @@ class PackageBuilder {
       PerformanceLog log = new PerformanceLog(null);
       AnalysisDriverScheduler scheduler = new AnalysisDriverScheduler(log);
       AnalysisOptionsImpl options = new AnalysisOptionsImpl();
+      options.strongMode = true;
       options.enableSuperMixins = true;
 
       // TODO(jcollins-g): Make use of currently not existing API for managing

test/model_test.dart

@@ -6,6 +6,7 @@ library dartdoc.model_test;
 
 import 'dart:io';
 
+import 'package:analyzer/dart/element/element.dart';
 import 'package:dartdoc/dartdoc.dart';
 import 'package:dartdoc/src/model.dart';
 import 'package:dartdoc/src/warnings.dart';
@@ -732,6 +733,15 @@ void main() {
     });
   });
 
+  group('Class edge cases', () {
+    test('ExecutableElements from private classes and from public interfaces (#1561)', () {
+      Class MIEEMixinWithOverride = fakeLibrary.publicClasses.firstWhere((c) => c.name == 'MIEEMixinWithOverride');
+      Operator problematicOperator = MIEEMixinWithOverride.inheritedOperators.firstWhere((o) => o.name == 'operator []=');
+      expect(problematicOperator.element.enclosingElement.name, equals('_MIEEPrivateOverride'));
+      expect(problematicOperator.canonicalModelElement.enclosingElement.name, equals('MIEEMixinWithOverride'));
+    });
+  });
+
   group('Class', () {
     List<Class> classes;
     Class Apple, B, Cat, Cool, Dog, F, Dep, SpecialList;

testing/test_package/lib/fake.dart

@@ -720,4 +720,25 @@ class ReferringClass {
   bool notAMethodFromPrivateClass() {
     return false;
   }
+}
+
+/// Test an edge case for cases where inherited ExecutableElements can come
+/// both from private classes and public interfaces.  The test makes sure the
+/// class still takes precedence (#1561).
+abstract class MIEEMixinWithOverride<K, V> = MIEEBase<K, V> with _MIEEPrivateOverride<K, V>;
+
+abstract class _MIEEPrivateOverride<K, V> implements MIEEThing<K, V> {
+  void operator[]=(K key, V value) {
+    throw new UnsupportedError("Never use this");
+  }
+}
+
+abstract class MIEEBase<K, V> extends MIEEMixin<K, V> {}
+
+abstract class MIEEMixin<K, V> implements MIEEThing<K, V> {
+  operator []=(K key, V value);
+}
+
+abstract class MIEEThing<K, V> {
+  void operator[]=(K key, V value);
 }

testing/test_package_docs/ex/Dog-class.html

@@ -375,7 +375,7 @@ <h2>Operators</h2>
       <dl class="callables">
         <dt id="operator ==" class="callable">
           <span class="name"><a href="ex/Dog/operator_equals.html">operator ==</a></span><span class="signature">(<wbr><span class="parameter" id="==-param-other"><span class="parameter-name">other</span></span>)
-            <span class="returntype parameter">&#8594; dynamic</span>
+            <span class="returntype parameter">&#8594; bool</span>
           </span>
         </dt>
         <dd>

testing/test_package_docs/ex/Dog/operator_equals.html

@@ -97,7 +97,7 @@ <h1>operator == method</h1>
           <li>@override</li>
         </ol>
       </div>
-      <span class="returntype">dynamic</span>
+      <span class="returntype">bool</span>
       <span class="name ">operator ==</span>
 (<wbr><span class="parameter" id="==-param-other"><span class="parameter-name">other</span></span>)
     </section>

testing/test_package_docs/ex/F-class.html

@@ -371,7 +371,7 @@ <h2>Operators</h2>
       <dl class="callables">
         <dt id="operator ==" class="callable inherited">
           <span class="name"><a href="ex/Dog/operator_equals.html">operator ==</a></span><span class="signature">(<wbr><span class="parameter" id="==-param-other"><span class="parameter-name">other</span></span>)
-            <span class="returntype parameter">&#8594; dynamic</span>
+            <span class="returntype parameter">&#8594; bool</span>
           </span>
         </dt>
         <dd class="inherited">

testing/test_package_docs/ex/Klass-class.html

@@ -200,7 +200,7 @@ <h2>Methods</h2>
 </dd>
         <dt id="toString" class="callable">
           <span class="name"><a href="ex/Klass/toString.html">toString</a></span><span class="signature">(<wbr>)
-            <span class="returntype parameter">&#8594; dynamic</span>
+            <span class="returntype parameter">&#8594; String</span>
           </span>
         </dt>
         <dd>

testing/test_package_docs/ex/Klass/toString.html

@@ -74,7 +74,7 @@ <h1>toString method</h1>
           <li>@override</li>
         </ol>
       </div>
-      <span class="returntype">dynamic</span>
+      <span class="returntype">String</span>
       <span class="name ">toString</span>
 (<wbr>)
     </section>

testing/test_package_docs/ex/ex-library.html

@@ -299,7 +299,7 @@ <h2>Constants</h2>
         </dd>
         <dt id="incorrectDocReference" class="constant">
           <span class="name "><a href="ex/incorrectDocReference-constant.html">incorrectDocReference</a></span>
-          <span class="signature">&#8594; const dynamic</span>
+          <span class="signature">&#8594; const String</span>
         </dt>
         <dd>
           This is the same name as a top-level const from the fake lib.
@@ -310,7 +310,7 @@ <h2>Constants</h2>
         </dd>
         <dt id="incorrectDocReferenceFromEx" class="constant">
           <span class="name "><a href="ex/incorrectDocReferenceFromEx-constant.html">incorrectDocReferenceFromEx</a></span>
-          <span class="signature">&#8594; const dynamic</span>
+          <span class="signature">&#8594; const String</span>
         </dt>
         <dd>
           This should <code>not work</code>.

testing/test_package_docs/fake/AClassUsingASuperMixin-class.html

@@ -63,6 +63,10 @@ <h5>fake library</h5>
       <li><a href="fake/InheritingClassTwo-class.html">InheritingClassTwo</a></li>
       <li><a href="fake/Interface-class.html">Interface</a></li>
       <li><a href="fake/LongFirstLine-class.html">LongFirstLine</a></li>
+      <li><a href="fake/MIEEBase-class.html">MIEEBase</a></li>
+      <li><a href="fake/MIEEMixin-class.html">MIEEMixin</a></li>
+      <li><a href="fake/MIEEMixinWithOverride-class.html">MIEEMixinWithOverride</a></li>
+      <li><a href="fake/MIEEThing-class.html">MIEEThing</a></li>
       <li><a href="fake/MixMeIn-class.html">MixMeIn</a></li>
       <li><a href="fake/NotAMixin-class.html">NotAMixin</a></li>
       <li><a href="fake/OperatorReferenceClass-class.html">OperatorReferenceClass</a></li>