Repository: dart-lang/dartdoc (dart)

Issue:
Bad state: typedef ... (TypeAliasElementImpl) cannot have parameter
### steps to reproduce

1. run `dart create -t package dartdoc_issue`
2. add the code below in `lib/dartdoc_issue.dart`
```dart
/// a typedef for a "multi-argument" [Function]
typedef Foo<T, S, U> = T Function(S, U);

/// a typedef for an async "multi-argument" [Function]
typedef Bar<T, S, U> = Foo<Future<T>, S, U>;
```
3. run `dart doc .`

### expected results

no errors

### actual results

<details>
  <summary>logs</summary>
  
  ```console
  ➜  dartdoc_issue dart doc .
Documenting dartdoc_issue...
Initialized dartdoc with 40 libraries
Generating docs for library dartdoc_issue from package:dartdoc_issue/dartdoc_issue.dart...
  warning: dartdoc_issue has no library level documentation comments
    from dartdoc_issue: (file:///Volumes/ROG/dev/dartdoc_issue/lib/dartdoc_issue.dart:1:9)

dartdoc 5.1.0-dev (/Volumes/ROG/dev/dartdoc_issue/dartdev) failed: Bad state: typedef Bar<out T, in S, in U> = Future<T> Function(S, U) (TypeAliasElementImpl) cannot have parameters
#0      ModelElement.parameters.<anonymous closure> (package:dartdoc/src/model/model_element.dart:882:7)
#1      ModelElement.parameters (package:dartdoc/src/model/model_element.dart:912:4)
#2      ModelElement.parameters (package:dartdoc/src/model/model_element.dart)
#3      FunctionTypedef.referenceChildren (package:dartdoc/src/model/typedef.dart:141:31)
#4      CommentReferable.referenceBy (package:dartdoc/src/model/comment_referable.dart:115:38)
#5      _getMatchingLinkElementCommentReferable (package:dartdoc/src/markdown_processor.dart:225:31)
#6      getMatchingLinkElement (package:dartdoc/src/markdown_processor.dart:266:16)
#7      _makeLinkNode (package:dartdoc/src/markdown_processor.dart:233:16)
#8      new MarkdownDocument.withElementLinkResolver.linkResolver (package:dartdoc/src/markdown_processor.dart:340:14)
#9      LinkSyntax._resolveReferenceLink (package:markdown/src/inline_parser.dart:1057:34)
#10     LinkSyntax._tryCreateReferenceLink (package:markdown/src/inline_parser.dart:1085:12)
#11     LinkSyntax.close (package:markdown/src/inline_parser.dart:982:14)
#12     InlineParser._linkOrImage (package:markdown/src/inline_parser.dart:152:29)
#13     InlineParser.parse (package:markdown/src/inline_parser.dart:108:9)
#14     MarkdownDocument._parseInlineContent (package:dartdoc/src/markdown_processor.dart:383:67)
#15     MarkdownDocument._parseInlineContent (package:dartdoc/src/markdown_processor.dart:388:9)
#16     MarkdownDocument.parseMarkdownText (package:dartdoc/src/markdown_processor.dart:373:5)
#17     Documentation._parseDocumentation (package:dartdoc/src/model/documentation.dart:61:21)
#18     Documentation._renderDocumentation (package:dartdoc/src/model/documentation.dart:40:23)
#19     Documentation.asOneLiner (package:dartdoc/src/model/documentation.dart:32:7)
#20     ModelElement.oneLineDoc (package:dartdoc/src/model/model_element.dart:831:50)
#21     __deduplicated_lib_templates_html__typedef_html_partial_type_2 (package:dartdoc/src/generator/templates.aot_renderers_for_html.dart:3579:25)
#22     _deduplicated_lib_templates_html__typedef_html (package:dartdoc/src/generator/templates.aot_renderers_for_html.dart:3505:18)
#23     _renderLibrary_partial_typedef_10 (package:dartdoc/src/generator/templates.aot_renderers_for_html.dart:2676:5)
#24     renderLibrary (package:dartdoc/src/generator/templates.aot_renderers_for_html.dart:1153:20)
#25     HtmlAotTemplates.renderLibrary (package:dartdoc/src/generator/templates.dart:157:30)
#26     DartdocGeneratorBackend.generateLibrary (package:dartdoc/src/generator/dartdoc_generator_backend.dart:192:29)
#27     GeneratorFrontEnd._generateDocs (package:dartdoc/src/generator/generator_frontend.dart:72:27)
#28     GeneratorFrontEnd.generate (package:dartdoc/src/generator/generator_frontend.dart:23:11)
#29     Dartdoc.generateDocsBase (package:dartdoc/src/dartdoc.dart:181:21)
<asynchronous suspension>
#30     Dartdoc.generateDocs (package:dartdoc/src/dartdoc.dart:223:24)
<asynchronous suspension>
#31     Dartdoc.executeGuarded.<anonymous closure> (package:dartdoc/src/dartdoc.dart:261:9)
<asynchronous suspension>
  ```
  
</details>

`Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_arm64"`

Pull request:
Fixes https://github.com/dart-lang/dartdoc/issues/3132

A typedef pointing to a typedef has a null `aliasElement`. We instead must use its `aliasType`.

* Improve documentation
* Improve type promotion in `ModelElement.parameters`.
* Use non-growable list in `ModelElement.parameters`.
* Use const empty list in `ModelElement.parameters`.
* Hard line wraps in relevant tests in model_test.dart.
* New unit test: typedef_test.dart.
* Some improved testing docs.

Patch:
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
index bd9b0cea26..e55db85022 100644
--- a/lib/src/model/model_element.dart
+++ b/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
diff --git a/lib/src/model/package.dart b/lib/src/model/package.dart
index c60aefa089..7465c5686e 100644
--- a/lib/src/model/package.dart
+++ b/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 patch:
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
index 33577b16fc..270a486518 100644
--- a/test/end2end/model_test.dart
+++ b/test/end2end/model_test.dart
@@ -4541,15 +4541,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', () {
diff --git a/test/src/test_descriptor_utils.dart b/test/src/test_descriptor_utils.dart
index 2bcba7544c..757915e63a 100644
--- a/test/src/test_descriptor_utils.dart
+++ b/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].
diff --git a/test/src/utils.dart b/test/src/utils.dart
index 5eaaf69eb0..970458984b 100644
--- a/test/src/utils.dart
+++ b/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([
diff --git a/test/typedef_test.dart b/test/typedef_test.dart
new file mode 100644
index 0000000000..8ab231a055
--- /dev/null
+++ b/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.
+    });
+  });
+}