Remove annotations from the features section of each element (#3268)

srawlins · web-flow · commit eb90a44c54fa · 2022-12-12T16:09:10.000-08:00
diff --git a/lib/src/generator/templates.aot_renderers_for_html.dart b/lib/src/generator/templates.aot_renderers_for_html.dart
diff --git a/lib/src/generator/templates.aot_renderers_for_md.dart b/lib/src/generator/templates.aot_renderers_for_md.dart
diff --git a/lib/src/model/field.dart b/lib/src/model/field.dart
@@ -114,7 +114,7 @@ class Field extends ModelElement
 
   @override
   Set<Feature> get features {
-    var allFeatures = super.features..addAll(comboFeatures);
+    var allFeatures = {...super.features, ...comboFeatures};
     // Combo features can indicate 'inherited' and 'override' if
     // either the getter or setter has one of those properties, but that's not
     // really specific enough for [Field]s that have public getter/setters.
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
@@ -414,10 +414,12 @@ abstract class ModelElement extends Canonicalization
   @override
   late final ModelNode? modelNode = packageGraph.getModelNodeFor(element);
 
-  // Skips over annotations with null elements or that are otherwise
-  // supposed to be invisible (@pragma).  While technically, null elements
-  // indicate invalid code from analyzer's perspective they are present in
-  // sky_engine (@Native) so we don't want to crash here.
+  /// This element's [Annotation]s.
+  ///
+  /// Does not include annotations with `null` elements or that are otherwise
+  /// supposed to be invisible (like `@pragma`). While `null` elements indicate
+  /// invalid code from analyzer's perspective, some are present in `sky_engine`
+  /// (`@Native`) so we don't want to crash here.
   late final List<Annotation> annotations = element.metadata
       .whereNot((m) =>
           m.element == null ||
@@ -458,24 +460,14 @@ abstract class ModelElement extends Canonicalization
       .where((s) => s.isNotEmpty)
       .toSet();
 
-  static const Set<String> _specialFeatures = {
-    // Replace the @override annotation with a feature that explicitly
-    // indicates whether an override has occurred.
-    'override',
-    // Drop the plain "deprecated" annotation; that's indicated via
-    // strikethroughs. Custom @Deprecated() will still appear.
-    'deprecated'
-  };
-
   bool get hasFeatures => features.isNotEmpty;
 
-  /// Usually a superset of [annotations] except where [_specialFeatures]
-  /// replace them, a list of annotations as well as tags applied by
-  /// Dartdoc itself when it notices characteristics of an element
-  /// that need to be documented.  See [Feature] for a list.
+  /// The set of attributes or "features" of this element.
+  ///
+  /// This includes tags applied by Dartdoc for various attributes that should
+  /// be called out. See [Feature] for a list.
   Set<Feature> get features {
     return {
-      ...annotations.whereNot((a) => _specialFeatures.contains(a.name)),
       // 'const' and 'static' are not needed here because 'const' and 'static'
       // elements get their own sections in the doc.
       if (isFinal) Feature.finalFeature,
diff --git a/lib/templates/html/_accessor_getter.html b/lib/templates/html/_accessor_getter.html
@@ -2,9 +2,10 @@
 <section id="getter">
 
 <section class="multi-line-signature">
+  {{ >annotations }}
   <span class="returntype">{{{ modelType.returnType.linkedName }}}</span>
-  {{>name_summary}}
-  {{>features}}
+  {{ >name_summary }}
+  {{ >features }}
 </section>
 
 {{>documentation}}
diff --git a/lib/templates/html/_accessor_setter.html b/lib/templates/html/_accessor_setter.html
@@ -2,9 +2,10 @@
 <section id="setter">
 
 <section class="multi-line-signature">
+  {{ >annotations }}
   <span class="returntype">void</span>
-  {{>name_summary}}<span class="signature">(<wbr>{{{ linkedParamsNoMetadata }}})</span>
-  {{>features}}
+  {{ >name_summary }}<span class="signature">(<wbr>{{{ linkedParamsNoMetadata }}})</span>
+  {{ >features }}
 </section>
 
 {{>documentation}}
diff --git a/lib/templates/html/_annotations.html b/lib/templates/html/_annotations.html
@@ -1,10 +1,9 @@
 {{ #hasAnnotations }}
-  <dt>Annotations</dt>
-  <dd>
-    <ul class="annotation-list {{ relationshipsClass }}">
+  <div>
+    <ol class="annotation-list">
       {{ #annotations }}
         <li>{{{ linkedNameWithParameters }}}</li>
       {{ /annotations }}
-    </ul>
-  </dd>
+    </ol>
+  </div>
 {{ /hasAnnotations }}
diff --git a/lib/templates/html/_callable_multiline.html b/lib/templates/html/_callable_multiline.html
@@ -1,12 +1,4 @@
-{{#hasAnnotations}}
-<div>
-  <ol class="annotation-list">
-    {{#annotations}}
-    <li>{{{linkedNameWithParameters}}}</li>
-    {{/annotations}}
-  </ol>
-</div>
-{{/hasAnnotations}}
+{{ >annotations }}
 
 <span class="returntype">{{{ modelType.returnType.linkedName }}}</span>
 {{>name_summary}}{{{genericParameters}}}(<wbr>{{#hasParameters}}{{{linkedParamsLines}}}{{/hasParameters}})
diff --git a/lib/templates/html/_container_annotations.html b/lib/templates/html/_container_annotations.html
@@ -0,0 +1,11 @@
+{{ #hasAnnotations }}
+  <dt>Annotations</dt>
+  <dd>
+    <ul class="annotation-list {{ relationshipsClass }}">
+      {{ #annotations }}
+        <li>{{{ linkedNameWithParameters }}}</li>
+      {{ /annotations }}
+    </ul>
+  </dd>
+{{ /hasAnnotations }}
+
diff --git a/lib/templates/html/class.html b/lib/templates/html/class.html
@@ -33,7 +33,7 @@
         </ul></dd>
         {{/hasPotentiallyApplicableExtensions}}
 
-        {{ >annotations }}
+        {{ >container_annotations }}
       </dl>
     </section>
     {{/hasModifiers}}
diff --git a/lib/templates/html/constructor.html b/lib/templates/html/constructor.html
@@ -7,15 +7,7 @@
 
     {{#constructor}}
     <section class="multi-line-signature">
-      {{#hasAnnotations}}
-      <div>
-        <ol class="annotation-list">
-          {{#annotations}}
-          <li>{{{linkedNameWithParameters}}}</li>
-          {{/annotations}}
-        </ol>
-      </div>
-      {{/hasAnnotations}}
+      {{ >annotations }}
       {{#isConst}}const{{/isConst}}
       <span class="name {{#isDeprecated}}deprecated{{/isDeprecated}}">{{{nameWithGenerics}}}</span>(<wbr>{{#hasParameters}}{{{linkedParamsLines}}}{{/hasParameters}})
     </section>
diff --git a/lib/templates/html/enum.html b/lib/templates/html/enum.html
@@ -19,7 +19,7 @@ <h1>
         {{ >super_chain }}
         {{ >interfaces }}
         {{ >mixed_in_types }}
-        {{ >annotations }}
+        {{ >container_annotations }}
       </dl>
     </section>
     {{ /hasModifiers }}
diff --git a/lib/templates/html/extension.html b/lib/templates/html/extension.html
@@ -18,7 +18,7 @@
           </ul>
         </dd>
       </dl>
-      {{ >annotations }}
+      {{ >container_annotations }}
     </section>
 
     {{#hasPublicInstanceFields}}
diff --git a/lib/templates/html/function.html b/lib/templates/html/function.html
@@ -7,7 +7,8 @@
 
     {{#function}}
     <section class="multi-line-signature">
-        {{>callable_multiline}}
+        {{ >callable_multiline }}
+        {{ >features }}
     </section>
     {{>documentation}}
 
diff --git a/lib/templates/html/library.html b/lib/templates/html/library.html
@@ -1,6 +1,7 @@
 {{>head}}
 
   <div id="dartdoc-main-content" class="main-content">
+    {{ !-- TODO(srawlins): Add annotations. }}
     {{#self}}
       <div>{{>source_link}}<h1><span class="kind-library">{{{name}}}</span> {{kind}} {{>feature_set}} {{>categorization}}</h1></div>
     {{/self}}
diff --git a/lib/templates/html/method.html b/lib/templates/html/method.html
@@ -7,8 +7,8 @@
 
     {{#method}}
     <section class="multi-line-signature">
-      {{>callable_multiline}}
-      {{>features}}
+      {{ >callable_multiline }}
+      {{ >features }}
     </section>
     {{>documentation}}
 
diff --git a/lib/templates/html/property.html b/lib/templates/html/property.html
@@ -8,9 +8,10 @@
     {{#self}}
       {{#hasNoGetterSetter}}
         <section class="multi-line-signature">
+          {{ >annotations }}
           {{{ modelType.linkedName }}}
-          {{>name_summary}}
-          {{>features}}
+          {{ >name_summary }}
+          {{ >features }}
         </section>
         {{>documentation}}
         {{>source_code}}
diff --git a/lib/templates/html/top_level_property.html b/lib/templates/html/top_level_property.html
@@ -6,9 +6,10 @@
 
       {{#hasNoGetterSetter}}
         <section class="multi-line-signature">
+          {{ >annotations }}
           {{{ modelType.linkedName }}}
-          {{>name_summary}}
-          {{>features}}
+          {{ >name_summary }}
+          {{ >features }}
         </section>
         {{>documentation}}
         {{>source_code}}
diff --git a/lib/templates/md/_accessor_getter.md b/lib/templates/md/_accessor_getter.md
@@ -1,6 +1,7 @@
 {{#getter}}
-{{{ modelType.returnType.linkedName }}} {{>name_summary}}  {{!two spaces intentional}}
-{{>features}}
+{{ >annotations }}
+{{{ modelType.returnType.linkedName }}} {{ >name_summary }}  {{ !two spaces intentional }}
+{{ >features }}
 
 {{>documentation}}
 
diff --git a/lib/templates/md/_accessor_setter.md b/lib/templates/md/_accessor_setter.md
@@ -1,6 +1,7 @@
 {{#setter}}
-{{>name_summary}}({{{ linkedParamsNoMetadata }}})  {{!two spaces intentional}}
-{{>features}}
+{{ >annotations }}
+{{ >name_summary }}({{{ linkedParamsNoMetadata }}})  {{ !two spaces intentional }}
+{{ >features }}
 
 {{>documentation}}
 
diff --git a/lib/templates/md/function.md b/lib/templates/md/function.md
@@ -9,7 +9,8 @@
 {{/self}}
 
 {{#function}}
-{{>callable_multiline}}
+{{ >callable_multiline }}
+{{ >features }}
 
 {{>documentation}}
 
diff --git a/lib/templates/md/library.md b/lib/templates/md/library.md
@@ -1,6 +1,7 @@
 {{>head}}
 
 {{#self}}
+{{ !-- TODO(srawlins): Add annotations. }}
 # {{{ name }}} {{ kind }}
 
 {{>source_link}}
diff --git a/lib/templates/md/property.md b/lib/templates/md/property.md
@@ -9,8 +9,9 @@
 
 {{#self}}
 {{#hasNoGetterSetter}}
-{{{ modelType.linkedName }}} {{>name_summary}}  {{!two spaces intentional}}
-{{>features}}
+{{ >annotations }}
+{{{ modelType.linkedName }}} {{ >name_summary }}  {{ !two spaces intentional }}
+{{ >features }}
 
 {{>documentation}}
 
diff --git a/lib/templates/md/top_level_property.md b/lib/templates/md/top_level_property.md
@@ -8,8 +8,9 @@
 {{>feature_set}}
 
 {{#hasNoGetterSetter}}
-{{{ modelType.linkedName }}} {{>name_summary}}  {{!two spaces intentional}}
-{{>features}}
+{{ >annotations }}
+{{{ modelType.linkedName }}} {{ >name_summary }}  {{ !two spaces intentional }}
+{{ >features }}
 
 {{>documentation}}
 
diff --git a/test/end2end/model_test.dart b/test/end2end/model_test.dart
@@ -190,23 +190,6 @@ void main() {
           mn.parameters
               .map((p) => p.annotations.first.linkedNameWithParameters),
           everyElement(equals(ab0)));
-
-      expect(genericMetadata.features.map((f) => f.linkedNameWithParameters),
-          contains(ab0));
-      expect(f.features.map((f) => f.linkedNameWithParameters), contains(ab0));
-      expect(C.features.map((f) => f.linkedNameWithParameters), contains(ab0));
-      expect(
-          C.typeParameters.first.features
-              .map((f) => f.linkedNameWithParameters),
-          contains(ab0));
-      expect(
-          mp.parameters
-              .map((p) => p.features.map((f) => f.linkedNameWithParameters)),
-          everyElement(contains(ab0)));
-      expect(
-          mn.parameters
-              .map((p) => p.features.map((f) => f.linkedNameWithParameters)),
-          everyElement(contains(ab0)));
     });
   });
 
@@ -3749,13 +3732,6 @@ String? topLevelFunction(int param1, bool param2, Cool coolBeans,
               '```'));
     });
 
-    test('annotations from getters and setters are accumulated in Fields', () {
-      expect(explicitGetterSetter.featuresAsString,
-          contains('a Getter Annotation'));
-      expect(explicitGetterSetter.featuresAsString,
-          contains('a Setter Annotation'));
-    });
-
     test('Docs from inherited implicit accessors are preserved', () {
       expect(
           explicitGetterImplicitSetter.setter!.documentationComment, isNot(''));
diff --git a/test/src/utils.dart b/test/src/utils.dart
@@ -360,6 +360,22 @@ extension ModelElementIterableExtensions<T extends ModelElement>
   T named(String name) => firstWhere((e) => e.name == name);
 }
 
+extension IterableStringExtensions on Iterable<String> {
+  /// The main content line of `this`.
+  Iterable<String> get mainContent =>
+      skipWhile((line) => !line.contains('"dartdoc-main-content"'))
+          .takeWhile((line) => !line.contains('/.main-content'));
+
+  /// Verifies that [mainContent] contains [matchers] in order.
+  void expectMainContentContainsAllInOrder(Iterable<Object?> matchers) {
+    expect(
+      mainContent,
+      containsAllInOrder(matchers),
+      reason: 'main content:\n\n${mainContent.join('\n')}',
+    );
+  }
+}
+
 /// Extension methods just for tests.
 extension on ResourceProvider {
   Future<void> writeDartdocResource(String path, String content) async {
diff --git a/test/templates/category_test.dart b/test/templates/category_test.dart
@@ -131,6 +131,7 @@ typedef T2 = T1;
   });
 
   test('category page links to classes annotated with category', () async {
+    // TODO(srawlins): Use expectMainContentContainsAllInOrder throughout.
     expect(
         topicOneLines,
         containsAllInOrder([
diff --git a/test/templates/enum_test.dart b/test/templates/enum_test.dart
diff --git a/test/templates/extension_test.dart b/test/templates/extension_test.dart
diff --git a/test/templates/field_test.dart b/test/templates/field_test.dart
diff --git a/test/templates/method_test.dart b/test/templates/method_test.dart
