Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link const annotations to their docs #2313

Merged
merged 2 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions lib/src/model/field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,14 @@ class Field extends ModelElement

@override
List<String> get annotations {
var all_annotations = <String>[];
all_annotations.addAll(super.annotations);
var allAnnotations = [...super.annotations];

if (element is PropertyInducingElement) {
var pie = element as PropertyInducingElement;
all_annotations.addAll(annotationsFromMetadata(pie.getter?.metadata));
all_annotations.addAll(annotationsFromMetadata(pie.setter?.metadata));
allAnnotations.addAll(annotationsFromMetadata(pie.getter?.metadata));
allAnnotations.addAll(annotationsFromMetadata(pie.setter?.metadata));
}
return all_annotations.toList(growable: false);
return allAnnotations;
}

@override
Expand Down
68 changes: 42 additions & 26 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,46 +399,63 @@ abstract class ModelElement extends Canonicalization
List<String> get annotations => annotationsFromMetadata(element.metadata);

/// Returns linked annotations from a given metadata set, with escaping.
// TODO(srawlins): Attempt to revive constructor arguments in an annotation,
// akin to source_gen's Reviver, in order to link to inner components. For
// example, in `@Foo(const Bar(), baz: <Baz>[Baz.one, Baz.two])`, link to
// `Foo`, `Bar`, `Baz`, `Baz.one`, and `Baz.two`.
List<String> annotationsFromMetadata(List<ElementAnnotation> md) {
var annotationStrings = <String>[];
if (md == null) return annotationStrings;
for (var a in md) {
var annotation = (const HtmlEscape()).convert(a.toSource());
var annotationElement = a.element;

ClassElement annotationClassElement;
if (annotationElement is ExecutableElement) {
if (annotationElement is ConstructorElement) {
// TODO(srawlins): I think we should actually link to the constructor,
// which may have details about parameters. For example, given the
// annotation `@Immutable('text')`, the constructor documents what the
// parameter is, and the class only references `immutable`. It's a
// lose-lose cycle of mis-direction.
annotationElement =
(annotationElement as ExecutableElement).returnType.element;
(annotationElement as ConstructorElement).returnType.element;
} else if (annotationElement is PropertyAccessorElement) {
annotationElement =
(annotationElement as PropertyAccessorElement).variable;
}
if (annotationElement is ClassElement) {
annotationClassElement = annotationElement;
if (annotationElement is Member) {
annotationElement = (annotationElement as Member).declaration;
}

// Some annotations are intended to be invisible (such as `@pragma`).
if (!_shouldDisplayAnnotation(annotationElement)) continue;

var annotationModelElement =
packageGraph.findCanonicalModelElementFor(annotationElement);
// annotationElement can be null if the element can't be resolved.
var annotationClass = packageGraph
.findCanonicalModelElementFor(annotationClassElement) as Class;
if (annotationClass == null &&
annotationElement != null &&
annotationClassElement != null) {
annotationClass =
ModelElement.fromElement(annotationClassElement, packageGraph)
as Class;
}
// Some annotations are intended to be invisible (@pragma)
if (annotationClass == null ||
!packageGraph.invisibleAnnotations.contains(annotationClass)) {
if (annotationModelElement != null) {
annotation = annotation.replaceFirst(
annotationModelElement.name, annotationModelElement.linkedName);
}
annotationStrings.add(annotation);
if (annotationModelElement != null) {
annotation = annotation.replaceFirst(
annotationModelElement.name, annotationModelElement.linkedName);
}
annotationStrings.add(annotation);
}
return annotationStrings;
}

bool _shouldDisplayAnnotation(Element annotationElement) {
if (annotationElement is ClassElement) {
var annotationClass =
packageGraph.findCanonicalModelElementFor(annotationElement) as Class;
if (annotationClass == null && annotationElement != null) {
annotationClass =
ModelElement.fromElement(annotationElement, packageGraph) as Class;
}

return annotationClass == null ||
packageGraph.isAnnotationVisible(annotationClass);
}
// We cannot resolve it, which does not prevent it from being displayed.
return true;
}

bool _isPublic;

@override
Expand Down Expand Up @@ -506,14 +523,13 @@ abstract class ModelElement extends Canonicalization
.where((s) => s.isNotEmpty));

Set<String> get features {
var allFeatures = <String>{};
allFeatures.addAll(annotations);
var allFeatures = <String>{...annotations};

// Replace the @override annotation with a feature that explicitly
// indicates whether an override has occurred.
allFeatures.remove('@override');

// Drop the plain "deprecated" annotation, that's indicated via
// Drop the plain "deprecated" annotation; that's indicated via
// strikethroughs. Custom @Deprecated() will still appear.
allFeatures.remove('@deprecated');
// const and static are not needed here because const/static elements get
Expand Down
12 changes: 5 additions & 7 deletions lib/src/model/package_graph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -641,13 +641,11 @@ class PackageGraph {

/// Returns the set of [Class] objects that are similar to pragma
/// in that we should never count them as documentable annotations.
Set<Class> get invisibleAnnotations {
if (_invisibleAnnotations == null) {
_invisibleAnnotations = {};
_invisibleAnnotations.add(specialClasses[SpecialClass.pragma]);
}
return _invisibleAnnotations;
}
Set<Class> get invisibleAnnotations =>
_invisibleAnnotations ??= {specialClasses[SpecialClass.pragma]};

bool isAnnotationVisible(Class class_) =>
!invisibleAnnotations.contains(class_);

@override
String toString() {
Expand Down
32 changes: 25 additions & 7 deletions test/model_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3671,7 +3671,10 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
fakeLibrary.functions.firstWhere((f) => f.name == 'paintImage1');
var params =
ParameterRendererHtml().renderLinkedParams(method.parameters);
expect(params, contains('@required'));
expect(
params,
contains(
'@<a href="${HTMLBASE_PLACEHOLDER}fake/required-constant.html">required</a>'));
});

test('param exported in library', () {
Expand All @@ -3685,8 +3688,9 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
.renderLinkedParams(methodWithTypedefParam.parameters);
expect(
params,
equals(
'<span class="parameter" id="methodWithTypedefParam-param-p"><span class="type-annotation"><a href="${HTMLBASE_PLACEHOLDER}ex/processMessage.html">processMessage</a></span> <span class="parameter-name">p</span></span><wbr>'));
equals('<span class="parameter" id="methodWithTypedefParam-param-p">'
'<span class="type-annotation"><a href="${HTMLBASE_PLACEHOLDER}ex/processMessage.html">processMessage</a></span> '
'<span class="parameter-name">p</span></span><wbr>'));
});
});

Expand Down Expand Up @@ -3765,20 +3769,34 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
expect(
forAnnotation.annotations.first,
equals(
'@<a href="${HTMLBASE_PLACEHOLDER}ex/ForAnnotation-class.html">ForAnnotation</a>(&#39;my value&#39;)'));
'@<a href="${HTMLBASE_PLACEHOLDER}ex/ForAnnotation-class.html">ForAnnotation</a>'
'(&#39;my value&#39;)'));
});

test('methods has the right annotation', () {
var m = dog.instanceMethods.singleWhere((m) => m.name == 'getClassA');
expect(m.hasAnnotations, isTrue);
expect(m.annotations.first, equals('@deprecated'));
expect(
m.annotations.first,
equals(
'@<a href="${HTMLBASE_PLACEHOLDER}ex/deprecated-constant.html">deprecated</a>'));
});

test('method annotations have the right link and are escaped', () {
test('constructor annotations have the right link and are escaped', () {
expect(
ctr.annotations[0],
equals(
'@<a href="${HTMLBASE_PLACEHOLDER}ex/Deprecated-class.html">Deprecated</a>(&quot;Internal use&quot;)'));
'@<a href="${HTMLBASE_PLACEHOLDER}ex/Deprecated-class.html">Deprecated</a>'
'(&quot;Internal use&quot;)'));
});

test('const annotations have the right link and are escaped', () {
var createDog2 =
dog.staticMethods.firstWhere((c) => c.name == 'createDog2');
expect(
createDog2.annotations[0],
equals(
'@<a href="${HTMLBASE_PLACEHOLDER}ex/deprecated-constant.html">deprecated</a>'));
});
});

Expand Down
5 changes: 5 additions & 0 deletions testing/test_package/lib/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,11 @@ class Dog implements Cat, E {
return Dog.deprecatedCreate(s);
}

@deprecated
static Dog createDog2(String s) {
return Dog.deprecatedCreate(s);
}

@override
void abstractMethod() {}
}
Expand Down