Repository: dart-lang/dartdoc (dart)

Issue:
enum documentation has weird constructor-like calls interspersed
https://master-api.flutter.dev/flutter/dart-ui/BoxHeightStyle.html
...has a few lines that just say `BoxHeightStyle()` spread throughout the docs in a way that doesn't have any context.

Pull request:
Fixes https://github.com/dart-lang/dartdoc/issues/3201

We used to print text like `const BoxHeightStyle(3)` for the 4th enum value of BoxHeightStyle. But this is totally pseudo-code, and misleading. The user cannot write this expression, nor is it the actual expression used to construct the enum value.

So  we guard whether we should display a constant implementation String behind a new `hasConstantValueForDisplay` bool getter.

Patch:
diff --git a/lib/src/generator/templates.aot_renderers_for_html.dart b/lib/src/generator/templates.aot_renderers_for_html.dart
index 3b37629d63..53d386ed32 100644
--- a/lib/src/generator/templates.aot_renderers_for_html.dart
+++ b/lib/src/generator/templates.aot_renderers_for_html.dart
@@ -3375,13 +3375,17 @@ String _deduplicated_lib_templates_html__constant_html(
   buffer.write(
       __deduplicated_lib_templates_html__constant_html_partial_features_1(
           context0));
+  if (context0.hasConstantValueForDisplay == true) {
+    buffer.writeln();
+    buffer.write('''
+    <div>
+      <span class="signature"><code>''');
+    buffer.write(context0.constantValueTruncated);
+    buffer.write('''</code></span>
+    </div>''');
+  }
   buffer.writeln();
   buffer.write('''
-  <div>
-    <span class="signature"><code>''');
-  buffer.write(context0.constantValueTruncated);
-  buffer.write('''</code></span>
-  </div>
 </dd>
 ''');
 
@@ -4573,13 +4577,17 @@ String
   buffer.write(
       ___deduplicated_lib_templates_html__static_constants_html_partial_constant_0_partial_features_1(
           context1));
+  if (context1.hasConstantValueForDisplay == true) {
+    buffer.writeln();
+    buffer.write('''
+    <div>
+      <span class="signature"><code>''');
+    buffer.write(context1.constantValueTruncated);
+    buffer.write('''</code></span>
+    </div>''');
+  }
   buffer.writeln();
   buffer.write('''
-  <div>
-    <span class="signature"><code>''');
-  buffer.write(context1.constantValueTruncated);
-  buffer.write('''</code></span>
-  </div>
 </dd>
 ''');
 
diff --git a/lib/src/generator/templates.runtime_renderers.dart b/lib/src/generator/templates.runtime_renderers.dart
index cb1e2e5c28..936b4f90ab 100644
--- a/lib/src/generator/templates.runtime_renderers.dart
+++ b/lib/src/generator/templates.runtime_renderers.dart
@@ -5378,6 +5378,19 @@ class _Renderer_Field extends RendererBase<Field> {
                         parent: r);
                   },
                 ),
+                'element': Property(
+                  getValue: (CT_ c) => c.element,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(
+                          c, remainingNames, 'FieldElement'),
+                  isNullValue: (CT_ c) => false,
+                  renderValue: (CT_ c, RendererBase<CT_> r,
+                      List<MustachioNode> ast, StringSink sink) {
+                    renderSimple(c.element, ast, r.template, sink,
+                        parent: r, getters: _invisibleGetters['FieldElement']!);
+                  },
+                ),
                 'enclosingElement': Property(
                   getValue: (CT_ c) => c.enclosingElement,
                   renderVariable:
@@ -6188,6 +6201,13 @@ class _Renderer_GetterSetterCombo extends RendererBase<GetterSetterCombo> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasAccessorsWithDocs == true,
                 ),
+                'hasConstantValueForDisplay': Property(
+                  getValue: (CT_ c) => c.hasConstantValueForDisplay,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasConstantValueForDisplay == true,
+                ),
                 'hasDocumentationComment': Property(
                   getValue: (CT_ c) => c.hasDocumentationComment,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -16029,6 +16049,7 @@ const _invisibleGetters = {
     'getter',
     'getterSetterBothAvailable',
     'hasAccessorsWithDocs',
+    'hasConstantValueForDisplay',
     'hasDocumentationComment',
     'hasExplicitGetter',
     'hasExplicitSetter',
diff --git a/lib/src/model/enum.dart b/lib/src/model/enum.dart
index a4659544e8..cd01b02a63 100644
--- a/lib/src/model/enum.dart
+++ b/lib/src/model/enum.dart
@@ -53,17 +53,28 @@ class Enum extends InheritingContainer
 class EnumField extends Field {
   final int index;
 
-  @override
-  bool get isEnumValue => true;
-
   EnumField.forConstant(this.index, FieldElement element, Library library,
       PackageGraph packageGraph, Accessor? getter)
       : super(
             element, library, packageGraph, getter as ContainerAccessor?, null);
 
+  @override
+  bool get isEnumValue => true;
+
+  @override
+  bool get hasConstantValueForDisplay {
+    final enum_ = element.enclosingElement3 as EnumElement;
+    final enumHasDefaultConstructor =
+        enum_.constructors.any((c) => c.isDefaultConstructor);
+    // If this enum does not have any explicit constructors (and so only has a
+    // default constructor), then there is no meaningful constant initializer to
+    // display.
+    return !enumHasDefaultConstructor;
+  }
+
   @override
   String get constantValueBase =>
-      element.library!.featureSet.isEnabled(Feature.enhanced_enums)
+      element.library.featureSet.isEnabled(Feature.enhanced_enums)
           ? super.constantValueBase
           : _fieldRenderer.renderValue(this);
 
diff --git a/lib/src/model/field.dart b/lib/src/model/field.dart
index cb5a8068e6..d37fd1ffb1 100644
--- a/lib/src/model/field.dart
+++ b/lib/src/model/field.dart
@@ -41,6 +41,9 @@ class Field extends ModelElement
     return newField;
   }
 
+  @override
+  FieldElement get element => super.element as FieldElement;
+
   @override
   String get documentation {
     if (enclosingElement is Enum) {
@@ -64,7 +67,7 @@ class Field extends ModelElement
   @override
   Container get enclosingElement => isInherited
       ? _enclosingContainer
-      : modelBuilder.from(field!.enclosingElement3, library) as Container;
+      : modelBuilder.from(element.enclosingElement3, library) as Container;
 
   @override
   String get filePath =>
@@ -78,23 +81,25 @@ class Field extends ModelElement
   }
 
   @override
-  bool get isConst => field!.isConst;
+  bool get isConst => element.isConst;
 
   /// Returns true if the FieldElement is covariant, or if the first parameter
   /// for the setter is covariant.
   @override
-  bool get isCovariant => setter?.isCovariant == true || field!.isCovariant;
+  bool get isCovariant => setter?.isCovariant == true || element.isCovariant;
 
+  /// Whether this field is final.
+  ///
+  /// `Element.isFinal` returns `true` even if it has an explicit getter (which
+  /// means we should not document it as "final").
   @override
   bool get isFinal {
-    /// isFinal returns true for the field even if it has an explicit getter
-    /// (which means we should not document it as "final").
     if (hasExplicitGetter) return false;
-    return field!.isFinal;
+    return element.isFinal;
   }
 
   @override
-  bool get isLate => isFinal && field!.isLate;
+  bool get isLate => isFinal && element.isLate;
 
   @override
   bool get isInherited => _isInherited;
@@ -102,10 +107,7 @@ class Field extends ModelElement
   @override
   String get kind => isConst ? 'constant' : 'property';
 
-  String get fullkind {
-    if (field!.isAbstract) return 'abstract $kind';
-    return kind;
-  }
+  String get fullkind => element.isAbstract ? 'abstract $kind' : kind;
 
   @override
   Set<Feature> get features {
@@ -135,7 +137,8 @@ class Field extends ModelElement
     return allFeatures;
   }
 
-  FieldElement? get field => element as FieldElement?;
+  @Deprecated('Use `element`')
+  FieldElement? get field => element;
 
   @override
   String get fileName => '${isConst ? '$name-constant' : name}.$fileType';
diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart
index ee206aae5a..253443a50e 100644
--- a/lib/src/model/getter_setter_combo.dart
+++ b/lib/src/model/getter_setter_combo.dart
@@ -57,6 +57,13 @@ mixin GetterSetterCombo on ModelElement {
 
   bool get isInherited;
 
+  /// Whether this has a constant value which should be displayed.
+  bool get hasConstantValueForDisplay {
+    final element = this.element;
+    if (element is! ConstVariableElement) return false;
+    return element.constantInitializer != null;
+  }
+
   Expression? get constantInitializer =>
       (element as ConstVariableElement).constantInitializer;
 
diff --git a/lib/templates/html/_constant.html b/lib/templates/html/_constant.html
index afb0df726b..72c1e0eb77 100644
--- a/lib/templates/html/_constant.html
+++ b/lib/templates/html/_constant.html
@@ -11,7 +11,9 @@
 <dd>
   {{{ oneLineDoc }}}
   {{ >features }}
-  <div>
-    <span class="signature"><code>{{{ constantValueTruncated }}}</code></span>
-  </div>
+  {{ #hasConstantValueForDisplay }}
+    <div>
+      <span class="signature"><code>{{{ constantValueTruncated }}}</code></span>
+    </div>
+  {{ /hasConstantValueForDisplay }}
 </dd>


Test patch:
diff --git a/test/templates/enum_test.dart b/test/templates/enum_test.dart
index 9c53ded856..9fe2aec39d 100644
--- a/test/templates/enum_test.dart
+++ b/test/templates/enum_test.dart
@@ -126,7 +126,16 @@ enum E<T> with M<T> implements C<T> {
   static void set gs1(int value) {}
 }
 
-enum EnumWithDefaultConstructor { four, five, six }
+enum EnumWithDefaultConstructor {
+  /// Doc comment for [four].
+  four,
+
+  /// Doc comment for [five].
+  five,
+
+  /// Doc comment for [six].
+  six;
+}
 '''),
         ],
         resourceProvider: resourceProvider,
@@ -178,14 +187,32 @@ enum EnumWithDefaultConstructor { four, five, six }
 
     test('enum page contains values', () async {
       expect(
-          eLines,
-          containsAllInOrder([
-            matches('<h2>Values</h2>'),
-            matches('<span class="name ">one</span>'),
-            matches('<p>Doc comment for <a href="../lib/E.html">one</a>.</p>'),
-            matches(
-                r'<span class="signature"><code>E&lt;int&gt;.named\(1\)</code></span>'),
-          ]));
+        eLines,
+        containsAllInOrder([
+          matches('<h2>Values</h2>'),
+          matches('<span class="name ">one</span>'),
+          matches('<p>Doc comment for <a href="../lib/E.html">one</a>.</p>'),
+          matches(
+              r'<span class="signature"><code>E&lt;int&gt;.named\(1\)</code></span>'),
+        ]),
+      );
+    });
+
+    test('enum page contains classic values', () async {
+      expect(
+        enumWithDefaultConstructorLines,
+        containsAllInOrder([
+          matches('<h2>Values</h2>'),
+          matches('<span class="name ">four</span>'),
+          matches(
+              '<p>Doc comment for <a href="../lib/EnumWithDefaultConstructor.html">six</a>.</p>'),
+        ]),
+      );
+      expect(
+        enumWithDefaultConstructorLines.join('\n'),
+        isNot(contains(
+            '<span class="signature"><code>EnumWithDefaultConstructor')),
+      );
     });
 
     test('enum page contains values with deprecated annotation', () async {
@@ -419,6 +446,5 @@ enum EnumWithDefaultConstructor { four, five, six }
 
     // TODO(srawlins): Add rendering tests.
     // * Add tests for rendered supertype (Enum) HTML.
-    // * Add tests for rendered field pages.
   });
 }