Cleanup some doc methods (#2510)

* Cleanup some doc methods

* Local doc

* Undo accidental match change

* Address feedback

* Fix typo - oops :>

Author: parlough

lib/src/generator/templates.renderers.dart

@@ -3,7 +3,7 @@
 // To change the contents of this library, make changes to the builder source
 // files in the tool/mustachio/ directory.
 
-// ignore_for_file: camel_case_types, unnecessary_cast, unused_element, unused_import, non_constant_identifier_names
+// ignore_for_file: camel_case_types, unnecessary_cast, unused_element, unused_import, non_constant_identifier_names, deprecated_member_use_from_same_package
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:dartdoc/dartdoc.dart';
 import 'package:dartdoc/src/generator/template_data.dart';

lib/src/model/documentable.dart

@@ -45,12 +45,6 @@ enum DocumentLocation {
 mixin MarkdownFileDocumentation implements Documentable, Canonicalization {
   DocumentLocation get documentedWhere;
 
-  @override
-  String get documentation => documentationFile == null
-      ? null
-      : packageGraph.resourceProvider
-          .readAsMalformedAllowedStringSync(documentationFile);
-
   Documentation __documentation;
 
   Documentation get _documentation {
@@ -63,11 +57,16 @@ mixin MarkdownFileDocumentation implements Documentable, Canonicalization {
   String get documentationAsHtml => _documentation.asHtml;
 
   @override
-  bool get hasDocumentation =>
-      documentationFile != null &&
-      packageGraph.resourceProvider
-          .readAsMalformedAllowedStringSync(documentationFile)
-          .isNotEmpty;
+  String get documentation {
+    final docFile = documentationFile;
+    return docFile == null
+        ? null
+        : packageGraph.resourceProvider
+            .readAsMalformedAllowedStringSync(docFile);
+  }
+
+  @override
+  bool get hasDocumentation => documentation?.isNotEmpty == true;
 
   @override
   bool get hasExtendedDocumentation =>

lib/src/model/documentation.dart

@@ -62,10 +62,10 @@ class Documentation {
   /// if [processFullDocs] is `true`. If more than one node is present,
   /// then [DocumentationParseResult.hasExtendedDocs] will be set to `true`.
   DocumentationParseResult _parseDocumentation(bool processFullDocs) {
-    if (!_element.hasDocumentation) {
+    final text = _element.documentation;
+    if (text == null || text.isEmpty) {
       return DocumentationParseResult.empty;
     }
-    var text = _element.documentation;
     showWarningsForGenericsOutsideSquareBracketsBlocks(text, _element);
     var document =
         MarkdownDocument.withElementLinkResolver(_element, commentRefs);

lib/src/model/getter_setter_combo.dart

@@ -120,17 +120,14 @@ mixin GetterSetterCombo on ModelElement {
     return _documentationFrom;
   }
 
-  bool get hasAccessorsWithDocs => (hasPublicGetter &&
-          !getter.isSynthetic &&
-          getter.documentation.isNotEmpty ||
-      hasPublicSetter &&
-          !setter.isSynthetic &&
-          setter.documentation.isNotEmpty);
+  bool get hasAccessorsWithDocs =>
+      (hasPublicGetter && !getter.isSynthetic && getter.hasDocumentation ||
+          hasPublicSetter && !setter.isSynthetic && setter.hasDocumentation);
 
   bool get getterSetterBothAvailable => (hasPublicGetter &&
-      getter.documentation.isNotEmpty &&
+      getter.hasDocumentation &&
       hasPublicSetter &&
-      setter.documentation.isNotEmpty);
+      setter.hasDocumentation);
 
   String _oneLineDoc;
 

lib/src/model/model_element.dart

@@ -877,8 +877,7 @@ abstract class ModelElement extends Canonicalization
   bool get hasAnnotations => annotations.isNotEmpty;
 
   @override
-  bool get hasDocumentation =>
-      documentation != null && documentation.isNotEmpty;
+  bool get hasDocumentation => documentation?.isNotEmpty == true;
 
   @override
   bool get hasExtendedDocumentation =>

lib/src/model/package.dart

@@ -118,29 +118,34 @@ class Package extends LibraryContainer
     return _documentationAsHtml;
   }
 
+  String /*?*/ _documentation;
+
   @override
   String get documentation {
-    return hasDocumentationFile
-        ? packageGraph.resourceProvider
-            .readAsMalformedAllowedStringSync(documentationFile)
-        : null;
+    if (_documentation == null) {
+      final docFile = documentationFile;
+      if (docFile != null) {
+        _documentation = packageGraph.resourceProvider
+            .readAsMalformedAllowedStringSync(docFile);
+      }
+    }
+    return _documentation;
   }
 
   @override
-  bool get hasDocumentation =>
-      documentationFile != null &&
-      packageGraph.resourceProvider
-          .readAsMalformedAllowedStringSync(documentationFile)
-          .isNotEmpty;
+  bool get hasDocumentation => documentation?.isNotEmpty == true;
 
   @override
-  bool get hasExtendedDocumentation => documentation.isNotEmpty;
+  bool get hasExtendedDocumentation => hasDocumentation;
+
+  File /*?*/ _documentationFile;
 
-  // TODO: Clients should use [documentationFile] so they can act differently on
-  // plain text or markdown.
+  @Deprecated(
+      'Instead use [documentationFile] which will be `null` if this package does not have one.')
   bool get hasDocumentationFile => documentationFile != null;
 
-  File get documentationFile => packageMeta.getReadmeContents();
+  File /*?*/ get documentationFile =>
+      _documentationFile ??= packageMeta.getReadmeContents();
 
   @override
   String get oneLineDoc => '';

test/end2end/dartdoc_test.dart

@@ -248,7 +248,9 @@ void main() {
         var packageGraph = results.packageGraph;
         var p = packageGraph.defaultPackage;
         expect(p.name, 'test_package');
+        // ignore: deprecated_member_use_from_same_package
         expect(p.hasDocumentationFile, isTrue);
+        expect(p.documentationFile, isNotNull);
         // Total number of public libraries in test_package.
         // +2 since we are not manually excluding anything.
         expect(packageGraph.defaultPackage.publicLibraries,
@@ -293,7 +295,9 @@ void main() {
 
       var p = results.packageGraph;
       expect(p.defaultPackage.name, 'sky_engine');
+      // ignore: deprecated_member_use_from_same_package
       expect(p.defaultPackage.hasDocumentationFile, isFalse);
+      expect(p.defaultPackage.documentationFile, isNull);
       expect(p.libraries, hasLength(3));
       expect(p.libraries.map((lib) => lib.name).contains('dart:core'), isTrue);
       expect(p.libraries.map((lib) => lib.name).contains('dart:async'), isTrue);

test/package_test.dart

@@ -146,15 +146,19 @@ int x;
         writeToJoinedPath(['README.md'], 'Readme text.');
         var packageGraph = await utils.bootBasicPackage(
             projectPath, packageMetaProvider, packageConfigProvider);
+        // ignore: deprecated_member_use_from_same_package
         expect(packageGraph.defaultPackage.hasDocumentationFile, true);
+        expect(packageGraph.defaultPackage.documentationFile, isNotNull);
         expect(packageGraph.defaultPackage.hasDocumentation, true);
       });
 
       test('has documentation via text README', () async {
         writeToJoinedPath(['README'], 'Readme text.');
         var packageGraph = await utils.bootBasicPackage(
             projectPath, packageMetaProvider, packageConfigProvider);
+        // ignore: deprecated_member_use_from_same_package
         expect(packageGraph.defaultPackage.hasDocumentationFile, true);
+        expect(packageGraph.defaultPackage.documentationFile, isNotNull);
         expect(packageGraph.defaultPackage.hasDocumentation, true);
       });
 
@@ -458,6 +462,7 @@ int x;
             projectPath, packageMetaProvider, packageConfigProvider);
 
         expect(packageGraph.defaultPackage.hasDocumentation, isFalse);
+        // ignore: deprecated_member_use_from_same_package
         expect(packageGraph.defaultPackage.hasDocumentationFile, isFalse);
         expect(packageGraph.defaultPackage.documentationFile, isNull);
         expect(packageGraph.defaultPackage.documentation, isNull);

tool/mustachio/codegen_runtime_renderer.dart

@@ -85,7 +85,7 @@ class RuntimeRenderersBuilder {
 // To change the contents of this library, make changes to the builder source
 // files in the tool/mustachio/ directory.
 
-// ignore_for_file: camel_case_types, unnecessary_cast, unused_element, unused_import, non_constant_identifier_names
+// ignore_for_file: camel_case_types, unnecessary_cast, unused_element, unused_import, non_constant_identifier_names, deprecated_member_use_from_same_package
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:dartdoc/dartdoc.dart';
 import 'package:dartdoc/src/generator/template_data.dart';