Skip to content

Commit 45ba6ed

Browse files
srawlinssrawlins
authored
Display constructor names in annotations (#4115)
Two changes in one here: * I saw in manual testing that a class annotated with `@Deprecated.extend()` was shown in dartdoc's HTML to just be annotated with `@Deprecated`. 🤕 ouch. Dunno how that has been the case for so many years; I guess named constructors are not used too much in annotations 🤷 . So this PR makes dartdoc use the _constructor_ involved (`Deprecated.extend`) instead of the _type_ which is constructed (`Deprecated`). * _However_, that change makes all regular `@Deprecated("reason")` annotations be printed as `@Deprecated.new("reason")`. That `.new` looks _very non-idiomatic_. Gross. So I coupled another change here where we override `displayName` on `Constructor` to _not_ include `.new` if we're looking at an unnamed constructor. This should be good to go. We might deprecate extending `RegExp` and `RegExpMatch` in Dart 3.10; it'd be cool if we displayed the deprecated annotations correctly for this release. Note that as per 669b15f, we won't show the element as ~~struck through~~, so the release is safe. But this PR would be a cherry on top. Fixes #4107
1 parent ec2a4fe commit 45ba6ed

File tree

10 files changed

+86
-25
lines changed

10 files changed

+86
-25
lines changed

‎lib/src/model/annotation.dart‎

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import 'dart:convert';
66

77
import 'package:analyzer/dart/element/element.dart';
8+
import 'package:analyzer/dart/element/type.dart';
89
import 'package:dartdoc/src/element_type.dart';
910
import 'package:dartdoc/src/model/attribute.dart';
1011
import 'package:dartdoc/src/model/class.dart';
@@ -36,14 +37,47 @@ final class Annotation extends Attribute {
3637
var parameterText =
3738
source.substring(startIndex == -1 ? source.length : startIndex);
3839

39-
return '@$linkedName${const HtmlEscape().convert(parameterText)}';
40+
var escapedParameterText = const HtmlEscape().convert(parameterText);
41+
return '@$linkedName$_linkedTypeArguments$escapedParameterText';
4042
}
4143

4244
@override
43-
String get linkedName => _annotation.element is PropertyAccessorElement
44-
? _packageGraph.getModelForElement(_annotation.element!).linkedName
45-
// TODO(jcollins-g): consider linking to constructor instead of type?
46-
: _modelType.linkedName;
45+
String get linkedName => switch (_annotation.element) {
46+
PropertyAccessorElement element =>
47+
_packageGraph.getModelForElement(element).linkedName,
48+
ConstructorElement element =>
49+
_packageGraph.getModelForElement(element).linkedName,
50+
_ => _modelType.linkedName
51+
};
52+
53+
/// The linked type argument text, with `<` and `>`, if there are any type
54+
/// arguments.
55+
String get _linkedTypeArguments {
56+
if (_annotation.element is PropertyAccessorElement) {
57+
return '';
58+
}
59+
60+
var type = _modelType.type;
61+
if (type is! InterfaceType) {
62+
return '';
63+
}
64+
65+
var typeArguments = type.typeArguments;
66+
if (typeArguments.isEmpty) {
67+
return '';
68+
}
69+
70+
var buffer = StringBuffer();
71+
buffer.write('&lt;');
72+
for (var t in typeArguments) {
73+
buffer.write(_packageGraph.getTypeFor(t, _library).linkedName);
74+
if (t != typeArguments.last) {
75+
buffer.write(', ');
76+
}
77+
}
78+
buffer.write('&gt;');
79+
return buffer.toString();
80+
}
4781

4882
late final ElementType _modelType = switch (_annotation.element) {
4983
ConstructorElement(:var returnType) =>

‎lib/src/model/constructor.dart‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ class Constructor extends ModelElement with ContainerMember, TypeParameters {
9292
@override
9393
Kind get kind => Kind.constructor;
9494

95-
late final Callable modelType =
96-
getTypeFor(element.type, library) as Callable;
95+
late final Callable modelType = getTypeFor(element.type, library) as Callable;
9796

9897
@override
9998
String get name =>
@@ -102,6 +101,11 @@ class Constructor extends ModelElement with ContainerMember, TypeParameters {
102101
// code there and elsewhere with simple references to the name.
103102
'${enclosingElement.name}.${element.name}';
104103

104+
@override
105+
String get displayName => isUnnamedConstructor
106+
? enclosingElement.name
107+
: '${enclosingElement.name}.${element.name}';
108+
105109
@override
106110
String get nameWithGenerics {
107111
var constructorName = element.name!;

‎test/annotations_test.dart‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ int value = 0;
4848
var annotation = valueVariable.annotations.single;
4949
expect(
5050
annotation.linkedName,
51-
'<a href="$dartCoreUrlPrefix/Deprecated-class.html">Deprecated</a>',
51+
'<a href="$dartCoreUrlPrefix/Deprecated/Deprecated.html">Deprecated</a>',
5252
);
5353
expect(
5454
annotation.linkedNameWithParameters,
55-
'@<a href="$dartCoreUrlPrefix/Deprecated-class.html">Deprecated</a>'
55+
'@<a href="$dartCoreUrlPrefix/Deprecated/Deprecated.html">Deprecated</a>'
5656
'(&#39;text&#39;)',
5757
);
5858
}
@@ -97,16 +97,40 @@ int value = 0;
9797
var annotation = valueVariable.annotations.single;
9898
expect(
9999
annotation.linkedName,
100-
'<a href="${htmlBasePlaceholder}annotations/MyAnnotation-class.html">'
100+
'<a href="${htmlBasePlaceholder}annotations/MyAnnotation/MyAnnotation.html">'
101101
'MyAnnotation</a>',
102102
);
103103
expect(
104104
annotation.linkedNameWithParameters,
105-
'@<a href="${htmlBasePlaceholder}annotations/MyAnnotation-class.html">'
105+
'@<a href="${htmlBasePlaceholder}annotations/MyAnnotation/MyAnnotation.html">'
106106
'MyAnnotation</a>(true)',
107107
);
108108
}
109109

110+
void test_locallyDeclaredConstructorCall_named() async {
111+
var library = await bootPackageWithLibrary('''
112+
class MyAnnotation {
113+
const MyAnnotation.named(bool b);
114+
}
115+
116+
@MyAnnotation.named(true)
117+
int value = 0;
118+
''');
119+
var valueVariable = library.properties.named('value');
120+
expect(valueVariable.hasAnnotations, true);
121+
var annotation = valueVariable.annotations.single;
122+
expect(
123+
annotation.linkedName,
124+
'<a href="${htmlBasePlaceholder}annotations/MyAnnotation/MyAnnotation.named.html">'
125+
'MyAnnotation.named</a>',
126+
);
127+
expect(
128+
annotation.linkedNameWithParameters,
129+
'@<a href="${htmlBasePlaceholder}annotations/MyAnnotation/MyAnnotation.named.html">'
130+
'MyAnnotation.named</a>(true)',
131+
);
132+
}
133+
110134
void test_genericConstructorCall() async {
111135
var library = await bootPackageWithLibrary('''
112136
class Ann<T> {
@@ -122,15 +146,13 @@ int value = 0;
122146
var annotation = valueVariable.annotations.single;
123147
expect(
124148
annotation.linkedName,
125-
'<a href="${htmlBasePlaceholder}annotations/Ann-class.html">Ann</a>'
126-
'<span class="signature">&lt;<wbr><span class="type-parameter">'
127-
'<a href="$dartCoreUrlPrefix/bool-class.html">bool</a></span>&gt;</span>',
149+
'<a href="${htmlBasePlaceholder}annotations/Ann/Ann.html">Ann</a>',
128150
);
129151
expect(
130152
annotation.linkedNameWithParameters,
131-
'@<a href="${htmlBasePlaceholder}annotations/Ann-class.html">Ann</a>'
132-
'<span class="signature">&lt;<wbr><span class="type-parameter">'
133-
'<a href="$dartCoreUrlPrefix/bool-class.html">bool</a></span>&gt;</span>(true)',
153+
'@<a href="${htmlBasePlaceholder}annotations/Ann/Ann.html">Ann</a>'
154+
'&lt;<a href="$dartCoreUrlPrefix/bool-class.html">bool</a>&gt;'
155+
'(true)',
134156
);
135157
}
136158
}

‎test/end2end/model_test.dart‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,9 @@ void main() async {
190190
test('Verify type arguments on annotations renders, including parameters',
191191
() {
192192
var ab0 =
193-
'@<a href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/A-class.html">A</a><span class="signature">&lt;<wbr><span class="type-parameter"><a href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/B.html">B</a></span>&gt;</span>(0)';
193+
'@<a href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/A/A.html">A</a>'
194+
'&lt;<a href="%%__HTMLBASE_dartdoc_internal__%%generic_metadata/B.html">B</a>&gt;'
195+
'(0)';
194196

195197
expect(genericMetadata.annotations.first.linkedNameWithParameters,
196198
equals(ab0));

‎test/enums_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ enum E { one, two, three }
207207
expect(eEnum.hasAnnotations, true);
208208
expect(eEnum.annotations, hasLength(1));
209209
expect(eEnum.annotations.single.linkedName,
210-
'<a href="$linkPrefix/C-class.html">C</a>');
210+
'<a href="$linkPrefix/C/C.html">C</a>');
211211
}
212212

213213
void test_hasDocComment() async {

‎test/templates/class_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ class C {
218218

219219
htmlLines.expectMainContentContainsAllInOrder([
220220
matches('<h2>Constructors</h2>'),
221-
matches('<a href="../lib/C/C.html">C.new</a>'),
221+
matches('<a href="../lib/C/C.html">C</a>'),
222222
matches('An unnamed constructor.'),
223223
]);
224224
}

‎test/templates/enum_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ extension Ext<T> on E<T> {}
180180
matches('<dt>Annotations</dt>'),
181181
matches('<ul class="annotation-list eNum-relationships">'),
182182
matches(
183-
r'<li>@<a href="../lib/C-class.html">C</a>\(&#39;message&#39;\)</li>'),
183+
r'<li>@<a href="../lib/C/C.html">C</a>&lt;dynamic&gt;\(&#39;message&#39;\)</li>'),
184184
matches('</ul>'),
185185
]);
186186
});

‎test/templates/extension_type_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ extension type One(int it) {
166166

167167
htmlLines.expectMainContentContainsAllInOrder([
168168
matches('<h2>Constructors</h2>'),
169-
matches('<a href="../lib/One/One.html">One.new</a>'),
169+
matches('<a href="../lib/One/One.html">One</a>'),
170170
matches('<a href="../lib/One/One.named.html">One.named</a>'),
171171
matches('A named constructor.'),
172172
]);

‎test/templates/field_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class A {
5454
matches('<ol class="annotation-list">'),
5555
matches('<li>@deprecated</li>'),
5656
matches(
57-
r'<li>@<a href="../../lib/A-class.html">A</a>\(&#39;message&#39;\)</li>'),
57+
r'<li>@<a href="../../lib/A/A.html">A</a>\(&#39;message&#39;\)</li>'),
5858
matches('</ol>'),
5959
],
6060
);

‎test/templates/method_test.dart‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import 'package:test/test.dart';
66
import 'package:test_reflective_loader/test_reflective_loader.dart';
77

88
import '../src/utils.dart';
9-
109
import 'template_test_base.dart';
1110

1211
void main() async {
@@ -53,7 +52,7 @@ class C {
5352
matches('<ol class="annotation-list">'),
5453
matches('<li>@deprecated</li>'),
5554
matches(
56-
r'<li>@<a href="../../lib/A-class.html">A</a>\(&#39;message&#39;\)</li>'),
55+
r'<li>@<a href="../../lib/A/A.html">A</a>\(&#39;message&#39;\)</li>'),
5756
matches('</ol>'),
5857
],
5958
);

0 commit comments

Comments
 (0)