documentationComment can now be non-nullable (#2819)

* Straight move

* Partial

* delete experiment

* rebuild

* moved computeDocumentationComment and documentationComment

* Move more documentation comment handling

* remove ??= from a cut and paste

* Documentation comment move for combos

* better as a getter

* Untangle synthetic combo documentation in preparing for non-null

* rebuild

* documentationComment can be made non-nullable

* Update test.yaml to delete sdk check from beta branch

* tab conversion error

* correct name

Author: jcollins-g

.github/workflows/test.yaml

@@ -36,6 +36,9 @@ jobs:
             job: flutter
           - sdk: beta
             job: sdk-analyzer
+            # TODO(jcollins-g): Delete exception as 2.15 beta 2 gets underway
+          - sdk: beta
+            job: sdk-docs
 
     steps:
       - name: Store date

lib/src/generator/templates.runtime_renderers.dart

@@ -165,6 +165,13 @@ class _Renderer_Accessor extends RendererBase<Accessor> {
                         parent: r);
                   },
                 ),
+                'hasDocumentationComment': Property(
+                  getValue: (CT_ c) => c.hasDocumentationComment,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasDocumentationComment == true,
+                ),
                 'href': Property(
                   getValue: (CT_ c) => c.href,
                   renderVariable:
@@ -4018,6 +4025,13 @@ class _Renderer_DocumentationComment
                         parent: r);
                   },
                 ),
+                'hasDocumentationComment': Property(
+                  getValue: (CT_ c) => c.hasDocumentationComment,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasDocumentationComment == true,
+                ),
                 'hasNodoc': Property(
                   getValue: (CT_ c) => c.hasNodoc,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -6003,6 +6017,13 @@ class _Renderer_GetterSetterCombo extends RendererBase<GetterSetterCombo> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasAccessorsWithDocs == true,
                 ),
+                'hasDocumentationComment': Property(
+                  getValue: (CT_ c) => c.hasDocumentationComment,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasDocumentationComment == true,
+                ),
                 'hasExplicitGetter': Property(
                   getValue: (CT_ c) => c.hasExplicitGetter,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -9720,6 +9741,13 @@ class _Renderer_ModelElement extends RendererBase<ModelElement> {
                       self.renderSimpleVariable(c, remainingNames, 'bool'),
                   getBool: (CT_ c) => c.hasDocumentation == true,
                 ),
+                'hasDocumentationComment': Property(
+                  getValue: (CT_ c) => c.hasDocumentationComment,
+                  renderVariable: (CT_ c, Property<CT_> self,
+                          List<String> remainingNames) =>
+                      self.renderSimpleVariable(c, remainingNames, 'bool'),
+                  getBool: (CT_ c) => c.hasDocumentationComment == true,
+                ),
                 'hasExtendedDocumentation': Property(
                   getValue: (CT_ c) => c.hasExtendedDocumentation,
                   renderVariable: (CT_ c, Property<CT_> self,
@@ -15224,6 +15252,7 @@ const _invisibleGetters = {
     'documentationAsHtml',
     'elementDocumentation',
     'documentationComment',
+    'hasDocumentationComment',
     'hasNodoc',
     'sourceFileName',
     'fullyQualifiedNameWithoutLibrary',
@@ -15441,6 +15470,7 @@ const _invisibleGetters = {
     'getterSetterBothAvailable',
     'oneLineDoc',
     'documentationComment',
+    'hasDocumentationComment',
     'modelType',
     'isCallable',
     'hasParameters',

lib/src/model/accessor.dart

@@ -80,22 +80,49 @@ class Accessor extends ModelElement implements EnclosedElement {
       : _documentationComment ??= () {
           _documentationCommentComputed = true;
           if (isSynthetic) {
-            // If we're a setter, only display something if we have something different than the getter.
-            // TODO(jcollins-g): modify analyzer to do this itself?
-            if (isGetter ||
-                definingCombo.hasNodoc ||
-                (isSetter &&
-                    definingCombo.hasGetter &&
-                    definingCombo.getter.documentationComment !=
-                        definingCombo.documentationComment)) {
-              return stripComments(definingCombo.documentationComment);
-            } else {
-              return '';
-            }
+            return _syntheticDocumentationComment;
           }
           return stripComments(super.documentationComment);
         }();
 
+  String /*!*/ __syntheticDocumentationComment;
+
+  /// Build a documentation comment for this accessor assuming it is synthetic.
+  /// Value here is not useful if [isSynthetic] is false.
+  String /*!*/ get _syntheticDocumentationComment =>
+      __syntheticDocumentationComment ??= () {
+        if (_hasSyntheticDocumentationComment) {
+          return definingCombo.documentationComment ?? '';
+        }
+        return '';
+      }();
+
+  /// If this is a getter, assume we want synthetic documentation.
+  /// If the definingCombo has a nodoc tag, we want synthetic documentation
+  /// for a synthetic accessor just in case it is inherited somewhere
+  /// down the line due to split inheritance.
+  bool get _hasSyntheticDocumentationComment =>
+      (isGetter || definingCombo.hasNodoc || _comboDocsAreIndependent()) &&
+      definingCombo.hasDocumentationComment;
+
+  // If we're a setter, and a getter exists, do not add synthetic
+  // documentation if the combo's documentation is actually derived
+  // from that getter.
+  bool _comboDocsAreIndependent() {
+    if (isSetter && definingCombo.hasGetter) {
+      if (definingCombo.getter.isSynthetic ||
+          !definingCombo.documentationFrom.contains(this)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  @override
+  bool get hasDocumentationComment => isSynthetic
+      ? _hasSyntheticDocumentationComment
+      : element.documentationComment != null;
+
   @override
   void warn(PackageWarning kind,
       {String message,

lib/src/model/documentation_comment.dart

@@ -52,7 +52,7 @@ mixin DocumentationComment
   @override
   List<DocumentationComment> get documentationFrom =>
       _documentationFrom ??= () {
-        if (documentationComment == null &&
+        if (!hasDocumentationComment &&
             this is Inheritable &&
             (this as Inheritable).overriddenElement != null) {
           return (this as Inheritable).overriddenElement.documentationFrom;
@@ -81,11 +81,15 @@ mixin DocumentationComment
     return _elementDocumentation;
   }
 
-  String get documentationComment;
+  String /*!*/ get documentationComment;
+
+  /// True if [this] has a synthetic/inherited or local documentation
+  /// comment.  False otherwise.
+  bool get hasDocumentationComment;
 
   /// Returns true if the raw documentation comment has a nodoc indication.
   bool get hasNodoc {
-    if (documentationComment != null &&
+    if (hasDocumentationComment &&
         (documentationComment.contains('@nodoc') ||
             documentationComment.contains('<nodoc>'))) {
       return true;

lib/src/model/getter_setter_combo.dart

@@ -161,44 +161,53 @@ mixin GetterSetterCombo on ModelElement {
   bool _documentationCommentComputed = false;
   String _documentationComment;
   @override
-  String get documentationComment => _documentationCommentComputed
+  String /*!*/ get documentationComment => _documentationCommentComputed
       ? _documentationComment
       : _documentationComment ??= () {
           _documentationCommentComputed = true;
           var docs = _getterSetterDocumentationComment;
-          if (docs.isEmpty) return element.documentationComment;
+          if (docs.isEmpty) return element.documentationComment ?? '';
           return docs;
         }();
 
-  /// Derive a synthetic documentation comment using the documentation from
-  String get _getterSetterDocumentationComment {
-    var buffer = StringBuffer();
-
-    // Check for synthetic before public, always, or stack overflow.
-    if (hasGetter && !getter.isSynthetic && getter.isPublic) {
-      assert(getter.documentationFrom.length == 1);
-      // We have to check against dropTextFrom here since documentationFrom
-      // doesn't yield the real elements for GetterSetterCombos.
-      if (!config.dropTextFrom
-          .contains(getter.documentationFrom.first.element.library.name)) {
-        var docs = getter.documentationFrom.first.documentationComment;
-        if (docs != null) buffer.write(docs);
-      }
-    }
+  @override
+  bool get hasDocumentationComment =>
+      _getterSetterDocumentationComment.isNotEmpty ||
+      element.documentationComment != null;
+
+  String __getterSetterDocumentationComment;
+
+  /// Derive a documentation comment for the combo by copying documentation
+  /// from the [getter] and/or [setter].
+  String /*!*/ get _getterSetterDocumentationComment =>
+      __getterSetterDocumentationComment ??= () {
+        var buffer = StringBuffer();
 
-    if (hasSetter && !setter.isSynthetic && setter.isPublic) {
-      assert(setter.documentationFrom.length == 1);
-      if (!config.dropTextFrom
-          .contains(setter.documentationFrom.first.element.library.name)) {
-        var docs = setter.documentationFrom.first.documentationComment;
-        if (docs != null) {
-          if (buffer.isNotEmpty) buffer.write('\n\n');
-          buffer.write(docs);
+        // Check for synthetic before public, always, or stack overflow.
+        if (hasGetter && !getter.isSynthetic && getter.isPublic) {
+          assert(getter.documentationFrom.length == 1);
+          var fromGetter = getter.documentationFrom.first;
+          // We have to check against dropTextFrom here since documentationFrom
+          // doesn't yield the real elements for GetterSetterCombos.
+          if (!config.dropTextFrom.contains(fromGetter.element.library.name)) {
+            if (fromGetter.hasDocumentationComment) {
+              buffer.write(fromGetter.documentationComment);
+            }
+          }
         }
-      }
-    }
-    return buffer.toString();
-  }
+
+        if (hasSetter && !setter.isSynthetic && setter.isPublic) {
+          assert(setter.documentationFrom.length == 1);
+          var fromSetter = setter.documentationFrom.first;
+          if (!config.dropTextFrom.contains(fromSetter.element.library.name)) {
+            if (fromSetter.hasDocumentationComment) {
+              if (buffer.isNotEmpty) buffer.write('\n\n');
+              buffer.write(fromSetter.documentationComment);
+            }
+          }
+        }
+        return buffer.toString();
+      }();
 
   ElementType get modelType {
     if (hasGetter) return getter.modelType.returnType;

lib/src/model/model_element.dart

@@ -917,7 +917,10 @@ abstract class ModelElement extends Canonicalization
   }
 
   @override
-  String get documentationComment => element.documentationComment;
+  String /*!*/ get documentationComment => element.documentationComment ?? '';
+
+  @override
+  bool get hasDocumentationComment => element.documentationComment != null;
 
   String _sourceCode;
   @override

lib/src/model/package_graph.dart

@@ -123,7 +123,7 @@ class PackageGraph with CommentReferable, Nameable {
 
     Iterable<Future<void>> precacheOneElement(ModelElement m) sync* {
       for (var d
-          in m.documentationFrom.where((d) => d.documentationComment != null)) {
+          in m.documentationFrom.where((d) => d.hasDocumentationComment)) {
         if (d.needsPrecache && !precachedElements.contains(d)) {
           precachedElements.add(d);
           yield d.precacheLocalDocs();

test/end2end/model_test.dart

@@ -630,7 +630,7 @@ void main() {
         () {
       // Verify setup of the test is correct.
       expect(invokeToolParentDoc.isCanonical, isTrue);
-      expect(invokeToolParentDoc.documentationComment, isNull);
+      expect(invokeToolParentDoc.hasDocumentationComment, isFalse);
       // Error message here might look strange due to toString() on Methods, but if this
       // fails that means we don't have the correct invokeToolParentDoc instance.
       expect(invokeToolParentDoc.documentationFrom,