Fix generic typedef pointing to typedef (#3193)

Author: srawlins

lib/src/model/model_element.dart

@@ -181,7 +181,7 @@ abstract class ModelElement extends Canonicalization
       return cachedModelElement;
     }
 
-    ModelElement? newModelElement;
+    ModelElement newModelElement;
     if (e is FieldElement) {
       if (enclosingContainer == null) {
         if (e.isEnumConstant) {
@@ -639,7 +639,7 @@ abstract class ModelElement extends Canonicalization
     return self.enclosingElement == self.canonicalEnclosingContainer;
   }
 
-  /// Returns the docs, stripped of their leading comments syntax.
+  /// The documentaion, stripped of its comment syntax, like `///` characters.
   @override
   String get documentation {
     return injectMacros(
@@ -830,38 +830,45 @@ abstract class ModelElement extends Canonicalization
   @override
   p.Context get pathContext => packageGraph.resourceProvider.pathContext;
 
+  // TODO(srawlins): This really smells like it should just be implemented in
+  // the subclasses.
   late final List<Parameter> parameters = () {
+    final element = this.element;
     if (!isCallable) {
       throw StateError(
           '$element (${element.runtimeType}) cannot have parameters');
     }
 
+    final List<ParameterElement> params;
     if (element is TypeAliasElement) {
-      return ModelElement._fromElement(
-              (element as TypeAliasElement).aliasedElement!, packageGraph)
-          .parameters;
-    }
-    List<ParameterElement>? params;
-    if (element is ExecutableElement) {
+      final aliasedType = element.aliasedType;
+      if (aliasedType is FunctionType) {
+        params = aliasedType.parameters;
+      } else {
+        return const <Parameter>[];
+      }
+    } else if (element is ExecutableElement) {
       if (_originalMember != null) {
         assert(_originalMember is ExecutableMember);
         params = (_originalMember as ExecutableMember).parameters;
       } else {
-        params = (element as ExecutableElement).parameters;
+        params = element.parameters;
       }
-    }
-    if (params == null && element is FunctionTypedElement) {
+    } else if (element is FunctionTypedElement) {
       if (_originalMember != null) {
         params = (_originalMember as FunctionTypedElement).parameters;
       } else {
-        params = (element as FunctionTypedElement).parameters;
+        params = element.parameters;
       }
+    } else {
+      return const <Parameter>[];
     }
 
-    return <Parameter>[
-      ...?params?.map(
-          (p) => ModelElement._from(p, library, packageGraph) as Parameter)
-    ];
+    return List.of(
+      params.map(
+          (p) => ModelElement._from(p, library, packageGraph) as Parameter),
+      growable: false,
+    );
   }();
 
   @override

lib/src/model/package.dart

@@ -117,6 +117,7 @@ class Package extends LibraryContainer
   @override
   late final String documentationAsHtml = Documentation.forElement(this).asHtml;
 
+  /// The documentation from the README contents.
   @override
   late final String? documentation = () {
     final docFile = packageMeta.getReadmeContents();

test/end2end/model_test.dart

@@ -4521,15 +4521,19 @@ String? topLevelFunction(int param1, bool param2, Cool coolBeans,
     test('anonymous nested functions inside typedefs are handled correctly',
         () {
       expect(
-          aComplexTypedef.modelType.returnType.linkedName,
-          equals(
-              'void Function<span class="signature">(<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>)</span>'));
+        aComplexTypedef.modelType.returnType.linkedName,
+        'void Function<span class="signature">'
+        '(<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>)</span>',
+      );
       expect(
-          aComplexTypedef.linkedParamsLines,
-          equals(
-              '<ol class="parameter-list"><li><span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">A3</span>, </span></li>\n'
-              '<li><span class="parameter" id="aComplexTypedef-param-"><span class="type-annotation">String</span></span></li>\n'
-              '</ol>'));
+        aComplexTypedef.linkedParamsLines,
+        '<ol class="parameter-list">'
+        '<li><span class="parameter" id="param-"><span class="type-annotation">A3</span>, </span></li>\n'
+        '<li><span class="parameter" id="param-"><span class="type-annotation">String</span></span></li>\n'
+        '</ol>',
+      );
     });
 
     test('has a fully qualified name', () {

test/src/test_descriptor_utils.dart

@@ -14,7 +14,7 @@ const _defaultPubspec = '''
 name: test_package
 version: 0.0.1
 environment:
-  sdk: '>=2.12.0 <3.0.0'
+  sdk: '>=2.15.0 <3.0.0'
 ''';
 
 /// Creates a pub package in a directory named [name].

test/src/utils.dart

@@ -89,6 +89,8 @@ void runPubGet(String dirPath) {
   }
 }
 
+/// Creates a package at [dirPath] and returns the [PackageGraph] built with
+/// [PubPackageBuilder.buildPackageGraph].
 Future<PackageGraph> bootBasicPackage(
   String dirPath,
   PackageMetaProvider packageMetaProvider,
@@ -97,11 +99,11 @@ Future<PackageGraph> bootBasicPackage(
   List<String> additionalArguments = const [],
   bool skipUnreachableSdkLibraries = true,
 }) async {
-  var resourceProvider = packageMetaProvider.resourceProvider;
+  final resourceProvider = packageMetaProvider.resourceProvider;
   if (resourceProvider == PhysicalResourceProvider.INSTANCE) {
     runPubGet(dirPath);
   }
-  var dir = resourceProvider.getFolder(resourceProvider.pathContext
+  final dir = resourceProvider.getFolder(resourceProvider.pathContext
       .absolute(resourceProvider.pathContext.normalize(dirPath)));
   return PubPackageBuilder(
     await contextFromArgv([

test/typedef_test.dart

@@ -0,0 +1,107 @@
+// Copyright (c) 2022, the Dart project authors.  Please see the AUTHORS file
+// 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/type.dart';
+import 'package:analyzer/file_system/memory_file_system.dart';
+import 'package:dartdoc/src/model/model.dart';
+import 'package:test/test.dart';
+
+import 'src/test_descriptor_utils.dart' as d;
+import 'src/utils.dart';
+
+void main() {
+  group('typedefs', () {
+    late Library library;
+
+    // It is expensive (~10s) to compute a package graph, even skipping
+    // unreachable Dart SDK libraries, so we set up this package once.
+    setUpAll(() async {
+      const libraryName = 'typedefs';
+      final packageMetaProvider = testPackageMetaProvider;
+
+      final packagePath = await d.createPackage(
+        libraryName,
+        libFiles: [
+          d.file('lib.dart', '''
+library $libraryName;
+
+/// Line _one_.
+///
+/// Line _two_.
+typedef Cb1 = void Function();
+
+typedef Cb2<T> = T Function(T);
+
+typedef Cb3<T> = Cb2<List<T>>;
+'''),
+        ],
+        resourceProvider:
+            packageMetaProvider.resourceProvider as MemoryResourceProvider,
+      );
+      final packageConfigProvider =
+          getTestPackageConfigProvider(packageMetaProvider.defaultSdkDir.path);
+      packageConfigProvider.addPackageToConfigFor(
+          packagePath, libraryName, Uri.file('$packagePath/'));
+
+      final packageGraph = await bootBasicPackage(
+        packagePath,
+        packageMetaProvider,
+        packageConfigProvider,
+      );
+      library = packageGraph.libraries.named(libraryName);
+    });
+
+    test('basic typedef', () async {
+      final cb1Typedef = library.typedefs.named('Cb1');
+
+      expect(cb1Typedef.nameWithGenerics, 'Cb1');
+      expect(cb1Typedef.genericParameters, '');
+      expect(cb1Typedef.aliasedType is FunctionType, isTrue);
+      expect(cb1Typedef.documentationComment, '''
+/// Line _one_.
+///
+/// Line _two_.''');
+      expect(cb1Typedef.documentation, '''
+Line _one_.
+
+Line _two_.''');
+      expect(cb1Typedef.oneLineDoc, 'Line <em>one</em>.');
+      expect(cb1Typedef.documentationAsHtml, '''
+<p>Line <em>one</em>.</p>
+<p>Line <em>two</em>.</p>''');
+    });
+
+    test('generic typedef', () async {
+      final cb2Typedef = library.typedefs.named('Cb2');
+
+      expect(
+        cb2Typedef.nameWithGenerics,
+        'Cb2&lt;<wbr><span class="type-parameter">T</span>&gt;',
+      );
+      expect(
+        cb2Typedef.genericParameters,
+        '&lt;<wbr><span class="type-parameter">T</span>&gt;',
+      );
+      expect(cb2Typedef.aliasedType is FunctionType, isTrue);
+    });
+
+    test('generic typedef referring to a generic typedef', () async {
+      final cb3Typedef = library.typedefs.named('Cb3');
+
+      expect(
+        cb3Typedef.nameWithGenerics,
+        'Cb3&lt;<wbr><span class="type-parameter">T</span>&gt;',
+      );
+      expect(
+        cb3Typedef.genericParameters,
+        '&lt;<wbr><span class="type-parameter">T</span>&gt;',
+      );
+      expect(cb3Typedef.aliasedType is FunctionType, isTrue);
+
+      expect(cb3Typedef.parameters, hasLength(1));
+
+      // TODO(srawlins): Dramatically improve typedef testing.
+    });
+  });
+}