Add a nodoc configuration option to allow declaring entire files to not be documented (#2369)

* Add a nodoc dartdoc configuration option with tests

* Add documentation for the nodoc configuration option.

* Fix bug on empty doc comment

* Sprinkle canonicalization fairy dust on paths for Windows

* more fairy dust

* pre-cleanup windows fix

* Tighten up Windows support and add a test

* Delete obsolete comment

* Re-add dart:io for Platform

Author: jcollins-g

README.md

@@ -109,6 +109,7 @@ dartdoc:
   categoryOrder: ["First Category", "Second Category"]
   examplePathPrefix: 'subdir/with/examples'
   includeExternal: ['bin/unusually_located_library.dart']
+  nodoc: ['lib/sekret/*.dart']
   linkTo:
     url: "https://my.dartdocumentationsite.org/dev/%v%"
   showUndocumentedCategories: true
@@ -136,7 +137,7 @@ Unrecognized options will be ignored.  Supported options:
     directives.
   * **exclude**:  Specify a list of library names to avoid generating docs for,
     overriding any specified in include.  All libraries listed must be local to this package, unlike
-    the command line `--exclude`.
+    the command line `--exclude`.  See also `nodoc`.
   * **errors**:  Specify warnings to be treated as errors.  See the lists of valid warnings in the command
     line help for `--errors`, `--warnings`, and `--ignore`.
   * **favicon**:  A path to a favicon for the generated docs.
@@ -175,6 +176,11 @@ Unrecognized options will be ignored.  Supported options:
       * `%f%`:  Relative path of file to the repository root
       * `%r%`:  Revision
       * `%l%`:  Line number
+  * **nodoc**: Specify files (via globs) which should be treated as though they have the `@nodoc`
+    tag in the documentation comment of every defined element.  Unlike `exclude` this can specify
+    source files directly, and neither inheritance nor reexports will cause these elements to be
+    documented when included in other libraries.  For more fine-grained control, use `@nodoc` in
+    element documentation comments directly, or the `exclude` directive.
   * **warnings**:  Specify otherwise ignored or set-to-error warnings to simply warn.  See the lists
     of valid warnings in the command line help for `--errors`, `--warnings`, and `--ignore`.
 

lib/src/dartdoc_options.dart

@@ -1489,6 +1489,8 @@ class DartdocOptionContext extends DartdocOptionContextBase
   // ignore: unused_element
   String get _linkToHosted => optionSet['linkTo']['hosted'].valueAt(context);
 
+  List<String> get nodoc => optionSet['nodoc'].valueAt(context);
+
   String get output => optionSet['output'].valueAt(context);
 
   PackageMeta get packageMeta => optionSet['packageMeta'].valueAt(context);
@@ -1669,6 +1671,11 @@ Future<List<DartdocOption<Object>>> createDartdocOptions(
             help: 'Allow links to be generated for packages outside this one.',
             negatable: true),
       ]),
+    DartdocOptionFileOnly<List<String>>('nodoc', [], resourceProvider,
+        optionIs: OptionKind.glob,
+        help: 'Dart symbols declared in these '
+            'files will be treated as though they have the @nodoc flag added to '
+            'their documentation comment.'),
     DartdocOptionArgOnly<String>('output',
         resourceProvider.pathContext.join('doc', 'api'), resourceProvider,
         optionIs: OptionKind.dir, help: 'Path to output directory.'),

lib/src/model/documentation_comment.dart

@@ -47,10 +47,14 @@ mixin DocumentationComment
   String computeDocumentationComment();
 
   /// Returns true if the raw documentation comment has a nodoc indication.
-  bool get hasNodoc =>
-      documentationComment != null &&
-      (documentationComment.contains('@nodoc') ||
-          documentationComment.contains('<nodoc>'));
+  bool get hasNodoc {
+    if (documentationComment != null &&
+        (documentationComment.contains('@nodoc') ||
+            documentationComment.contains('<nodoc>'))) {
+      return true;
+    }
+    return packageGraph.configSetsNodocFor(element.source.fullName);
+  }
 
   /// Process a [documentationComment], performing various actions based on
   /// `{@}`-style directives, except `{@tool}`, returning the processed result.

lib/src/model/model_element.dart

@@ -472,11 +472,7 @@ abstract class ModelElement extends Canonicalization
           !(enclosingElement as Extension).isPublic) {
         _isPublic = false;
       } else {
-        if (documentationComment == null) {
-          _isPublic = utils.hasPublicName(element);
-        } else {
-          _isPublic = utils.hasPublicName(element) && !hasNodoc;
-        }
+        _isPublic = utils.hasPublicName(element) && !hasNodoc;
       }
     }
     return _isPublic;

lib/src/model/package_graph.dart

@@ -22,6 +22,7 @@ import 'package:dartdoc/src/render/renderer_factory.dart';
 import 'package:dartdoc/src/special_elements.dart';
 import 'package:dartdoc/src/tuple.dart';
 import 'package:dartdoc/src/warnings.dart';
+import 'package:dartdoc/src/model_utils.dart' show matchGlobs;
 
 class PackageGraph {
   PackageGraph.UninitializedPackageGraph(
@@ -950,6 +951,25 @@ class PackageGraph {
         allLocalModelElements.where((e) => e.isCanonical).toList();
   }
 
+  /// Glob lookups can be expensive.  Cache per filename.
+  final _configSetsNodocFor = HashMap<String, bool>();
+
+  /// Given an element's location, look up the nodoc configuration data and
+  /// determine whether to unconditionally treat the element as "nodoc".
+  bool configSetsNodocFor(String fullName) {
+    if (!_configSetsNodocFor.containsKey(fullName)) {
+      var file = resourceProvider.getFile(fullName);
+      // Direct lookup instead of generating a custom context will save some
+      // cycles.  We can't use the element's [DartdocOptionContext] because that
+      // might not be where the element was defined, which is what's important
+      // for nodoc's semantics.  Looking up the defining element just to pull
+      // a context is again, slow.
+      List<String> globs = config.optionSet['nodoc'].valueAt(file.parent);
+      _configSetsNodocFor[fullName] = matchGlobs(globs, fullName);
+    }
+    return _configSetsNodocFor[fullName];
+  }
+
   String getMacro(String name) {
     assert(_localDocumentationBuilt);
     return _macros[name];

lib/src/model_utils.dart

@@ -5,15 +5,56 @@
 library dartdoc.model_utils;
 
 import 'dart:convert';
+import 'dart:io' show Platform;
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/src/dart/ast/utilities.dart';
+import 'package:dartdoc/dartdoc.dart';
 import 'package:dartdoc/src/model/model.dart';
+import 'package:path/path.dart' as path;
+import 'package:glob/glob.dart';
+
+final _driveLetterMatcher = RegExp(r'^\w:\\');
 
 final Map<String, String> _fileContents = <String, String>{};
 
+/// This will handle matching globs, including on Windows.
+///
+/// On windows, globs are assumed to use absolute Windows paths with drive
+/// letters in combination with globs, e.g. `C:\foo\bar\*.txt`.  `fullName`
+/// also is assumed to have a drive letter.
+bool matchGlobs(List<String> globs, String fullName, {bool isWindows}) {
+  isWindows ??= Platform.isWindows;
+  var filteredGlobs = <String>[];
+
+  if (isWindows) {
+    // TODO(jcollins-g): port this special casing to the glob package.
+    var fullNameDriveLetter = _driveLetterMatcher.stringMatch(fullName);
+    if (fullNameDriveLetter == null) {
+      throw DartdocFailure(
+          'Unable to recognize drive letter on Windows in:  $fullName');
+    }
+    // Build a matcher from the [fullName]'s drive letter to filter the globs.
+    var driveGlob = RegExp(fullNameDriveLetter.replaceFirst(r'\', r'\\'),
+        caseSensitive: false);
+    fullName = fullName.replaceFirst(_driveLetterMatcher, r'\');
+    for (var glob in globs) {
+      // Globs don't match if they aren't for the same drive.
+      if (!driveGlob.hasMatch(glob)) continue;
+      // `C:\` => `\` for rejoining via posix.
+      glob = glob.replaceFirst(_driveLetterMatcher, r'/');
+      filteredGlobs.add(path.posix.joinAll(path.windows.split(glob)));
+    }
+  } else {
+    filteredGlobs.addAll(globs);
+  }
+
+  return filteredGlobs.any((g) =>
+      Glob(g, context: isWindows ? path.windows : null).matches(fullName));
+}
+
 /// Returns the [AstNode] for a given [Element].
 ///
 /// Uses a precomputed map of [element.source.fullName] to [CompilationUnit]

pubspec.yaml

@@ -12,6 +12,7 @@ dependencies:
   collection: ^1.2.0
   cli_util: '>=0.1.4 <0.3.0'
   crypto: ^2.0.6
+  glob: '>=1.1.2 <2.0.0'
   html: '>=0.12.1 <0.15.0'
   logging: ^0.11.3+1
   markdown: ^2.1.5
@@ -29,7 +30,6 @@ dev_dependencies:
   build_version: ^2.0.1
   coverage: ^0.14.0
   dhttpd: ^3.0.0
-  glob: ^1.1.5
   grinder: ^0.8.2
   http: ^0.12.0
   pedantic: ^1.9.0

test/dartdoc_options_test.dart

@@ -383,13 +383,20 @@ dartdoc:
     });
 
     group('glob options', () {
+      String canonicalize(String path) =>
+          resourceProvider.pathContext.canonicalize(path);
+
       test('work via the command line', () {
-        dartdocOptionSetAll.parseArguments(['--glob-option', 'foo/**']);
+        dartdocOptionSetAll
+            .parseArguments(['--glob-option', path.join('foo', '**')]);
         expect(
             dartdocOptionSetAll['globOption'].valueAtCurrent(),
             equals([
-              resourceProvider.pathContext
-                  .join(resourceProvider.pathContext.current, 'foo/**')
+              resourceProvider.pathContext.joinAll([
+                canonicalize(resourceProvider.pathContext.current),
+                'foo',
+                '**'
+              ])
             ]));
       });
 
@@ -398,22 +405,26 @@ dartdoc:
         expect(
             dartdocOptionSetAll['globOption'].valueAt(secondDir),
             equals([
-              resourceProvider.pathContext.join(secondDir.path, 'q*.html'),
-              resourceProvider.pathContext.join(secondDir.path, 'e*.dart')
+              canonicalize(
+                  resourceProvider.pathContext.join(secondDir.path, 'q*.html')),
+              canonicalize(
+                  resourceProvider.pathContext.join(secondDir.path, 'e*.dart')),
             ]));
         // No child override, should be the same as parent
         expect(
             dartdocOptionSetAll['globOption'].valueAt(secondDirSecondSub),
             equals([
-              resourceProvider.pathContext.join(secondDir.path, 'q*.html'),
-              resourceProvider.pathContext.join(secondDir.path, 'e*.dart')
+              canonicalize(
+                  resourceProvider.pathContext.join(secondDir.path, 'q*.html')),
+              canonicalize(
+                  resourceProvider.pathContext.join(secondDir.path, 'e*.dart')),
             ]));
         // Child directory overrides
         expect(
             dartdocOptionSetAll['globOption'].valueAt(secondDirFirstSub),
             equals([
-              resourceProvider.pathContext
-                  .join(secondDirFirstSub.path, '**/*.dart')
+              resourceProvider.pathContext.joinAll(
+                  [canonicalize(secondDirFirstSub.path), '**', '*.dart'])
             ]));
       });
     });

test/end2end/model_test.dart

@@ -849,6 +849,22 @@ void main() {
       });
     });
 
+    group('Comment processing', () {
+      test('can virtually add nodoc via options file', () {
+        var NodocMeLibrary = packageGraph.defaultPackage.allLibraries
+            .firstWhere((l) => l.name == 'nodocme');
+        expect(NodocMeLibrary.hasNodoc, isTrue);
+        var NodocMeImplementation = fakeLibrary.allClasses
+            .firstWhere((c) => c.name == 'NodocMeImplementation');
+        expect(NodocMeImplementation.hasNodoc, isTrue);
+        expect(NodocMeImplementation.isPublic, isFalse);
+        var MeNeitherEvenWithoutADocComment = fakeLibrary.allClasses
+            .firstWhere((c) => c.name == 'MeNeitherEvenWithoutADocComment');
+        expect(MeNeitherEvenWithoutADocComment.hasNodoc, isTrue);
+        expect(MeNeitherEvenWithoutADocComment.isPublic, isFalse);
+      });
+    });
+
     group('doc references', () {
       String docsAsHtml;
 

test/model_utils_test.dart

@@ -8,6 +8,22 @@ import 'package:dartdoc/src/model_utils.dart';
 import 'package:test/test.dart';
 
 void main() {
+  group('match glob', () {
+    test('basic POSIX', () {
+      expect(
+          matchGlobs(['/a/b/*', '/b/c/*'], '/b/c/d', isWindows: false), isTrue);
+      expect(matchGlobs(['/q/r/s'], '/foo', isWindows: false), isFalse);
+    });
+
+    test('basic Windows', () {
+      expect(matchGlobs([r'C:\a\b\*'], r'c:\a\b\d', isWindows: true), isTrue);
+    });
+
+    test('Windows does not pass for different drive letters', () {
+      expect(matchGlobs([r'C:\a\b\*'], r'D:\a\b\d', isWindows: true), isFalse);
+    });
+  });
+
   group('model_utils stripIndentFromSource', () {
     test('no indent', () {
       expect(stripIndentFromSource('void foo() {\n  print(1);\n}\n'),