Reduce use of generators in finding files to document

Author: srawlins

lib/src/model/package_builder.dart

@@ -222,13 +222,8 @@ class PubPackageBuilder implements PackageBuilder {
   /// Uses [processedLibraries] to prevent calling [addLibrary] more than once
   /// with the same [LibraryElement]. Adds each [LibraryElement] found to
   /// [processedLibraries].
-  ///
-  /// [addingSpecials] indicates that only [SpecialClass]es are being resolved
-  /// in this round.
-  Future<void> _discoverLibraries(
-      void Function(DartDocResolvedLibrary) addLibrary,
-      Set<LibraryElement> processedLibraries,
-      Set<String> files) async {
+  Future<void> _discoverLibraries(PackageGraph uninitializedPackageGraph,
+      Set<LibraryElement> processedLibraries, Set<String> files) async {
     files = {...files};
     // Discover Dart libraries in a loop. In each iteration of the loop, we take
     // a set of files (starting with the ones passed into the function), resolve
@@ -283,7 +278,7 @@ class PubPackageBuilder implements PackageBuilder {
         if (processedLibraries.contains(resolvedLibrary.element)) {
           continue;
         }
-        addLibrary(resolvedLibrary);
+        uninitializedPackageGraph.addLibraryToGraph(resolvedLibrary);
         processedLibraries.add(resolvedLibrary.element);
       }
       files.addAll(newFiles);
@@ -300,17 +295,13 @@ class PubPackageBuilder implements PackageBuilder {
       // (so we can generate the right hyperlinks), it's vital that we add all
       // libraries in dependent packages. So if the analyzer discovers some
       // files in a package we haven't seen yet, add files for that package.
-      for (var meta in packages.difference(knownPackages)) {
-        if (meta.isSdk) {
+      for (var packageMeta in packages.difference(knownPackages)) {
+        if (packageMeta.isSdk) {
           if (!_skipUnreachableSdkLibraries) {
             files.addAll(_sdkFilesToDocument);
           }
         } else {
-          files.addAll(await _findFilesToDocumentInPackage(
-            meta.dir.path,
-            includeDependencies: false,
-            filterExcludes: false,
-          ).toList());
+          files.addAll(_findFilesToDocumentInPackage({packageMeta.dir.path}));
         }
       }
       knownPackages.addAll(packages);
@@ -320,40 +311,19 @@ class PubPackageBuilder implements PackageBuilder {
 
   /// Returns all top level library files in the 'lib/' directory of the given
   /// package root directory.
-  ///
-  /// If [includeDependencies], then all top level library files in the 'lib/'
-  /// directory of every package in [basePackageDir]'s package config are also
-  /// included.
-  Stream<String> _findFilesToDocumentInPackage(
-    String basePackageDir, {
-    required bool includeDependencies,
-    required bool filterExcludes,
-  }) async* {
-    var packageDirs = {basePackageDir};
-
-    if (includeDependencies) {
-      var packageConfig = (await _packageConfigProvider
-          .findPackageConfig(_resourceProvider.getFolder(basePackageDir)))!;
-      for (var package in packageConfig.packages) {
-        if (filterExcludes && _config.exclude.contains(package.name)) {
-          continue;
-        }
-        packageDirs.add(_pathContext.dirname(
-            _pathContext.fromUri(packageConfig[package.name]!.packageUriRoot)));
-      }
-    }
-
+  List<String> _findFilesToDocumentInPackage(Set<String> packageRoots) {
     var sep = _pathContext.separator;
     var packagesWithSeparators = '${sep}packages$sep';
-    for (var packageDir in packageDirs) {
-      var packageLibDir = _pathContext.join(packageDir, 'lib');
+    var filesToDocument = <String>[];
+    for (var packageRoot in packageRoots) {
+      var packageLibDir = _pathContext.join(packageRoot, 'lib');
       var packageLibSrcDir = _pathContext.join(packageLibDir, 'src');
       var packageDirContainsPackages =
-          packageDir.contains(packagesWithSeparators);
+          packageRoot.contains(packagesWithSeparators);
       // To avoid analyzing 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 '/packages/' in its path.
-      for (var filePath in _listDir(packageDir, const {})) {
+      for (var filePath in _listDir(packageRoot, const {})) {
         if (!filePath.endsWith('.dart')) continue;
         if (!packageDirContainsPackages &&
             filePath.contains(packagesWithSeparators)) {
@@ -368,30 +338,32 @@ class PubPackageBuilder implements PackageBuilder {
           continue;
         }
 
-        yield filePath;
+        filesToDocument.add(filePath);
       }
     }
+    return filesToDocument;
   }
 
   /// Lists the files in [directory].
   ///
   /// Excludes files and directories beginning with `.`.
   ///
   /// The returned paths are guaranteed to begin with [directory].
-  Iterable<String> _listDir(
-      String directory, Set<String> listedDirectories) sync* {
+  List<String> _listDir(String directory, Set<String> listedDirectories) {
     // Avoid recursive symlinks.
     var resolvedPath =
         _resourceProvider.getFolder(directory).resolveSymbolicLinksSync().path;
     if (listedDirectories.contains(resolvedPath)) {
-      return;
+      return const [];
     }
 
     listedDirectories = {
       ...listedDirectories,
       resolvedPath,
     };
 
+    var dirs = <String>[];
+
     for (var resource
         in _packageDirList(_resourceProvider.getFolder(directory))) {
       // Skip hidden files and directories.
@@ -400,13 +372,15 @@ class PubPackageBuilder implements PackageBuilder {
       }
 
       if (resource is File) {
-        yield resource.path;
+        dirs.add(resource.path);
         continue;
       }
       if (resource is Folder) {
-        yield* _listDir(resource.path, listedDirectories);
+        dirs.addAll(_listDir(resource.path, listedDirectories));
       }
     }
+
+    return dirs;
   }
 
   /// Calculates 'includeExternal' based on a list of files.
@@ -429,22 +403,45 @@ class PubPackageBuilder implements PackageBuilder {
   /// This takes into account the 'auto-include-dependencies' option, the
   /// 'exclude' option, and the 'include-external' option.
   Future<Set<String>> _getFilesToDocument() async {
-    var files = _config.topLevelPackageMeta.isSdk
-        ? _sdkFilesToDocument
-        : await _findFilesToDocumentInPackage(
-            _config.inputDir,
-            includeDependencies: _config.autoIncludeDependencies,
-            filterExcludes: true,
-          ).toList();
-    var externals = _includedExternalsFrom(files);
-    if (externals.isNotEmpty) {
-      includeExternalsWasSpecified = true;
+    if (_config.topLevelPackageMeta.isSdk) {
+      return _sdkFilesToDocument
+          .map((s) => _pathContext.absolute(_resourceProvider.getFile(s).path))
+          .toSet();
+    } else {
+      var packagesToDocument = await _findPackagesToDocument(
+        _config.inputDir,
+      );
+      var files = _findFilesToDocumentInPackage(packagesToDocument).toList();
+      var externals = _includedExternalsFrom(files);
+      if (externals.isNotEmpty) {
+        includeExternalsWasSpecified = true;
+        files = [...files, ...externals];
+      }
+      return {
+        ...files.map(
+            (s) => _pathContext.absolute(_resourceProvider.getFile(s).path)),
+        ..._embedderSdkFiles,
+      };
     }
-    files = [...files, ...externals];
+  }
+
+  /// Returns a set of package roots that are to be documented.
+  ///
+  /// If `_config.autoIncludeDependencies` is `true`, then every package in
+  /// [basePackageRoot]'s package config is included.
+  Future<Set<String>> _findPackagesToDocument(String basePackageRoot) async {
+    if (!_config.autoIncludeDependencies) {
+      return {basePackageRoot};
+    }
+
+    var packageConfig = (await _packageConfigProvider
+        .findPackageConfig(_resourceProvider.getFolder(basePackageRoot)))!;
     return {
-      ...files
-          .map((s) => _pathContext.absolute(_resourceProvider.getFile(s).path)),
-      ..._embedderSdkFiles,
+      basePackageRoot,
+      for (var package in packageConfig.packages)
+        if (!_config.exclude.contains(package.name))
+          _pathContext.dirname(_pathContext
+              .fromUri(packageConfig[package.name]!.packageUriRoot)),
     };
   }
 
@@ -472,7 +469,7 @@ class PubPackageBuilder implements PackageBuilder {
     logInfo('Discovering libraries...');
     var foundLibraries = <LibraryElement>{};
     await _discoverLibraries(
-      uninitializedPackageGraph.addLibraryToGraph,
+      uninitializedPackageGraph,
       foundLibraries,
       files,
     );