Extract nodoc into mixin and refactor (#2368)

jcollins-g · web-flow · commit f06af8bfd3bb · 2020-10-01T12:07:24.000-07:00
* Extract nodoc into mixin and refactor

* Rename files

* Fix ordering
diff --git a/lib/src/model/accessor.dart b/lib/src/model/accessor.dart
@@ -23,15 +23,29 @@ class Accessor extends ModelElement implements EnclosedElement {
 
   bool get isSynthetic => element.isSynthetic;
 
+  GetterSetterCombo _definingCombo;
+  // The [enclosingCombo] where this element was defined.
+  GetterSetterCombo get definingCombo {
+    if (_definingCombo == null) {
+      var variable = (element as PropertyAccessorElement).variable;
+      var accessor = isGetter ? variable.getter : variable.setter;
+      var accessorLibrary = Library(accessor.library, packageGraph);
+      var definingAccessor =
+          ModelElement.from(accessor, accessorLibrary, packageGraph)
+              as Accessor;
+      _definingCombo = definingAccessor.enclosingCombo;
+    }
+    return _definingCombo;
+  }
+
   String _sourceCode;
 
   @override
   String get sourceCode {
     if (_sourceCode == null) {
       if (isSynthetic) {
-        _sourceCode = packageGraph
-            .getModelNodeFor((element as PropertyAccessorElement).variable)
-            .sourceCode;
+        _sourceCode =
+            packageGraph.getModelNodeFor(definingCombo.element).sourceCode;
       } else {
         _sourceCode = super.sourceCode;
       }
@@ -42,19 +56,15 @@ class Accessor extends ModelElement implements EnclosedElement {
   @override
   String computeDocumentationComment() {
     if (isSynthetic) {
-      var docComment =
-          (element as PropertyAccessorElement).variable.documentationComment;
       // 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 ||
-          // TODO(jcollins-g): @nodoc reading from comments is at the wrong abstraction level here.
-          (docComment != null &&
-              (docComment.contains('<nodoc>') ||
-                  docComment.contains('@nodoc'))) ||
+          definingCombo.hasNodoc ||
           (isSetter &&
-              enclosingCombo.hasGetter &&
-              enclosingCombo.getter.documentationComment != docComment)) {
-        return stripComments(docComment);
+              definingCombo.hasGetter &&
+              definingCombo.getter.documentationComment !=
+                  definingCombo.documentationComment)) {
+        return stripComments(definingCombo.documentationComment);
       } else {
         return '';
       }
diff --git a/lib/src/model/documentation_comment.dart b/lib/src/model/documentation_comment.dart
@@ -4,7 +4,6 @@ import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/render/model_element_renderer.dart';
 import 'package:dartdoc/src/utils.dart';
 import 'package:dartdoc/src/warnings.dart';
-import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 
 final _templatePattern = RegExp(
@@ -27,8 +26,33 @@ final _examplePattern = RegExp(r'{@example\s+([^}]+)}');
 ///
 /// [processCommentWithoutTools] and [processComment] are the primary
 /// entrypoints.
-mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
-  /// Process [documentationComment], performing various actions based on
+mixin DocumentationComment
+    on Documentable, Warnable, Locatable, SourceCodeMixin {
+  /// The documentation comment on the Element may be null, so memoization
+  /// cannot rely on the null-ness of [_documentationComment], it must be
+  /// more explicit.
+  bool _documentationCommentComputed = false;
+  String _documentationComment;
+
+  String get documentationComment {
+    if (_documentationCommentComputed == false) {
+      _documentationComment = computeDocumentationComment();
+      _documentationCommentComputed = true;
+    }
+    return _documentationComment;
+  }
+
+  /// Implement to derive the raw documentation comment string from the
+  /// analyzer.
+  String computeDocumentationComment();
+
+  /// Returns true if the raw documentation comment has a nodoc indication.
+  bool get hasNodoc =>
+      documentationComment != null &&
+      (documentationComment.contains('@nodoc') ||
+          documentationComment.contains('<nodoc>'));
+
+  /// Process a [documentationComment], performing various actions based on
   /// `{@}`-style directives, except `{@tool}`, returning the processed result.
   String processCommentWithoutTools(String documentationComment) {
     var docs = stripComments(documentationComment);
@@ -68,17 +92,13 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
     return docs;
   }
 
-  String get _sourceFileName => element.source.fullName;
+  String get sourceFileName;
 
-  String get _fullyQualifiedNameWithoutLibrary =>
-      // Remember, periods are legal in library names.
-      fullyQualifiedName.replaceFirst('${library.fullyQualifiedName}.', '');
+  String get fullyQualifiedNameWithoutLibrary;
 
-  path.Context get pathContext => packageGraph.resourceProvider.pathContext;
+  path.Context get pathContext;
 
-  @visibleForTesting
-  ModelElementRenderer get modelElementRenderer =>
-      packageGraph.rendererFactory.modelElementRenderer;
+  ModelElementRenderer get modelElementRenderer;
 
   static const _allDirectiveNames = [
     'animation',
@@ -209,13 +229,13 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
             'SOURCE_LINE': characterLocation?.lineNumber.toString(),
             'SOURCE_COLUMN': characterLocation?.columnNumber.toString(),
             'SOURCE_PATH':
-                (_sourceFileName == null || package?.packagePath == null)
+                (sourceFileName == null || package?.packagePath == null)
                     ? null
-                    : path.relative(_sourceFileName, from: package.packagePath),
+                    : path.relative(sourceFileName, from: package.packagePath),
             'PACKAGE_PATH': package?.packagePath,
             'PACKAGE_NAME': package?.name,
             'LIBRARY_NAME': library?.fullyQualifiedName,
-            'ELEMENT_NAME': _fullyQualifiedNameWithoutLibrary,
+            'ELEMENT_NAME': fullyQualifiedNameWithoutLibrary,
             'INVOCATION_INDEX': invocationIndex.toString(),
             'PACKAGE_INVOCATION_INDEX':
                 (package.toolInvocationIndex++).toString(),
diff --git a/lib/src/model/getter_setter_combo.dart b/lib/src/model/getter_setter_combo.dart
@@ -156,7 +156,8 @@ mixin GetterSetterCombo on ModelElement {
   String get getterSetterDocumentationComment {
     var buffer = StringBuffer();
 
-    if (hasPublicGetter && !getter.isSynthetic) {
+    // 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.
@@ -167,7 +168,7 @@ mixin GetterSetterCombo on ModelElement {
       }
     }
 
-    if (hasPublicSetter && !setter.isSynthetic) {
+    if (hasSetter && !setter.isSynthetic && setter.isPublic) {
       assert(setter.documentationFrom.length == 1);
       if (!config.dropTextFrom
           .contains(setter.documentationFrom.first.element.library.name)) {
diff --git a/lib/src/model/model.dart b/lib/src/model/model.dart
@@ -7,12 +7,12 @@ export 'canonicalization.dart';
 export 'categorization.dart';
 export 'category.dart';
 export 'class.dart';
-export 'comment_processable.dart';
 export 'constructor.dart';
 export 'container.dart';
 export 'container_member.dart';
 export 'documentable.dart';
 export 'documentation.dart';
+export 'documentation_comment.dart';
 export 'dynamic.dart';
 export 'enclosed_element.dart';
 export 'enum.dart';
diff --git a/lib/src/model/model_element.dart b/lib/src/model/model_element.dart
@@ -16,7 +16,7 @@ import 'package:analyzer/src/dart/element/member.dart'
 import 'package:collection/collection.dart';
 import 'package:dartdoc/src/dartdoc_options.dart';
 import 'package:dartdoc/src/element_type.dart';
-import 'package:dartdoc/src/model/comment_processable.dart';
+import 'package:dartdoc/src/model/documentation_comment.dart';
 import 'package:dartdoc/src/model/feature_set.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/model_utils.dart' as utils;
@@ -135,7 +135,7 @@ abstract class ModelElement extends Canonicalization
         SourceCodeMixin,
         Indexable,
         FeatureSet,
-        CommentProcessable
+        DocumentationComment
     implements Comparable<ModelElement>, Documentable {
   final Element _element;
 
@@ -472,13 +472,10 @@ abstract class ModelElement extends Canonicalization
           !(enclosingElement as Extension).isPublic) {
         _isPublic = false;
       } else {
-        var docComment = documentationComment;
-        if (docComment == null) {
+        if (documentationComment == null) {
           _isPublic = utils.hasPublicName(element);
         } else {
-          _isPublic = utils.hasPublicName(element) &&
-              !(docComment.contains('@nodoc') ||
-                  docComment.contains('<nodoc>'));
+          _isPublic = utils.hasPublicName(element) && !hasNodoc;
         }
       }
     }
@@ -821,7 +818,7 @@ abstract class ModelElement extends Canonicalization
   /// does not exist.
   String get extendedDocLink {
     if (hasExtendedDocumentation) {
-      return _modelElementRenderer.renderExtendedDocLink(this);
+      return modelElementRenderer.renderExtendedDocLink(this);
     }
     return '';
   }
@@ -843,13 +840,15 @@ abstract class ModelElement extends Canonicalization
   }
 
   String _fullyQualifiedNameWithoutLibrary;
+  @override
   String get fullyQualifiedNameWithoutLibrary {
     // Remember, periods are legal in library names.
     _fullyQualifiedNameWithoutLibrary ??=
         fullyQualifiedName.replaceFirst('${library.fullyQualifiedName}.', '');
     return _fullyQualifiedNameWithoutLibrary;
   }
 
+  @override
   String get sourceFileName => element.source.fullName;
 
   CharacterLocation _characterLocation;
@@ -956,7 +955,9 @@ abstract class ModelElement extends Canonicalization
     return _linkedName;
   }
 
-  ModelElementRenderer get _modelElementRenderer =>
+  @visibleForTesting
+  @override
+  ModelElementRenderer get modelElementRenderer =>
       packageGraph.rendererFactory.modelElementRenderer;
 
   ParameterRenderer get _parameterRenderer =>
@@ -1067,6 +1068,9 @@ abstract class ModelElement extends Canonicalization
     return _allParameters;
   }
 
+  @override
+  path.Context get pathContext => packageGraph.resourceProvider.pathContext;
+
   List<Parameter> get parameters {
     if (!canHaveParameters) {
       throw StateError('$element cannot have parameters');
@@ -1112,22 +1116,9 @@ abstract class ModelElement extends Canonicalization
         extendedDebug: extendedDebug);
   }
 
+  @override
   String computeDocumentationComment() => element.documentationComment;
 
-  /// The documentation comment on the Element may be null, so memoization
-  /// cannot rely on the null-ness of [_documentationComment], it must be
-  /// more explicit.
-  bool _documentationCommentComputed = false;
-  String _documentationComment;
-
-  String get documentationComment {
-    if (_documentationCommentComputed == false) {
-      _documentationComment = computeDocumentationComment();
-      _documentationCommentComputed = true;
-    }
-    return _documentationComment;
-  }
-
   /// Unconditionally precache local documentation.
   ///
   /// Use only in factory for [PackageGraph].
@@ -1191,7 +1182,7 @@ abstract class ModelElement extends Canonicalization
       return htmlEscape.convert(name);
     }
 
-    return _modelElementRenderer.renderLinkedName(this);
+    return modelElementRenderer.renderLinkedName(this);
   }
 
   /// Replace &lt;<dartdoc-html>[digest]</dartdoc-html>&gt; in API comments with
diff --git a/test/documentation_comment_test.dart b/test/documentation_comment_test.dart
