Skip to content

Commit 5e6cd06

Browse files
astashovkeertip
astashov
authored and
keertip
committed
Fix bug when parameterized typedef props have wrong return type [#1263] (#1300)
@HansMuller noticed, that for properties, that have parameterized typedef (i.e. with generic parameter) as a return type - they don't show that parameter in the docs in their return types. It appears we don't show them, because in that case `FunctionType#typeFormals` return an empty array. If we use `FunctionType#typeArguments`, then everything works fine. After further investigation I figured that if we run dartdoc on the whole Flutter codebase, `FunctionType#typeFormals` always returns empty array. So, not sure why it was introduced in the first place. `git blame` has shown it was introduced here 9d96d8d by @keertip, as part of upgrade to the latest analyzer (0.27.0). Though after the changes in this PR all unit tests are still green, and also I couldn't find anything broken in the Flutter docs after quick glance, I'm still a bit nervous about this change, because: * I don't quite understand what's the difference between `typeFormals` and `typeArguments`, and in what cases we need to use `typeFormals`. * I don't understand why that `if` condition with `typeFormals` was initially introduced. So, I'd like to get advice from @keertip in that case, I think.
1 parent 91f4643 commit 5e6cd06

File tree

79 files changed

+481
-9
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+481
-9
lines changed

‎lib/src/element_type.dart

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class ElementType {
2323

2424
bool get isParameterizedType {
2525
if (_type is FunctionType) {
26-
return (_type as FunctionType).typeFormals.isNotEmpty;
26+
return typeArguments.isNotEmpty;
2727
} else if (_type is ParameterizedType) {
2828
return (_type as ParameterizedType).typeArguments.isNotEmpty;
2929
}
@@ -75,11 +75,18 @@ class ElementType {
7575
}
7676

7777
List<ElementType> get typeArguments {
78-
if (_type is FunctionType) {
79-
return (_type as FunctionType)
80-
.typeFormals
81-
.map((f) => _getElementTypeFrom(f.type))
82-
.toList();
78+
var type = _type;
79+
if (type is FunctionType) {
80+
Iterable<DartType> typeArguments;
81+
if (type.element is FunctionTypeAliasElement && type.typeFormals.isEmpty) {
82+
// TODO(jmesserly): simplify check above; we should have a way
83+
// to find instantiated typedefs without consulting the element.
84+
// Also, it will not work if we support typedefs declared inside classes.
85+
typeArguments = type.typeArguments;
86+
} else {
87+
typeArguments = type.typeFormals.map((f) => f.type);
88+
}
89+
return typeArguments.map(_getElementTypeFrom).toList();
8390
} else {
8491
return (_type as ParameterizedType)
8592
.typeArguments

‎test/model_test.dart

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,11 +608,11 @@ void main() {
608608
});
609609

610610
test('get instance fields', () {
611-
expect(Apple.instanceProperties, hasLength(2));
611+
expect(Apple.instanceProperties, hasLength(3));
612612
});
613613

614614
test('get inherited properties, including properties of Object', () {
615-
expect(B.inheritedProperties, hasLength(4));
615+
expect(B.inheritedProperties, hasLength(5));
616616
});
617617

618618
test('get methods', () {
@@ -1269,6 +1269,16 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
12691269
final doc = classB.allInstanceProperties.firstWhere((p) => p.name == "s").getter.documentation;
12701270
expect(doc, equals("The getter for `s`"));
12711271
});
1272+
1273+
test("has correct linked return type if the return type is a parameterized typedef", () {
1274+
Class apple = exLibrary.classes.firstWhere((c) => c.name == 'Apple');
1275+
final fieldWithTypedef = apple
1276+
.allInstanceProperties
1277+
.firstWhere((m) => m.name == "fieldWithTypedef");
1278+
expect(
1279+
fieldWithTypedef.linkedReturnType,
1280+
equals('<a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a>&lt;bool&gt;'));
1281+
});
12721282
});
12731283

12741284
group('Top-level Variable', () {
@@ -1480,6 +1490,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
14801490
test('linked return type', () {
14811491
expect(t.linkedReturnType, equals('String'));
14821492
});
1493+
1494+
test("name with generics", () {
1495+
expect(t.nameWithGenerics, equals('processMessage&lt;T&gt;'));
1496+
});
14831497
});
14841498

14851499
group('Parameter', () {

‎testing/test_package/lib/example.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ T genericFunction<T>(T arg) {
5454

5555
typedef String processMessage<T>(String msg);
5656

57+
typedef String ParameterizedTypedef<T>(T msg, int foo);
58+
5759
enum Animal {
5860
/// Single line docs.
5961
CAT,
@@ -128,6 +130,11 @@ class Apple {
128130
void paramFromExportLib(Helper helper) {}
129131

130132
void printMsg(String msg, [bool linebreak]) {}
133+
134+
/**
135+
* fieldWithTypedef docs here
136+
*/
137+
final ParameterizedTypedef<bool> fieldWithTypedef;
131138
}
132139

133140
class WithGeneric<T> {

‎testing/test_package_docs/ex/Animal-class.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
8686
<li><a href="ex/PRETTY_COLORS-constant.html">PRETTY_COLORS</a></li>
8787

8888
<li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
89+
<li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
8990
<li><a href="ex/processMessage.html">processMessage</a></li>
9091

9192
<li class="section-title"><a href="ex/ex-library.html#properties">Properties</a></li>

‎testing/test_package_docs/ex/Apple-class.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ <h5><a href="ex/ex-library.html">ex</a></h5>
9090
<li><a href="ex/PRETTY_COLORS-constant.html">PRETTY_COLORS</a></li>
9191

9292
<li class="section-title"><a href="ex/ex-library.html#typedefs">Typedefs</a></li>
93+
<li><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a></li>
9394
<li><a href="ex/processMessage.html">processMessage</a></li>
9495

9596
<li class="section-title"><a href="ex/ex-library.html#properties">Properties</a></li>
@@ -216,6 +217,16 @@ <h2>Constructors</h2>
216217
<h2>Properties</h2>
217218

218219
<dl class="properties">
220+
<dt id="fieldWithTypedef" class="property">
221+
<span class="name"><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></span>
222+
<span class="signature">&#8594; <a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a>&lt;bool&gt;</span>
223+
</dt>
224+
<dd>
225+
<p>fieldWithTypedef docs here</p>
226+
<div class="features">
227+
read-only
228+
</div>
229+
</dd>
219230
<dt id="hashCode" class="property inherited">
220231
<span class="name"><a href="ex/Apple/hashCode.html">hashCode</a></span>
221232
<span class="signature">&#8594; int</span>
@@ -365,6 +376,7 @@ <h5>Apple</h5>
365376
<li class="section-title">
366377
<a href="ex/Apple-class.html#instance-properties">Properties</a>
367378
</li>
379+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
368380
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
369381
<li><a href="ex/Apple/m.html">m</a></li>
370382
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>

‎testing/test_package_docs/ex/Apple/Apple.fromString.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ <h5><a href="ex/Apple-class.html">Apple</a></h5>
8888
<li class="section-title">
8989
<a href="ex/Apple-class.html#instance-properties">Properties</a>
9090
</li>
91+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
9192
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
9293
<li><a href="ex/Apple/m.html">m</a></li>
9394
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>

‎testing/test_package_docs/ex/Apple/Apple.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ <h5><a href="ex/Apple-class.html">Apple</a></h5>
8888
<li class="section-title">
8989
<a href="ex/Apple-class.html#instance-properties">Properties</a>
9090
</li>
91+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
9192
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
9293
<li><a href="ex/Apple/m.html">m</a></li>
9394
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>

‎testing/test_package_docs/ex/Apple/fieldWithTypedef.html

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<meta http-equiv="X-UA-Compatible" content="IE=edge">
6+
<meta name="viewport" content="width=device-width, initial-scale=1">
7+
<meta name="description" content="API docs for the fieldWithTypedef property from the Apple class, for the Dart programming language.">
8+
<title>fieldWithTypedef property - Apple class - ex library - Dart API</title>
9+
<!-- required because all the links are pseudo-absolute -->
10+
<base href="../..">
11+
12+
<link rel='stylesheet' href='https://fonts.googleapis.com/css?family=Source+Code+Pro|Roboto:500,400italic,300,400' type='text/css'>
13+
<link rel="stylesheet" href="static-assets/prettify.css">
14+
<link rel="stylesheet" href="static-assets/css/bootstrap.min.css">
15+
<link rel="stylesheet" href="static-assets/styles.css">
16+
<link rel="icon" href="static-assets/favicon.png">
17+
18+
<!-- Do not remove placeholder -->
19+
<!-- Header Placeholder -->
20+
</head>
21+
22+
<body>
23+
24+
<div id="overlay-under-drawer"></div>
25+
26+
<header class="container-fluid" id="title">
27+
<nav class="navbar navbar-fixed-top">
28+
<div class="container">
29+
<div class="row">
30+
<div class="col-sm-12 contents">
31+
<button id="sidenav-left-toggle" type="button">&nbsp;</button>
32+
<ol class="breadcrumbs gt-separated hidden-xs">
33+
<li><a href="index.html">test_package</a></li>
34+
<li><a href="ex/ex-library.html">ex</a></li>
35+
<li><a href="ex/Apple-class.html">Apple</a></li>
36+
<li class="self-crumb">fieldWithTypedef</li>
37+
</ol>
38+
<div class="self-name">fieldWithTypedef</div>
39+
<form class="search navbar-right" role="search">
40+
<input type="text" id="search-box" autocomplete="off" disabled class="form-control typeahead" placeholder="Loading search...">
41+
</form>
42+
</div> <!-- /col -->
43+
</div> <!-- /row -->
44+
</div> <!-- /container -->
45+
</nav>
46+
47+
<div class="container masthead">
48+
<div class="row">
49+
<div class="col-sm-12 contents">
50+
<ol class="breadcrumbs gt-separated visible-xs">
51+
<li><a href="index.html">test_package</a></li>
52+
<li><a href="ex/ex-library.html">ex</a></li>
53+
<li><a href="ex/Apple-class.html">Apple</a></li>
54+
<li class="self-crumb">fieldWithTypedef</li>
55+
</ol>
56+
<div class="title-description">
57+
<h1 class="title">
58+
<span class="kind">property</span> fieldWithTypedef
59+
</h1>
60+
</div>
61+
</div> <!-- /col -->
62+
</div> <!-- /row -->
63+
</div> <!-- /container -->
64+
65+
</header>
66+
67+
<div class="container body">
68+
<div class="row">
69+
70+
<div class="col-xs-6 col-sm-3 col-md-2 sidebar sidebar-offcanvas-left">
71+
<h5><a href="index.html">test_package</a></h5>
72+
<h5><a href="ex/ex-library.html">ex</a></h5>
73+
<h5><a href="ex/Apple-class.html">Apple</a></h5>
74+
75+
<ol>
76+
<li class="section-title"><a href="ex/Apple-class.html#constants">Constants</a></li>
77+
<li><a href="ex/Apple/n-constant.html">n</a></li>
78+
79+
<li class="section-title"><a href="ex/Apple-class.html#static-properties">Static properties</a></li>
80+
<li><a href="ex/Apple/string.html">string</a></li>
81+
82+
83+
<li class="section-title"><a href="ex/Apple-class.html#constructors">Constructors</a></li>
84+
<li><a href="ex/Apple/Apple.html">Apple</a></li>
85+
<li><a href="ex/Apple/Apple.fromString.html">fromString</a></li>
86+
87+
<li class="section-title">
88+
<a href="ex/Apple-class.html#instance-properties">Properties</a>
89+
</li>
90+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
91+
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
92+
<li><a href="ex/Apple/m.html">m</a></li>
93+
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>
94+
<li><a href="ex/Apple/s.html">s</a></li>
95+
96+
<li class="section-title"><a href="ex/Apple-class.html#operators">Operators</a></li>
97+
<li><a href="ex/Apple/operator_multiply.html">operator *</a></li>
98+
<li class="inherited"><a href="ex/Apple/operator_equals.html">operator ==</a></li>
99+
100+
<li class="section-title"><a href="ex/Apple-class.html#instance-methods">Methods</a></li>
101+
<li><a href="ex/Apple/isGreaterThan.html">isGreaterThan</a></li>
102+
<li><a href="ex/Apple/m1.html">m1</a></li>
103+
<li><a href="ex/Apple/methodWithTypedefParam.html">methodWithTypedefParam</a></li>
104+
<li class="inherited"><a href="ex/Apple/noSuchMethod.html">noSuchMethod</a></li>
105+
<li><a href="ex/Apple/paramFromExportLib.html">paramFromExportLib</a></li>
106+
<li><a href="ex/Apple/printMsg.html">printMsg</a></li>
107+
<li class="inherited"><a href="ex/Apple/toString.html">toString</a></li>
108+
</ol>
109+
110+
</div><!--/.sidebar-offcanvas-->
111+
112+
<div class="col-xs-12 col-sm-9 col-md-8 main-content">
113+
114+
<section class="multi-line-signature">
115+
<span class="returntype"><a href="ex/ParameterizedTypedef.html">ParameterizedTypedef</a>&lt;bool&gt;</span>
116+
<span class="name ">fieldWithTypedef</span> <div class="features">
117+
read-only
118+
</div>
119+
</section>
120+
121+
<section class="desc markdown">
122+
<p>fieldWithTypedef docs here</p>
123+
</section>
124+
125+
126+
127+
</div> <!-- /.main-content -->
128+
129+
</div> <!-- row -->
130+
</div> <!-- container -->
131+
132+
<footer>
133+
<div class="container-fluid">
134+
<div class="container">
135+
<p class="text-center">
136+
<span class="no-break">
137+
test_package 0.0.1
138+
</span>
139+
&bull;
140+
<span class="no-break">
141+
<a href="https://www.dartlang.org">
142+
<img src="static-assets/favicon.png" alt="Dart" title="Dart" width="16" height="16">
143+
</a>
144+
</span>
145+
&bull;
146+
<span class="copyright no-break">
147+
<a href="http://creativecommons.org/licenses/by-sa/4.0/">cc license</a>
148+
</span>
149+
</p>
150+
</div>
151+
</div>
152+
</footer>
153+
154+
<script src="https://code.jquery.com/jquery-2.1.4.min.js"></script>
155+
<script src="static-assets/typeahead.bundle.min.js"></script>
156+
<script src="static-assets/prettify.js"></script>
157+
<script src="static-assets/URI.js"></script>
158+
<script src="static-assets/script.js"></script>
159+
<!-- Do not remove placeholder -->
160+
<!-- Footer Placeholder -->
161+
162+
</body>
163+
164+
</html>

‎testing/test_package_docs/ex/Apple/hashCode.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ <h5><a href="ex/Apple-class.html">Apple</a></h5>
8787
<li class="section-title">
8888
<a href="ex/Apple-class.html#instance-properties">Properties</a>
8989
</li>
90+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
9091
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
9192
<li><a href="ex/Apple/m.html">m</a></li>
9293
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>

‎testing/test_package_docs/ex/Apple/isGreaterThan.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ <h5><a href="ex/Apple-class.html">Apple</a></h5>
8787
<li class="section-title">
8888
<a href="ex/Apple-class.html#instance-properties">Properties</a>
8989
</li>
90+
<li><a href="ex/Apple/fieldWithTypedef.html">fieldWithTypedef</a></li>
9091
<li class="inherited"><a href="ex/Apple/hashCode.html">hashCode</a></li>
9192
<li><a href="ex/Apple/m.html">m</a></li>
9293
<li class="inherited"><a href="ex/Apple/runtimeType.html">runtimeType</a></li>

0 commit comments

Comments
 (0)