Fix @observable generic types when using import aliases

There were a number of bugs with the previous implementation of the
`LibraryScopedNameFinder`. This resolves them, as well as ensures that a
single code path is followed whether or not the analyzed source code
contains named imports, reducing the potential for future bugs.

The following bugs have been corrected when using named imports:
* Missing type arguments on classes
* Missing type arguments on function typedefs
* Missing prefixes from imported typedefs

Fixes #367

MR: import_alias_generics

mobx_codegen/lib/src/type_names.dart

@@ -1,7 +1,5 @@
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-// ignore: implementation_imports
-import 'package:analyzer/src/dart/resolver/scope.dart';
 import 'package:mobx_codegen/src/template/comma_list.dart';
 
 /// Determines the names of types within the context of a library, determining
@@ -29,17 +27,20 @@ class LibraryScopedNameFinder {
       return _namesByElement;
     }
 
-    // Reverse each import's export namespace, so that we can easily map
-    // Elements to names.
     _namesByElement = {};
-    final namespaceBuilder = NamespaceBuilder();
-    for (final import in library.imports) {
-      // Build a namespace so that combinators (show/hide) can be computed
-      final namespace =
-          namespaceBuilder.createImportNamespaceForDirective(import);
 
-      // Record each element by name
-      for (final entry in namespace.definedNames.entries) {
+    // Add all of this library's type-defining elements to the name map
+    final libraryElements =
+        library.topLevelElements.whereType<TypeDefiningElement>();
+    for (final element in libraryElements) {
+      _namesByElement[element] = element.name;
+    }
+
+    // Reverse each import's export namespace so we can map elements to their
+    // library-local names. Note that the definedNames include a prefix if there
+    // is one.
+    for (final import in library.imports) {
+      for (final entry in import.namespace.definedNames.entries) {
         _namesByElement[entry.value] = entry.key;
       }
     }
@@ -48,74 +49,68 @@ class LibraryScopedNameFinder {
   }
 
   String findVariableTypeName(VariableElement variable) =>
-      _getTypeName(variable.type);
+      _getDartTypeName(variable.type);
 
   String findGetterTypeName(PropertyAccessorElement getter) {
     assert(getter.isGetter);
     return findReturnTypeName(getter);
   }
 
   String findParameterTypeName(ParameterElement parameter) =>
-      _getTypeName(parameter.type);
+      _getDartTypeName(parameter.type);
 
   String findReturnTypeName(FunctionTypedElement executable) =>
-      _getTypeName(executable.returnType);
+      _getDartTypeName(executable.returnType);
 
   List<String> findReturnTypeArgumentTypeNames(ExecutableElement executable) {
     final returnType = executable.returnType;
     return returnType is ParameterizedType
-        ? returnType.typeArguments.map(_getTypeName).toList()
+        ? returnType.typeArguments.map(_getDartTypeName).toList()
         : [];
   }
 
   String findTypeParameterBoundsTypeName(TypeParameterElement typeParameter) {
     assert(typeParameter.bound != null);
-    return _getTypeName(typeParameter.bound);
+    return _getDartTypeName(typeParameter.bound);
   }
 
   /// Calculates a type name, including its type arguments
   ///
   /// The returned string will include import prefixes on all applicable types.
-  String _getTypeName(DartType type) {
-    final typeElement = type.element;
+  String _getDartTypeName(DartType type) {
+    var typeElement = type.element;
     if (type is FunctionType) {
-      // A type with a typedef should be written as is
-      if (typeElement is GenericFunctionTypeElement &&
-          typeElement.enclosingElement is GenericTypeAliasElement) {
-        return typeElement.enclosingElement.displayName;
+      // If we're dealing with a typedef, we let it undergo the standard name
+      // lookup. Otherwise, we special case the function naming.
+      if (typeElement?.enclosingElement is GenericTypeAliasElement) {
+        typeElement = typeElement.enclosingElement;
+      } else {
+        return _getFunctionTypeName(type);
       }
-
-      return _findFunctionTypeName(type);
     } else if (
-        // If the library has no named imports, we don't need to worry about
-        // prefixes
-        library.prefixes.isEmpty ||
-            // Some types don't have associated elements, like void
-            typeElement == null ||
+        // Some types don't have associated elements, like void
+        typeElement == null ||
             // This is a bare type param, like "T"
-            type is TypeParameterType ||
-            // If the type lives in this library, no prefix necessary
-            typeElement.library == library) {
+            type is TypeParameterType) {
       // TODO(shyndman): This ignored deprecation can be removed when we
       // increase the analyzer dependency's lower bound to 0.39.2, and
       // migrate to using `DartType.getDisplayString`.
       // ignore: deprecated_member_use
       return type.displayName;
     }
 
-    assert(namesByElement.containsKey(typeElement));
-    return namesByElement[typeElement];
+    return _getNamedElementTypeName(typeElement, type);
   }
 
-  String _findFunctionTypeName(FunctionType type) {
-    final returnTypeName = _getTypeName(type.returnType);
+  String _getFunctionTypeName(FunctionType type) {
+    final returnTypeName = _getDartTypeName(type.returnType);
 
     final normalParameterTypeNames =
-        CommaList(type.normalParameterTypes.map(_getTypeName).toList());
+        CommaList(type.normalParameterTypes.map(_getDartTypeName).toList());
     final optionalParameterTypeNames = SurroundedCommaList(
-        '[', ']', type.optionalParameterTypes.map(_getTypeName).toList());
+        '[', ']', type.optionalParameterTypes.map(_getDartTypeName).toList());
     final namedParameterPairs = type.namedParameterTypes.entries
-        .map((entry) => '${_getTypeName(entry.value)} ${entry.key}')
+        .map((entry) => '${_getDartTypeName(entry.value)} ${entry.key}')
         .toList();
     final namedParameterTypeNames =
         SurroundedCommaList('{', '}', namedParameterPairs);
@@ -128,4 +123,19 @@ class LibraryScopedNameFinder {
 
     return '$returnTypeName Function($parameterTypeNames)';
   }
+
+  String _getNamedElementTypeName(Element typeElement, DartType type) {
+    // Determine the name of the type, without type arguments.
+    assert(namesByElement.containsKey(typeElement),
+        '$typeElement not found in name map');
+
+    // If the type is parameterized, we recursively name its type arguments
+    if (type is ParameterizedType && type.typeArguments.isNotEmpty) {
+      final typeArgNames = SurroundedCommaList(
+          '<', '>', type.typeArguments.map(_getDartTypeName).toList());
+      return '${namesByElement[typeElement]}$typeArgNames';
+    }
+
+    return namesByElement[typeElement];
+  }
 }

mobx_codegen/pubspec.yaml

@@ -5,7 +5,7 @@ version: 0.4.0+1
 homepage: https://github.com/mobxjs/mobx.dart
 
 environment:
-  sdk: '>=2.2.0 <3.0.0'
+  sdk: '>=2.2.2 <3.0.0'
 
 dependencies:
   analyzer: '>=0.38.5 <0.40.0'
@@ -16,15 +16,10 @@ dependencies:
   source_gen: ^0.9.4
 
 dev_dependencies:
-#  flutter:
-#    sdk: flutter
   build_runner: ^1.7.2
   build_test: ^0.10.9
   logging: ^0.11.3
   mockito: ^4.0.0
   test: ^1.9.4
   test_coverage:
     git: https://github.com/shyndman/test-coverage
-
-#flutter:
-#  uses-material-design: true

mobx_codegen/test/data/valid_generic_store_input.dart

@@ -9,4 +9,7 @@ class Item<A extends num> = _Item<A> with _$Item<A>;
 abstract class _Item<T extends num> with Store {
   @observable
   T value;
+
+  @observable
+  List<T> values;
 }

mobx_codegen/test/data/valid_generic_store_output.dart

@@ -15,4 +15,21 @@ mixin _$Item<T extends num> on _Item<T>, Store {
       _$valueAtom.reportChanged();
     }, _$valueAtom, name: '${_$valueAtom.name}_set');
   }
+
+  final _$valuesAtom = Atom(name: '_Item.values');
+
+  @override
+  List<T> get values {
+    _$valuesAtom.context.enforceReadPolicy(_$valuesAtom);
+    _$valuesAtom.reportObserved();
+    return super.values;
+  }
+
+  @override
+  set values(List<T> value) {
+    _$valuesAtom.context.conditionallyRunInAction(() {
+      super.values = value;
+      _$valuesAtom.reportChanged();
+    }, _$valuesAtom, name: '${_$valuesAtom.name}_set');
+  }
 }

mobx_codegen/test/data/valid_import_prefixed_input.dart

@@ -7,23 +7,42 @@ import 'package:mobx/mobx.dart';
 
 part 'generator_sample.g.dart';
 
-class User = UserBase with _$User;
+class User<T extends io.Process> = UserBase<T> with _$User<T>;
 
-abstract class UserBase with Store {
+abstract class UserBase<T extends io.Process> with Store {
   UserBase();
 
+  @observable
+  List<String> names;
+
+  @observable
+  List<io.File> files;
+
+  @observable
+  List<T> processes;
+
   @observable
   io.File biography;
 
+  // This should output the type's constraint, prefixed
   @observable
-  User friend;
+  User friendWithImplicitTypeArgument;
 
   @observable
-  void Function(io.File, {io.File another}) callback;
+  User<T> friendWithExplicitTypeArgument;
+
+  @observable
+  void Function(io.File, {T another}) callback;
 
   @observable
   io.File Function(String, [int, io.File]) callback2;
 
+  @observable
+  ValueCallback<io.Process> localTypedefCallback;
+
+  @observable
+  io.BadCertificateCallback prefixedTypedefCallback;
+
   @computed
   io.File get biographyNotes => io.File('${biography.path}.notes');
 
@@ -41,3 +60,5 @@ abstract class UserBase with Store {
     yield directory;
   }
 }
+
+typedef ValueCallback<T> = void Function(T);