Skip to content

Simplify logic in features rendering. #2595

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

Closed
wants to merge 3 commits into from
Closed
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
12 changes: 0 additions & 12 deletions lib/src/generator/templates.renderers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8143,18 +8143,6 @@ class _Renderer_GetterSetterCombo extends RendererBase<GetterSetterCombo> {
parent: r);
},
),
'comboFeatures': Property(
getValue: (CT_ c) => c.comboFeatures,
renderVariable: (CT_ c, Property<CT_> self,
List<String> remainingNames) =>
self.renderSimpleVariable(
c, remainingNames, 'Set<String>'),
renderIterable:
(CT_ c, RendererBase<CT_> r, List<MustachioNode> ast) {
return c.comboFeatures.map(
(e) => _render_String(e, ast, r.template, parent: r));
},
),
'constantInitializer': Property(
getValue: (CT_ c) => c.constantInitializer,
renderVariable: (CT_ c, Property<CT_> self,
Expand Down
9 changes: 4 additions & 5 deletions lib/src/model/container_member.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ mixin ContainerMember on ModelElement implements EnclosedElement {
}

@override
Set<String> get features {
var _features = super.features;
if (isExtended) _features.add('extended');
return _features;
}
Set<String> get features => {
...super.features,
if (isExtended) 'extended',
};

bool _canonicalEnclosingContainerIsSet = false;
Container _canonicalEnclosingContainer;
Expand Down
2 changes: 2 additions & 0 deletions lib/src/model/getter_setter_combo.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:dartdoc/src/element_type.dart';
import 'package:dartdoc/src/model/model.dart';
import 'package:dartdoc/src/utils.dart';
import 'package:dartdoc/src/warnings.dart';
import 'package:meta/meta.dart';

/// Mixin for top-level variables and fields (aka properties)
mixin GetterSetterCombo on ModelElement {
Expand All @@ -25,6 +26,7 @@ mixin GetterSetterCombo on ModelElement {
}
}

@protected
Set<String> get comboFeatures {
var allFeatures = <String>{};
if (hasExplicitGetter && hasPublicGetter) {
Expand Down
13 changes: 6 additions & 7 deletions lib/src/model/inheritable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,12 @@ mixin Inheritable on ContainerMember {
bool get isCovariant;

@override
Set<String> get features {
var _features = super.features;
if (isOverride) _features.add('override');
if (isInherited) _features.add('inherited');
if (isCovariant) _features.add('covariant');
return _features;
}
Set<String> get features => {
...super.features,
if (isOverride) 'override',
if (isInherited) 'inherited',
if (isCovariant) 'covariant',
};

@override
Library get canonicalLibrary => canonicalEnclosingContainer?.canonicalLibrary;
Expand Down
56 changes: 34 additions & 22 deletions lib/src/model/model_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,8 @@ abstract class ModelElement extends Canonicalization
throw 'Unknown type ${e.runtimeType}';
}

static const _htmlEscape = HtmlEscape();

// Stub for mustache, which would otherwise search enclosing elements to find
// names for members.
bool get hasCategoryNames => false;
Expand Down Expand Up @@ -412,30 +414,11 @@ abstract class ModelElement extends Canonicalization
var annotationStrings = <String>[];
if (md == null) return annotationStrings;
for (var a in md) {
var annotation = (const HtmlEscape()).convert(a.toSource());
var annotationElement = a.element;

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 ConstructorElement).returnType.element;
} else if (annotationElement is PropertyAccessorElement) {
annotationElement =
(annotationElement as PropertyAccessorElement).variable;
}
if (annotationElement is Member) {
annotationElement = (annotationElement as Member).declaration;
}

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

var annotation = _htmlEscape.convert(a.toSource());
var annotationModelElement =
packageGraph.findCanonicalModelElementFor(annotationElement);
packageGraph.findCanonicalModelElementFor(a.realElement);
if (annotationModelElement != null) {
annotation = annotation.replaceFirst(
annotationModelElement.name, annotationModelElement.linkedName);
Expand All @@ -445,6 +428,10 @@ abstract class ModelElement extends Canonicalization
return annotationStrings;
}

/// Returns whether [annotationElement] should be displayed when rendering
/// this element.
///
/// Some annotations are intended to be invisible (such as `@pragma`).
bool _shouldDisplayAnnotation(Element annotationElement) {
if (annotationElement is ClassElement) {
var annotationClass =
Expand Down Expand Up @@ -540,6 +527,8 @@ abstract class ModelElement extends Canonicalization
};
}

/// Returns [features] as a single String, sorted [byFeatureOrdering], joined
/// with commas.
String get featuresAsString {
var allFeatures = features.toList()..sort(byFeatureOrdering);
return allFeatures.join(', ');
Expand Down Expand Up @@ -1222,3 +1211,26 @@ abstract class ModelElement extends Canonicalization
});
}
}

extension on ElementAnnotation {
/// Returns this annotation's "real" element, passing through unused,
/// intermediate elements.
Element get realElement {
Element getDeclaration(Element e) => e is Member ? e.declaration : e;

var element = this.element;

if (element 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.
return getDeclaration(element.returnType.element);
} else if (element is PropertyAccessorElement) {
return getDeclaration(element.variable);
} else {
return getDeclaration(element);
}
}
}
2 changes: 1 addition & 1 deletion lib/templates/html/_features.html
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{ #featuresAsString.isNotEmpty }}<div class="features">{{{featuresAsString}}}</div>{{ /featuresAsString.isNotEmpty }}
{{ #features }}<div class="features">{{{featuresAsString}}}</div>{{ /features }}
2 changes: 1 addition & 1 deletion lib/templates/md/_features.md
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{{ #featuresAsString.isNotEmpty }}_{{{featuresAsString}}}_{{ /featuresAsString.isNotEmpty }}
{{ #features }}_{{{featuresAsString}}}_{{ /features }}