isAssignable, isSuper fix (#183)

* TypeChecker: Add test for generic types

* Clarify the behavior of isSuper and isAssignable

and fixe related issue due to bug in pkg/analyzer

dart-lang/sdk#29767

* oops

Author: kevmoo

lib/generators/json_serializable_generator.dart

@@ -310,8 +310,9 @@ DartType _getImplementationType(DartType type, TypeChecker checker) {
   if (checker.isExactlyType(type)) return type;
 
   if (type is InterfaceType) {
-    var tests =
-        type.interfaces.map((type) => _getImplementationType(type, checker));
+    var tests = [type.interfaces, type.mixins]
+        .expand((e) => e)
+        .map((type) => _getImplementationType(type, checker));
     var match = _firstNotNull(tests);
 
     if (match != null) return match;

lib/src/type_checker.dart

@@ -52,13 +52,13 @@ abstract class TypeChecker {
       .map((a) => a.computeConstantValue())
       .where((a) => isExactlyType(a.type));
 
-  /// Returns `true` if representing the exact same class as or a superclass of
-  /// [element]
+  /// Returns `true` if the type of [element] can be assigned to the type
+  /// represented by `this`.
   bool isAssignableFrom(Element element) =>
-      isExactly(element) || isSuperOf(element);
+      isExactly(element) || _getAllSupertypes(element).any(isExactlyType);
 
-  /// Returns `true` if representing the exact same type as or a supertype of
-  /// [staticType].
+  /// Returns `true` if [staticType] can be assigned to the type represented
+  /// by `this`.
   bool isAssignableFromType(DartType staticType) =>
       isAssignableFrom(staticType.element);
 
@@ -69,13 +69,55 @@ abstract class TypeChecker {
   bool isExactlyType(DartType staticType) => isExactly(staticType.element);
 
   /// Returns `true` if representing a super class of [element].
-  bool isSuperOf(Element element) =>
-      element is ClassElement && element.allSupertypes.any(isExactlyType);
+  ///
+  /// This check only takes into account the *extends* hierarchy. If you wish
+  /// to check mixins and interfaces, use [isAssignableFrom].
+  bool isSuperOf(Element element) {
+    if (element is ClassElement) {
+      var theSuper = element.supertype;
+
+      do {
+        if (isExactlyType(theSuper)) {
+          return true;
+        }
+
+        theSuper = theSuper.superclass;
+      } while (theSuper != null);
+    }
+
+    return false;
+  }
 
   /// Returns `true` if representing a super type of [staticType].
+  ///
+  /// This only takes into account the *extends* hierarchy. If you wish
+  /// to check mixins and interfaces, use [isAssignableFromType].
   bool isSuperTypeOf(DartType staticType) => isSuperOf(staticType.element);
 }
 
+//TODO(kevmoo) Remove when bug with `ClassElement.allSupertypes` is fixed
+// https://github.com/dart-lang/sdk/issues/29767
+Iterable<InterfaceType> _getAllSupertypes(Element element) sync* {
+  if (element is ClassElement) {
+    var processed = new Set<InterfaceType>();
+    var toExplore = new List<InterfaceType>.from(element.allSupertypes);
+
+    while (toExplore.isNotEmpty) {
+      var item = toExplore.removeLast();
+
+      if (processed.add(item)) {
+        yield item;
+
+        // Now drill into nested superTypes - but make sure not to duplicate
+        // any of them.
+        toExplore.addAll(item.element.allSupertypes.where((e) {
+          return !toExplore.contains(e) && !processed.contains(e);
+        }));
+      }
+    }
+  }
+}
+
 Uri _normalizeUrl(Uri url) {
   switch (url.scheme) {
     case 'dart':

test/type_checker_test.dart

@@ -12,8 +12,11 @@ import 'package:test/test.dart';
 
 void main() {
   // Resolved top-level types from dart:core and dart:collection.
+  InterfaceType staticUri;
   DartType staticMap;
   DartType staticHashMap;
+  DartType staticUnmodifiableListView;
+  TypeChecker staticIterableChecker;
   TypeChecker staticMapChecker;
   TypeChecker staticHashMapChecker;
 
@@ -29,12 +32,17 @@ void main() {
     ''');
 
     final core = resolver.getLibraryByName('dart.core');
+    var staticIterable = core.getType('Iterable').type;
+    staticIterableChecker = new TypeChecker.fromStatic(staticIterable);
+    staticUri = core.getType('Uri').type;
     staticMap = core.getType('Map').type;
     staticMapChecker = new TypeChecker.fromStatic(staticMap);
 
     final collection = resolver.getLibraryByName('dart.collection');
     staticHashMap = collection.getType('HashMap').type;
     staticHashMapChecker = new TypeChecker.fromStatic(staticHashMap);
+    staticUnmodifiableListView =
+        collection.getType('UnmodifiableListView').type;
 
     final sourceGen =
         new LibraryReader(resolver.getLibraryByName('source_gen'));
@@ -48,11 +56,20 @@ void main() {
 
   // Run a common set of type comparison checks with various implementations.
   void commonTests({
+    @required TypeChecker checkIterable(),
     @required TypeChecker checkMap(),
     @required TypeChecker checkHashMap(),
     @required TypeChecker checkGenerator(),
     @required TypeChecker checkGeneratorForAnnotation(),
   }) {
+    group('(Iterable)', () {
+      test('should be assignable from dart:collection#UnmodifiableListView',
+          () {
+        expect(checkIterable().isAssignableFromType(staticUnmodifiableListView),
+            true);
+      });
+    });
+
     group('(Map)', () {
       test('should equal dart:core#Map', () {
         expect(checkMap().isExactlyType(staticMap), isTrue,
@@ -69,12 +86,22 @@ void main() {
       });
 
       test('should be a super type of dart:collection#HashMap', () {
-        expect(checkMap().isSuperTypeOf(staticHashMap), isTrue);
+        expect(checkMap().isSuperTypeOf(staticHashMap), isFalse);
       });
 
       test('should be assignable from dart:collection#HashMap', () {
         expect(checkMap().isAssignableFromType(staticHashMap), isTrue);
       });
+
+      // Ensure we're consistent WRT generic types
+      test('should be assignable from Map<String, String>', () {
+        // Using Uri.queryParamaters to get a Map<String, String>
+        var stringStringMapType =
+            staticUri.getGetter('queryParameters').returnType;
+
+        expect(checkMap().isAssignableFromType(stringStringMapType), isTrue);
+        expect(checkMap().isExactlyType(stringStringMapType), isTrue);
+      });
     });
 
     group('(HashMap)', () {
@@ -122,6 +149,7 @@ void main() {
 
   group('TypeChecker.forRuntime', () {
     commonTests(
+        checkIterable: () => const TypeChecker.fromRuntime(Iterable),
         checkMap: () => const TypeChecker.fromRuntime(Map),
         checkHashMap: () => const TypeChecker.fromRuntime(HashMap),
         checkGenerator: () => const TypeChecker.fromRuntime(Generator),
@@ -131,6 +159,7 @@ void main() {
 
   group('TypeChecker.forStatic', () {
     commonTests(
+        checkIterable: () => staticIterableChecker,
         checkMap: () => staticMapChecker,
         checkHashMap: () => staticHashMapChecker,
         checkGenerator: () => staticGeneratorChecker,
@@ -139,6 +168,7 @@ void main() {
 
   group('TypeChecker.fromUrl', () {
     commonTests(
+        checkIterable: () => const TypeChecker.fromUrl('dart:core#Iterable'),
         checkMap: () => const TypeChecker.fromUrl('dart:core#Map'),
         checkHashMap: () =>
             const TypeChecker.fromUrl('dart:collection#HashMap'),