Allow for anonymous typed functions (#1506)

* Basic functionality for typed functions without typedefs

* Refactor and add more tests

* dartfmt

* Move constructor logic to a more logical place

* Make AppVeyor run pub get for the test package as well to regenerate .packages

* Doc update

* appveyor experiment

* appveyor experiment 2

* appveyor experiment #3

* update pubspec.locks

* Fix private detection to include dart:_ prefix

* travis experiment

* travis experiment 2

* travis experiment 3

* Reset to 05ddca0

* Fix private detection to include dart:_ prefix

* Review comments

* typo

Author: jcollins-g

appveyor.yml

@@ -9,6 +9,11 @@ install:
   - set PATH=%PATH%;C:\tools\dart-sdk\bin
   - set PATH=%PATH%;%APPDATA%\Pub\Cache\bin
   - pub get
+  - cmd: cd testing
+  - cmd: cd test_package
+  - cmd: pub get
+  - cmd: cd ..
+  - cmd: cd ..
 
 build: off
 

lib/src/element_type.dart

@@ -21,42 +21,38 @@ class ElementType {
 
   bool get isFunctionType => (_type is FunctionType);
 
-  bool get isParameterizedType {
-    if (_type is FunctionType) {
-      return typeArguments.isNotEmpty;
-    } else if (_type is ParameterizedType) {
-      return (_type as ParameterizedType).typeArguments.isNotEmpty;
-    }
-    return false;
-  }
+  bool get isParameterizedType => (_type is ParameterizedType);
 
   bool get isParameterType => (_type is TypeParameterType);
 
   String get linkedName {
-    if (_linkedName != null) return _linkedName;
-
-    StringBuffer buf = new StringBuffer();
-
-    if (isParameterType) {
-      buf.write(name);
-    } else {
-      buf.write(element.linkedName);
-    }
+    if (_linkedName == null) {
+      StringBuffer buf = new StringBuffer();
 
-    // not TypeParameterType or Void or Union type
-    if (isParameterizedType) {
-      if (typeArguments.every((t) => t.linkedName == 'dynamic')) {
-        _linkedName = buf.toString();
-        return _linkedName;
+      if (isParameterType) {
+        buf.write(name);
+      } else {
+        buf.write(element.linkedName);
       }
-      if (typeArguments.isNotEmpty) {
-        buf.write('<');
-        buf.writeAll(typeArguments.map((t) => t.linkedName), ', ');
-        buf.write('>');
+
+      // not TypeParameterType or Void or Union type
+      if (isParameterizedType) {
+        if (!typeArguments.every((t) => t.linkedName == 'dynamic') &&
+            typeArguments.isNotEmpty) {
+          buf.write('<');
+          buf.writeAll(typeArguments.map((t) => t.linkedName), ', ');
+          buf.write('>');
+        }
+        // Hide parameters if there's a an explicit typedef behind this
+        // element, but if there is no typedef, be explicit.
+        if (element is ModelFunctionAnonymous) {
+          buf.write('(');
+          buf.write(element.linkedParams());
+          buf.write(')');
+        }
       }
+      _linkedName = buf.toString();
     }
-    _linkedName = buf.toString();
-
     return _linkedName;
   }
 

lib/src/model.dart

@@ -769,7 +769,7 @@ class Class extends ModelElement implements EnclosedElement {
 
   @override
   String get nameWithGenerics {
-    if (!modelType.isParameterizedType) return name;
+    if (!modelType.isParameterizedType || _typeParameters.isEmpty) return name;
     return '$name<${_typeParameters.map((t) => t.name).join(', ')}>';
   }
 
@@ -2216,6 +2216,14 @@ abstract class ModelElement extends Nameable
         }
         if (e is FunctionElement) {
           newModelElement = new ModelFunction(e, library);
+        } else if (e is GenericFunctionTypeElement) {
+          if (e is FunctionTypeAliasElement) {
+            assert(e.name != '');
+            newModelElement = new ModelFunctionTypedef(e, library);
+          } else {
+            assert(e.name == '');
+            newModelElement = new ModelFunctionAnonymous(e, library);
+          }
         }
         if (e is FunctionTypeAliasElement) {
           newModelElement = new Typedef(e, library);
@@ -2285,10 +2293,7 @@ abstract class ModelElement extends Nameable
         }
       }
     }
-    // TODO(jcollins-g): Consider subclass for ModelFunctionTyped.
-    if (e is GenericFunctionTypeElement) {
-      newModelElement = new ModelFunctionTyped(e, library);
-    }
+
 
     if (newModelElement == null) throw "Unknown type ${e.runtimeType}";
     if (enclosingClass != null) assert(newModelElement is Inheritable);
@@ -3203,6 +3208,7 @@ abstract class ModelElement extends Nameable
   }
 }
 
+/// A [ModelElement] for a [FunctionElement] that isn't part of a type definition.
 class ModelFunction extends ModelFunctionTyped {
   ModelFunction(FunctionElement element, Library library)
       : super(element, library);
@@ -3212,10 +3218,52 @@ class ModelFunction extends ModelFunctionTyped {
     return _func.isStatic;
   }
 
+  @override
+  String get name {
+    if (element.enclosingElement is ParameterElement && super.name.isEmpty)
+      return element.enclosingElement.name;
+    return super.name;
+  }
+
   @override
   FunctionElement get _func => (element as FunctionElement);
 }
 
+/// A [ModelElement] for a [GenericModelFunctionElement] that is an
+/// explicit typedef.
+///
+/// Distinct from ModelFunctionTypedef in that it doesn't
+/// have a name, but we document it as "Function" to match how these are
+/// written in declarations.
+class ModelFunctionAnonymous extends ModelFunctionTyped {
+  ModelFunctionAnonymous(FunctionTypedElement element, Library library)
+      : super(element, library) {}
+
+  @override
+  String get name => 'Function';
+
+  @override
+  bool get isPublic => false;
+}
+
+/// A [ModelElement] for a [GenericModelFunctionElement] that is part of an
+/// explicit typedef.
+class ModelFunctionTypedef extends ModelFunctionTyped {
+  ModelFunctionTypedef(FunctionTypedElement element, Library library)
+      : super(element, library) {}
+
+  @override
+  String get name {
+    Element e = element;
+    while (e != null) {
+      if (e is FunctionTypeAliasElement) return e.name;
+      e = e.enclosingElement;
+    }
+    assert(false);
+    return super.name;
+  }
+}
+
 class ModelFunctionTyped extends ModelElement
     with SourceCodeMixin
     implements EnclosedElement {
@@ -3238,13 +3286,6 @@ class ModelFunctionTyped extends ModelElement
 
   String get fileName => "$name.html";
 
-  @override
-  String get name {
-    if (element.enclosingElement is ParameterElement && super.name.isEmpty)
-      return element.enclosingElement.name;
-    return super.name;
-  }
-
   @override
   String get href {
     if (canonicalLibrary == null) return null;
@@ -4590,7 +4631,7 @@ class Typedef extends ModelElement
 
   @override
   String get nameWithGenerics {
-    if (!modelType.isParameterizedType) return name;
+    if (!modelType.isParameterizedType || _typeParameters.isEmpty) return name;
     return '$name<${_typeParameters.map((t) => t.name).join(', ')}>';
   }
 

lib/src/model_utils.dart

@@ -54,12 +54,14 @@ bool isInExportedLibraries(
 
 final RegExp slashes = new RegExp('[\/]');
 bool hasPrivateName(Element e) {
-  if (e.name.startsWith('_') ||
-      (e is LibraryElement &&
-          (e.identifier == 'dart:_internal' ||
-              e.identifier == 'dart:nativewrappers'))) {
+  if (e.name.startsWith('_')) {
     return true;
   }
+  if (e is LibraryElement &&
+      (e.identifier.startsWith('dart:_') ||
+          ['dart:nativewrappers'].contains(e.identifier))) {
+      return true;
+  }
   if (e is LibraryElement) {
     List<String> locationParts = e.location.components[0].split(slashes);
     // TODO(jcollins-g): Implement real cross package detection

pubspec.lock

@@ -121,6 +121,12 @@ packages:
       url: "https://pub.dartlang.org"
     source: hosted
     version: "1.0.0"
+  js:
+    description:
+      name: js
+      url: "https://pub.dartlang.org"
+    source: hosted
+    version: "0.6.1"
   kernel:
     description:
       name: kernel
@@ -169,6 +175,12 @@ packages:
       url: "https://pub.dartlang.org"
     source: hosted
     version: "1.1.0"
+  node_preamble:
+    description:
+      name: node_preamble
+      url: "https://pub.dartlang.org"
+    source: hosted
+    version: "1.4.0"
   package_config:
     description:
       name: package_config
@@ -294,7 +306,7 @@ packages:
       name: test
       url: "https://pub.dartlang.org"
     source: hosted
-    version: "0.12.20+13"
+    version: "0.12.24+6"
   tuple:
     description:
       name: tuple
@@ -350,4 +362,4 @@ packages:
     source: hosted
     version: "2.1.12"
 sdks:
-  dart: ">=1.23.0-dev.11.5 <2.0.0-dev.infinity"
+  dart: ">=1.23.0 <=2.0.0-dev.2.0"

pubspec.yaml

@@ -28,6 +28,6 @@ dev_dependencies:
   http: ^0.11.0
   meta: ^1.0.0
   pub_semver: ^1.0.0
-  test: ^0.12.0
+  test: '^0.12.20+24'
 executables:
   dartdoc: null

test/model_test.dart

@@ -686,7 +686,7 @@ void main() {
     });
 
     test('correctly finds all the classes', () {
-      expect(classes, hasLength(21));
+      expect(classes, hasLength(22));
     });
 
     test('abstract', () {
@@ -1027,9 +1027,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
   });
 
   group('Method', () {
-    Class classB, klass, HasGenerics, Cat, CatString;
+    Class classB, klass, HasGenerics, Cat, CatString, TypedFunctionsWithoutTypedefs;
     Method m1, isGreaterThan, m4, m5, m6, m7, convertToMap, abstractMethod;
     Method inheritedClear, testGeneric, testGenericMethod;
+    Method getAFunctionReturningVoid;
 
     setUp(() {
       klass = exLibrary.classes.singleWhere((c) => c.name == 'Klass');
@@ -1061,6 +1062,8 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
           .singleWhere((m) => m.name == 'testGenericMethod');
       convertToMap = HasGenerics.instanceMethods
           .singleWhere((m) => m.name == 'convertToMap');
+      TypedFunctionsWithoutTypedefs = exLibrary.classes.singleWhere((c) => c.name == 'TypedFunctionsWithoutTypedefs');
+      getAFunctionReturningVoid = TypedFunctionsWithoutTypedefs.instanceMethods.singleWhere((m) => m.name == 'getAFunctionReturningVoid');
     });
 
     tearDown(() {
@@ -1070,6 +1073,15 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
       }
     });
 
+    test('verify parameters to types are displayed', () {
+      var matcher = new RegExp('Function\\(<span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T.</span></span> <span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T.</span></span>\\)');
+      expect(matcher.hasMatch(getAFunctionReturningVoid.linkedReturnType), isTrue);
+    });
+
+    test('verify parameter types are correctly displayed', () {
+      expect(getAFunctionReturningVoid.linkedReturnType, equals('Function(<span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T1</span></span> <span class="parameter" id="getAFunctionReturningVoid-param-"><span class="type-annotation">T2</span></span>)'));
+    }, skip: 'blocked on https://github.com/dart-lang/sdk/issues/30146');
+
     test('has a fully qualified name', () {
       expect(m1.fullyQualifiedName, 'ex.B.m1');
     });
@@ -1757,13 +1769,26 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
   group('Typedef', () {
     Typedef t;
     Typedef generic;
+    Typedef aComplexTypedef;
 
     setUp(() {
       t = exLibrary.typedefs.firstWhere((t) => t.name == 'processMessage');
       generic =
           fakeLibrary.typedefs.firstWhere((t) => t.name == 'NewGenericTypedef');
+      aComplexTypedef = exLibrary.typedefs.firstWhere((t) => t.name == 'aComplexTypedef');
     });
 
+    test('anonymous nested functions inside typedefs are handled', () {
+      expect(aComplexTypedef, isNotNull);
+      expect(aComplexTypedef.linkedReturnType, startsWith('Function'));
+      expect(aComplexTypedef.nameWithGenerics, equals('aComplexTypedef&lt;A1, A2, A3&gt;'));
+    });
+
+    test('anonymous nested functions inside typedefs are handled correctly', () {
+      expect(aComplexTypedef.linkedReturnType, equals('Function(<span class="parameter" id="-param-"><span class="type-annotation">A1</span></span> <span class="parameter" id="-param-"><span class="type-annotation">A2</span></span> <span class="parameter" id="-param-"><span class="type-annotation">A3</span></span>)'));
+      expect(aComplexTypedef.linkedParamsLines, equals('<span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">A3</span></span> <span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">String</span></span>'));
+    }, skip: 'blocked on https://github.com/dart-lang/sdk/issues/30146');
+
     test('has a fully qualified name', () {
       expect(t.fullyQualifiedName, 'ex.processMessage');
       expect(generic.fullyQualifiedName, 'fake.NewGenericTypedef');

testing/test_package/lib/example.dart

@@ -424,3 +424,16 @@ class _RetainedEnum {
   @override
   String toString() => name;
 }
+
+/// Someone might do this some day.
+typedef aComplexTypedef<A1, A2, A3> = void Function(A1, A2, A3) Function(A3, String);
+
+/// This class has a complicated type situation.
+abstract class TypedFunctionsWithoutTypedefs {
+  /// Returns a function that returns a void with some generic types sprinkled in.
+  void Function(T1, T2) getAFunctionReturningVoid<T1, T2>(
+      void callback(T1 argument1, T2 argument2));
+
+  /// Returns a complex typedef that includes some anonymous typed functions.
+  aComplexTypedef getAComplexTypedef<A4, A5, A6>();
+}

testing/test_package_docs/ex/Animal-class.html

@@ -58,6 +58,7 @@ <h5>library ex</h5>
       <li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
       <li><a href="ex/ShapeType-class.html">ShapeType</a></li>
       <li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
+      <li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
       <li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
       <li><a href="ex/WithGenericSub-class.html">WithGenericSub</a></li>
 
@@ -87,6 +88,7 @@ <h5>library ex</h5>
       <li><a href="ex/Animal-class.html">Animal</a></li>
 
       <li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
+      <li><a href="ex/aComplexTypedef.html">aComplexTypedef</a></li>
       <li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
       <li><a href="ex/processMessage.html">processMessage</a></li>
 

testing/test_package_docs/ex/Apple-class.html

@@ -58,6 +58,7 @@ <h5>library ex</h5>
       <li><a href="ex/PublicClassImplementsPrivateInterface-class.html">PublicClassImplementsPrivateInterface</a></li>
       <li><a href="ex/ShapeType-class.html">ShapeType</a></li>
       <li><a href="ex/SpecializedDuration-class.html">SpecializedDuration</a></li>
+      <li><a href="ex/TypedFunctionsWithoutTypedefs-class.html">TypedFunctionsWithoutTypedefs</a></li>
       <li><a href="ex/WithGeneric-class.html">WithGeneric</a></li>
       <li><a href="ex/WithGenericSub-class.html">WithGenericSub</a></li>
 
@@ -87,6 +88,7 @@ <h5>library ex</h5>
       <li><a href="ex/Animal-class.html">Animal</a></li>
 
       <li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
+      <li><a href="ex/aComplexTypedef.html">aComplexTypedef</a></li>
       <li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
       <li><a href="ex/processMessage.html">processMessage</a></li>