Skip to content

Commit 3399c3f

Browse files
jcollins-gjcollins-g
authored
Rewrite parameter handling and add required named parameters for nnbd (#2075)
* Rewrite parameter handling and add required named parameters for nnbd * Reduce indentation from named/optional parameters
1 parent ab52275 commit 3399c3f

File tree

7 files changed

+266
-131
lines changed

7 files changed

+266
-131
lines changed

‎lib/resources/styles.css‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,12 @@ dl dt.callable .name {
381381
display: unset;
382382
}
383383

384+
.parameter-list {
385+
display: table-cell;
386+
margin-left: 10px;
387+
list-style-type: none;
388+
}
389+
384390
.signature {
385391
color: #727272;
386392
}

‎lib/src/model/model_element.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,8 @@ abstract class ModelElement extends Canonicalization
930930

931931
String get linkedParams => utils.linkedParams(parameters);
932932

933-
String get linkedParamsLines => utils.linkedParams(parameters).trim();
933+
String get linkedParamsLines =>
934+
utils.linkedParams(parameters, asList: true).trim();
934935

935936
String get linkedParamsNoMetadata =>
936937
utils.linkedParams(parameters, showMetadata: false);

‎lib/src/model/parameter.dart‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,15 @@ class Parameter extends ModelElement implements EnclosedElement {
6262

6363
bool get isCovariant => _parameter.isCovariant;
6464

65-
bool get isOptional => _parameter.isOptional;
65+
bool get isRequiredPositional => _parameter.isRequiredPositional;
6666

67-
bool get isOptionalNamed => _parameter.isNamed;
67+
bool get isNamed => _parameter.isNamed;
6868

6969
bool get isOptionalPositional => _parameter.isOptionalPositional;
7070

71+
/// Only true if this is a required named parameter.
72+
bool get isRequiredNamed => _parameter.isRequiredNamed;
73+
7174
@override
7275
String get kind => 'parameter';
7376

‎lib/src/model_utils.dart‎

Lines changed: 161 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -16,130 +16,184 @@ import 'package:dartdoc/src/element_type.dart';
1616

1717
final Map<String, String> _fileContents = <String, String>{};
1818

19-
String linkedParams(List<Parameter> parameters,
20-
{bool showMetadata = true,
21-
bool showNames = true,
22-
String separator = ', '}) {
23-
List<Parameter> requiredParams =
24-
parameters.where((Parameter p) => !p.isOptional).toList();
25-
List<Parameter> positionalParams =
26-
parameters.where((Parameter p) => p.isOptionalPositional).toList();
27-
List<Parameter> namedParams =
28-
parameters.where((Parameter p) => p.isOptionalNamed).toList();
29-
30-
StringBuffer builder = StringBuffer();
31-
32-
// prefix
33-
if (requiredParams.isEmpty && positionalParams.isNotEmpty) {
34-
builder.write('[');
35-
} else if (requiredParams.isEmpty && namedParams.isNotEmpty) {
36-
builder.write('{');
37-
}
38-
39-
// index over params
40-
for (Parameter param in requiredParams) {
41-
bool isLast = param == requiredParams.last;
42-
String ext;
43-
if (isLast && positionalParams.isNotEmpty) {
44-
ext = ', [';
45-
} else if (isLast && namedParams.isNotEmpty) {
46-
ext = ', {';
47-
} else {
48-
ext = isLast ? '' : ', ';
49-
}
50-
builder.write(renderParam(param, ext, showMetadata, showNames));
51-
builder.write(' ');
52-
}
53-
for (Parameter param in positionalParams) {
54-
bool isLast = param == positionalParams.last;
55-
builder
56-
.write(renderParam(param, isLast ? '' : ', ', showMetadata, showNames));
57-
builder.write(' ');
58-
}
59-
for (Parameter param in namedParams) {
60-
bool isLast = param == namedParams.last;
61-
builder
62-
.write(renderParam(param, isLast ? '' : ', ', showMetadata, showNames));
63-
builder.write(' ');
64-
}
19+
/// Render HTML in an extended vertical format using <ol> tag.
20+
class ParameterRendererHtmlList extends ParameterRendererHtml {
21+
ParameterRendererHtmlList({bool showMetadata = true, bool showNames = true})
22+
: super(showMetadata: showMetadata, showNames: showNames);
23+
@override
24+
String listItem(String listItem) => '<li>$listItem</li>\n';
25+
@override
26+
// TODO(jcollins-g): consider comma separated lists and more advanced css.
27+
String orderedList(String listItems) =>
28+
'<ol class="parameter-list">$listItems</ol>\n';
29+
}
6530

66-
// suffix
67-
if (namedParams.isNotEmpty) {
68-
builder.write('}');
69-
} else if (positionalParams.isNotEmpty) {
70-
builder.write(']');
71-
}
31+
/// Render HTML suitable for a single, wrapped line.
32+
class ParameterRendererHtml extends ParameterRenderer {
33+
@override
34+
final bool showMetadata;
35+
@override
36+
final bool showNames;
37+
ParameterRendererHtml({this.showMetadata = true, this.showNames = true});
7238

73-
return builder.toString().trim();
39+
@override
40+
String listItem(String listItem) => '${listItem}<wbr>';
41+
@override
42+
String orderedList(String listItems) => listItems;
43+
@override
44+
String annotation(String annotation) => '<span>$annotation</span>';
45+
@override
46+
String covariant(String covariant) => '<span>$covariant</span>';
47+
@override
48+
String defaultValue(String defaultValue) =>
49+
'<span class="default-value">$defaultValue</span>';
50+
@override
51+
String parameter(String parameter, String htmlId) =>
52+
'<span class="parameter" id="${htmlId}">$parameter</span>';
53+
@override
54+
String parameterName(String parameterName) =>
55+
'<span class="parameter-name">$parameterName</span>';
56+
@override
57+
String typeName(String typeName) =>
58+
'<span class="type-annotation">$typeName</span>';
59+
@override
60+
String required(String required) => '<span>$required</span>';
7461
}
7562

76-
String renderParam(
77-
Parameter param, String suffix, bool showMetadata, bool showNames) {
78-
StringBuffer buf = StringBuffer();
79-
ElementType paramModelType = param.modelType;
63+
abstract class ParameterRenderer {
64+
bool get showMetadata;
65+
bool get showNames;
8066

81-
buf.write('<span class="parameter" id="${param.htmlId}">');
82-
if (showMetadata && param.hasAnnotations) {
83-
param.annotations.forEach((String annotation) {
84-
buf.write('<span>$annotation</span> ');
67+
String listItem(String item);
68+
String orderedList(String listItems);
69+
String annotation(String annotation);
70+
String covariant(String covariant);
71+
String defaultValue(String defaultValue);
72+
String parameter(String parameter, String id);
73+
String parameterName(String parameterName);
74+
String typeName(String typeName);
75+
String required(String required);
76+
77+
String _linkedParameterSublist(List<Parameter> parameters, bool trailingComma,
78+
{String thisOpenBracket = '', String thisCloseBracket = ''}) {
79+
StringBuffer builder = StringBuffer();
80+
parameters.forEach((p) {
81+
String prefix = '';
82+
String suffix = '';
83+
if (identical(p, parameters.first)) {
84+
prefix = thisOpenBracket;
85+
}
86+
if (identical(p, parameters.last)) {
87+
suffix += thisCloseBracket;
88+
if (trailingComma) suffix += ', ';
89+
} else {
90+
suffix += ', ';
91+
}
92+
builder.write(
93+
listItem(parameter(prefix + renderParam(p) + suffix, p.htmlId)));
8594
});
95+
return builder.toString();
8696
}
87-
if (param.isCovariant) {
88-
buf.write('<span>covariant</span> ');
89-
}
90-
if (paramModelType is CallableElementTypeMixin ||
91-
paramModelType.type is FunctionType) {
92-
String returnTypeName;
93-
if (paramModelType.isTypedef) {
94-
returnTypeName = paramModelType.linkedName;
95-
} else {
96-
returnTypeName = paramModelType.createLinkedReturnTypeName();
97+
98+
String linkedParams(List<Parameter> parameters) {
99+
List<Parameter> positionalParams =
100+
parameters.where((Parameter p) => p.isRequiredPositional).toList();
101+
List<Parameter> optionalPositionalParams =
102+
parameters.where((Parameter p) => p.isOptionalPositional).toList();
103+
List<Parameter> namedParams =
104+
parameters.where((Parameter p) => p.isNamed).toList();
105+
106+
String positional = '', optional = '', named = '';
107+
if (positionalParams.isNotEmpty) {
108+
positional = _linkedParameterSublist(positionalParams,
109+
optionalPositionalParams.isNotEmpty || namedParams.isNotEmpty);
97110
}
98-
buf.write('<span class="type-annotation">${returnTypeName}</span>');
99-
if (showNames) {
100-
buf.write(' <span class="parameter-name">${param.name}</span>');
101-
} else if (paramModelType.isTypedef ||
102-
paramModelType is CallableAnonymousElementType ||
103-
paramModelType.type is FunctionType) {
104-
buf.write(' <span class="parameter-name">${paramModelType.name}</span>');
111+
if (optionalPositionalParams.isNotEmpty) {
112+
optional = _linkedParameterSublist(
113+
optionalPositionalParams, namedParams.isNotEmpty,
114+
thisOpenBracket: '[', thisCloseBracket: ']');
105115
}
106-
if (!paramModelType.isTypedef && paramModelType is DefinedElementType) {
107-
buf.write('(');
108-
buf.write(linkedParams(paramModelType.element.parameters,
109-
showNames: showNames, showMetadata: showMetadata));
110-
buf.write(')');
116+
if (namedParams.isNotEmpty) {
117+
named = _linkedParameterSublist(namedParams, false,
118+
thisOpenBracket: '{', thisCloseBracket: '}');
111119
}
112-
if (!paramModelType.isTypedef && paramModelType.type is FunctionType) {
113-
buf.write('(');
114-
buf.write(linkedParams(
115-
(paramModelType as UndefinedElementType).parameters,
116-
showNames: showNames,
117-
showMetadata: showMetadata));
118-
buf.write(')');
120+
return (orderedList(positional + optional + named));
121+
}
122+
123+
String renderParam(Parameter param) {
124+
StringBuffer buf = StringBuffer();
125+
ElementType paramModelType = param.modelType;
126+
127+
if (showMetadata && param.hasAnnotations) {
128+
buf.write(param.annotations.map(annotation).join(' ') + ' ');
119129
}
120-
} else if (param.modelType != null) {
121-
String typeName = paramModelType.linkedName;
122-
if (typeName.isNotEmpty) {
123-
buf.write('<span class="type-annotation">$typeName</span>');
130+
if (param.isRequiredNamed) {
131+
buf.write(required('required') + ' ');
124132
}
125-
if (typeName.isNotEmpty && showNames && param.name.isNotEmpty) {
126-
buf.write(' ');
133+
if (param.isCovariant) {
134+
buf.write(covariant('covariant') + ' ');
127135
}
128-
if (showNames && param.name.isNotEmpty) {
129-
buf.write('<span class="parameter-name">${param.name}</span>');
136+
if (paramModelType is CallableElementTypeMixin ||
137+
paramModelType.type is FunctionType) {
138+
String returnTypeName;
139+
if (paramModelType.isTypedef) {
140+
returnTypeName = paramModelType.linkedName;
141+
} else {
142+
returnTypeName = paramModelType.createLinkedReturnTypeName();
143+
}
144+
buf.write(typeName(returnTypeName));
145+
if (showNames) {
146+
buf.write(' ${parameterName(param.name)}');
147+
} else if (paramModelType.isTypedef ||
148+
paramModelType is CallableAnonymousElementType ||
149+
paramModelType.type is FunctionType) {
150+
buf.write(' ${parameterName(paramModelType.name)}');
151+
}
152+
if (!paramModelType.isTypedef && paramModelType is DefinedElementType) {
153+
buf.write('(');
154+
buf.write(linkedParams(paramModelType.element.parameters));
155+
buf.write(')');
156+
}
157+
if (!paramModelType.isTypedef && paramModelType.type is FunctionType) {
158+
buf.write('(');
159+
buf.write(
160+
linkedParams((paramModelType as UndefinedElementType).parameters));
161+
buf.write(')');
162+
}
163+
} else if (param.modelType != null) {
164+
String linkedTypeName = paramModelType.linkedName;
165+
if (linkedTypeName.isNotEmpty) {
166+
buf.write(typeName(linkedTypeName));
167+
if (showNames && param.name.isNotEmpty) {
168+
buf.write(' ');
169+
}
170+
}
171+
if (showNames && param.name.isNotEmpty) {
172+
buf.write(parameterName(param.name));
173+
}
130174
}
131-
}
132175

133-
if (param.hasDefaultValue) {
134-
if (param.isOptionalNamed) {
135-
buf.write(': ');
136-
} else {
137-
buf.write(' = ');
176+
if (param.hasDefaultValue) {
177+
if (param.isNamed) {
178+
buf.write(': ');
179+
} else {
180+
buf.write(' = ');
181+
}
182+
buf.write(defaultValue(param.defaultValue));
138183
}
139-
buf.write('<span class="default-value">${param.defaultValue}</span>');
184+
return buf.toString();
185+
}
186+
}
187+
188+
String linkedParams(List<Parameter> parameters,
189+
{showMetadata = true, showNames = true, asList = false}) {
190+
if (asList) {
191+
return ParameterRendererHtmlList(
192+
showMetadata: showMetadata, showNames: showNames)
193+
.linkedParams(parameters);
140194
}
141-
buf.write('${suffix}</span>');
142-
return buf.toString();
195+
return ParameterRendererHtml(showMetadata: showMetadata, showNames: showNames)
196+
.linkedParams(parameters);
143197
}
144198

145199
/// Returns the [AstNode] for a given [Element].

‎test/model_special_cases_test.dart‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,61 @@ void main() {
2929
// Experimental features not yet enabled by default. Move tests out of this block
3030
// when the feature is enabled by default.
3131
group('Experiments', () {
32-
Library lateFinalWithoutInitializer;
32+
Library lateFinalWithoutInitializer, nnbdClassMemberDeclarations;
33+
Class b;
3334
setUpAll(() async {
3435
lateFinalWithoutInitializer = (await utils.testPackageGraphExperiments)
3536
.libraries
3637
.firstWhere((lib) => lib.name == 'late_final_without_initializer');
38+
nnbdClassMemberDeclarations = (await utils.testPackageGraphExperiments)
39+
.libraries
40+
.firstWhere((lib) => lib.name == 'nnbd_class_member_declarations');
41+
b = nnbdClassMemberDeclarations.allClasses
42+
.firstWhere((c) => c.name == 'B');
43+
});
44+
45+
test('method parameters with required', () {
46+
Method m1 = b.allInstanceMethods.firstWhere((m) => m.name == 'm1');
47+
Parameter p1 = m1.allParameters.firstWhere((p) => p.name == 'p1');
48+
Parameter p2 = m1.allParameters.firstWhere((p) => p.name == 'p2');
49+
expect(p1.isRequiredNamed, isTrue);
50+
expect(p2.isRequiredNamed, isFalse);
51+
expect(p2.isNamed, isTrue);
52+
53+
expect(
54+
m1.linkedParamsLines,
55+
equals(
56+
'<ol class="parameter-list"><li><span class="parameter" id="m1-param-some"><span class="type-annotation">int</span> <span class="parameter-name">some</span>, </span></li>\n'
57+
'<li><span class="parameter" id="m1-param-regular"><span class="type-annotation">dynamic</span> <span class="parameter-name">regular</span>, </span></li>\n'
58+
'<li><span class="parameter" id="m1-param-parameters"><span>covariant</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">parameters</span>, </span></li>\n'
59+
'<li><span class="parameter" id="m1-param-p1">{<span>required</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">p1</span>, </span></li>\n'
60+
'<li><span class="parameter" id="m1-param-p2"><span class="type-annotation">int</span> <span class="parameter-name">p2</span>: <span class="default-value">3</span>, </span></li>\n'
61+
'<li><span class="parameter" id="m1-param-p3"><span>required</span> <span>covariant</span> <span class="type-annotation">dynamic</span> <span class="parameter-name">p3</span>, </span></li>\n'
62+
'<li><span class="parameter" id="m1-param-p4"><span>required</span> <span>covariant</span> <span class="type-annotation">int</span> <span class="parameter-name">p4</span>}</span></li>\n'
63+
'</ol>'));
64+
});
65+
66+
test('verify no regression on ordinary optionals', () {
67+
Method m2 = b.allInstanceMethods.firstWhere((m) => m.name == 'm2');
68+
Parameter sometimes =
69+
m2.allParameters.firstWhere((p) => p.name == 'sometimes');
70+
Parameter optionals =
71+
m2.allParameters.firstWhere((p) => p.name == 'optionals');
72+
expect(sometimes.isRequiredNamed, isFalse);
73+
expect(sometimes.isRequiredPositional, isTrue);
74+
expect(sometimes.isOptionalPositional, isFalse);
75+
expect(optionals.isRequiredNamed, isFalse);
76+
expect(optionals.isRequiredPositional, isFalse);
77+
expect(optionals.isOptionalPositional, isTrue);
78+
79+
expect(
80+
m2.linkedParamsLines,
81+
equals(
82+
'<ol class="parameter-list"><li><span class="parameter" id="m2-param-sometimes"><span class="type-annotation">int</span> <span class="parameter-name">sometimes</span>, </span></li>\n'
83+
'<li><span class="parameter" id="m2-param-we"><span class="type-annotation">dynamic</span> <span class="parameter-name">we</span>, </span></li>\n'
84+
'<li><span class="parameter" id="m2-param-have">[<span class="type-annotation">String</span> <span class="parameter-name">have</span>, </span></li>\n'
85+
'<li><span class="parameter" id="m2-param-optionals"><span class="type-annotation">double</span> <span class="parameter-name">optionals</span>]</span></li>\n'
86+
'</ol>'));
3787
});
3888

3989
test('Late final class member test', () {

0 commit comments

Comments
 (0)