diff --git a/pkg/docgen/lib/docgen.dart b/pkg/docgen/lib/docgen.dart index 4aaef720f57..da817fe11db 100644 --- a/pkg/docgen/lib/docgen.dart +++ b/pkg/docgen/lib/docgen.dart @@ -576,9 +576,10 @@ class _Generator { // To avoid anaylzing package files twice, only files with paths not // containing '/packages' will be added. The only exception is if the file // to analyze already has a '/package' in its path. - var files = listDir(packageName, recursive: true).where( - (f) => f.endsWith('.dart') && (!f.contains('${path.separator}packages') - || packageName.contains('${path.separator}packages'))).toList(); + var files = listDir(packageName, recursive: true, listDir: _packageDirList) + .where((f) => f.endsWith('.dart') + && (!f.contains('${path.separator}packages') + || packageName.contains('${path.separator}packages'))).toList(); files.forEach((String lib) { // Only include libraries at the top level of "lib" @@ -596,6 +597,28 @@ class _Generator { return libraries; } + /// If [dir] contains both a `lib` directory and a `pubspec.yaml` file treat + /// it like a package and only return the `lib` dir. + /// + /// This ensures that packages don't have non-`lib` content documented. + static List _packageDirList(Directory dir) { + var entities = dir.listSync(); + + var pubspec = entities + .firstWhere((e) => e is File && + path.basename(e.path) == 'pubspec.yaml', orElse: () => null); + + var libDir = entities + .firstWhere((e) => e is Directory && + path.basename(e.path) == 'lib', orElse: () => null); + + if (pubspec != null && libDir != null) { + return [libDir]; + } else { + return entities; + } + } + /// All of the directories for our dependent packages /// If this is not a package, return an empty list. static List _allDependentPackageDirs(String packageDirectory) { diff --git a/pkg/docgen/lib/src/io.dart b/pkg/docgen/lib/src/io.dart index 96dad80a7fc..65e69e81bb4 100644 --- a/pkg/docgen/lib/src/io.dart +++ b/pkg/docgen/lib/src/io.dart @@ -9,7 +9,6 @@ library docgen.io; // pub/lib/src/io.dart. If the io.dart file becomes a package, should remove // copy of the functions. -import 'dart:collection'; import 'dart:io'; import 'package:path/path.dart' as path; @@ -20,138 +19,43 @@ import 'package:path/path.dart' as path; /// Excludes files and directories beginning with `.` /// /// The returned paths are guaranteed to begin with [dir]. -List listDir(String dir, {bool recursive: false}) { - List doList(String dir, Set listedDirectories) { - var contents = []; +List listDir(String dir, {bool recursive: false, + List listDir(Directory dir)}) { + if (listDir == null) listDir = (Directory dir) => dir.listSync(); - // Avoid recursive symlinks. - var resolvedPath = canonicalize(dir); - if (listedDirectories.contains(resolvedPath)) return []; - - listedDirectories = new Set.from(listedDirectories); - listedDirectories.add(resolvedPath); - - var children = []; - for (var entity in new Directory(dir).listSync()) { - if (path.basename(entity.path).startsWith('.')) { - continue; - } - - contents.add(entity.path); - if (entity is Directory) { - // TODO(nweiz): don't manually recurse once issue 4794 is fixed. - // Note that once we remove the manual recursion, we'll need to - // explicitly filter out files in hidden directories. - if (recursive) { - children.addAll(doList(entity.path, listedDirectories)); - } - } - } - - contents.addAll(children); - return contents; - } - - return doList(dir, new Set()); + return _doList(dir, new Set(), recursive, listDir); } -/// Returns the canonical path for [pathString]. This is the normalized, -/// absolute path, with symlinks resolved. As in [transitiveTarget], broken or -/// recursive symlinks will not be fully resolved. -/// -/// This doesn't require [pathString] to point to a path that exists on the -/// filesystem; nonexistent or unreadable path entries are treated as normal -/// directories. -String canonicalize(String pathString) { - var seen = new Set(); - var components = new Queue.from( - path.split(path.normalize(path.absolute(pathString)))); +List _doList(String dir, Set listedDirectories, bool recurse, + List listDir(Directory dir)) { + var contents = []; - // The canonical path, built incrementally as we iterate through [components]. - var newPath = components.removeFirst(); + // Avoid recursive symlinks. + var resolvedPath = new Directory(dir).resolveSymbolicLinksSync(); + if (listedDirectories.contains(resolvedPath)) return []; - // Move through the components of the path, resolving each one's symlinks as - // necessary. A resolved component may also add new components that need to be - // resolved in turn. - while (!components.isEmpty) { - seen.add(path.join(newPath, path.joinAll(components))); - var resolvedPath = resolveLink( - path.join(newPath, components.removeFirst())); - var relative = path.relative(resolvedPath, from: newPath); + listedDirectories = new Set.from(listedDirectories); + listedDirectories.add(resolvedPath); - // If the resolved path of the component relative to `newPath` is just ".", - // that means component was a symlink pointing to its parent directory. We - // can safely ignore such components. - if (relative == '.') continue; - - var relativeComponents = new Queue.from(path.split(relative)); - - // If the resolved path is absolute relative to `newPath`, that means it's - // on a different drive. We need to canonicalize the entire target of that - // symlink again. - if (path.isAbsolute(relative)) { - // If we've already tried to canonicalize the new path, we've encountered - // a symlink loop. Avoid going infinite by treating the recursive symlink - // as the canonical path. - if (seen.contains(relative)) { - newPath = relative; - } else { - newPath = relativeComponents.removeFirst(); - relativeComponents.addAll(components); - components = relativeComponents; - } + var children = []; + for (var entity in listDir(new Directory(dir))) { + // Skip hidden files and directories + if (path.basename(entity.path).startsWith('.')) { continue; } - // Pop directories off `newPath` if the component links upwards in the - // directory hierarchy. - while (relativeComponents.first == '..') { - newPath = path.dirname(newPath); - relativeComponents.removeFirst(); + contents.add(entity.path); + if (entity is Directory) { + // TODO(nweiz): don't manually recurse once issue 4794 is fixed. + // Note that once we remove the manual recursion, we'll need to + // explicitly filter out files in hidden directories. + if (recurse) { + children.addAll(_doList(entity.path, listedDirectories, recurse, + listDir)); + } } - - // If there's only one component left, [resolveLink] guarantees that it's - // not a link (or is a broken link). We can just add it to `newPath` and - // continue resolving the remaining components. - if (relativeComponents.length == 1) { - newPath = path.join(newPath, relativeComponents.single); - continue; - } - - // If we've already tried to canonicalize the new path, we've encountered a - // symlink loop. Avoid going infinite by treating the recursive symlink as - // the canonical path. - var newSubPath = path.join(newPath, path.joinAll(relativeComponents)); - if (seen.contains(newSubPath)) { - newPath = newSubPath; - continue; - } - - // If there are multiple new components to resolve, add them to the - // beginning of the queue. - relativeComponents.addAll(components); - components = relativeComponents; } - return newPath; -} -/// Returns the transitive target of [link] (if A links to B which links to C, -/// this will return C). If [link] is part of a symlink loop (e.g. A links to B -/// which links back to A), this returns the path to the first repeated link (so -/// `transitiveTarget("A")` would return `"A"` and `transitiveTarget("A")` would -/// return `"B"`). -/// -/// This accepts paths to non-links or broken links, and returns them as-is. -String resolveLink(String link) { - var seen = new Set(); - while (linkExists(link) && !seen.contains(link)) { - seen.add(link); - link = path.normalize(path.join( - path.dirname(link), new Link(link).targetSync())); - } - return link; + contents.addAll(children); + return contents; } - -/// Returns whether [link] exists on the file system. This will return `true` -/// for any symlink, regardless of what it points at or whether it's broken. -bool linkExists(String link) => new Link(link).existsSync(); diff --git a/pkg/docgen/test/generate_json_test.dart b/pkg/docgen/test/generate_json_test.dart index afb32feea5e..da11e2409fc 100644 --- a/pkg/docgen/test/generate_json_test.dart +++ b/pkg/docgen/test/generate_json_test.dart @@ -93,6 +93,34 @@ void main() { expect(comparator['comment'], expectedComment); }); }); + + test('exclude non-lib code from package docs', () { + schedule(() { + var thisScript = Platform.script; + var thisPath = p.fromUri(thisScript); + expect(p.basename(thisPath), 'generate_json_test.dart'); + expect(p.dirname(thisPath), endsWith('test')); + + + var codeDir = p.normalize(p.join(thisPath, '..', '..')); + print(codeDir); + expect(FileSystemEntity.isDirectorySync(codeDir), isTrue); + return dg.docgen(['$codeDir/'], out: p.join(d.defaultRoot, 'docs')); + }); + + d.dir('docs', [ + d.dir('docgen', [ + d.matcherFile('docgen.json', _isJsonMap) + ]), + d.matcherFile('index.json', _isJsonMap), + d.matcherFile('index.txt', _hasSortedLines), + d.matcherFile('library_list.json', _isJsonMap), + d.nothing('test_lib.json'), + d.nothing('test_lib-bar.json'), + d.nothing('test_lib-foo.json') + ]).validate(); + + }); } final Matcher _hasSortedLines = predicate((String input) { diff --git a/utils/apidoc/docgen.gyp b/utils/apidoc/docgen.gyp index 59aa15fd575..49af512d519 100644 --- a/utils/apidoc/docgen.gyp +++ b/utils/apidoc/docgen.gyp @@ -85,10 +85,6 @@ '--exclude-lib=async_helper', '--exclude-lib=expect', '--exclude-lib=docgen', - '--exclude-lib=canonicalization.a', - '--exclude-lib=canonicalization.b', - '--exclude-lib=canonicalization.c', - '--exclude-lib=canonicalization.d', '../../pkg', ], 'message': 'Running docgen: <(_action)',