PackageConfigProvider, MockSdk, etc for improved unit testing (#2332)

* The new PackageConfigProvider class abstracts over PackageConfig from package_config. PhysicalPackageConfigProvider uses the real one; MemoryPackageConfigProvider is used in tests.
* The new `isSdkLibraryDocumented` abstracts over SdkLibrary's `isDocumented`, because it throws unimplemented for `MockSdkLibrary`.
* Remove `ResourceProviderExtensions.defaultSdkDir`. Now this is a property of PackageMetaProvider.
* **Breaking change**: Move `io_utils` `listDir` to be a private method in PackageBuilder.
* **Breaking change**: Add parameter to PubPackageBuilder constructor for a PackageConfigProvider.
* **Breaking change**: Add two parameters to the PackageMetaProvider constructor, one for the default SDK directory, and one for the DartSdk.
* Deprecate package.dart's `substituteNameVersion`.
* Shorten doc comments here and there to 80 columns.
* Move any tests which use `testing/test_package_small` to be unit tests; delete the package in `testing/`.
* Move some tests for properties of package which use the ginormous testing package to unit tests

Author: srawlins

bin/dartdoc.dart

@@ -17,7 +17,9 @@ Future<void> main(List<String> arguments) async {
     // There was an error while parsing options.
     return;
   }
-  final packageBuilder = PubPackageBuilder(config, pubPackageMetaProvider);
+  final packageConfigProvider = PhysicalPackageConfigProvider();
+  final packageBuilder =
+      PubPackageBuilder(config, pubPackageMetaProvider, packageConfigProvider);
   final dartdoc = config.generateDocs
       ? await Dartdoc.fromContext(config, packageBuilder)
       : await Dartdoc.withEmptyGenerator(config, packageBuilder);

lib/dartdoc.dart

@@ -33,6 +33,7 @@ export 'package:dartdoc/src/dartdoc_options.dart';
 export 'package:dartdoc/src/element_type.dart';
 export 'package:dartdoc/src/generator/generator.dart';
 export 'package:dartdoc/src/model/model.dart';
+export 'package:dartdoc/src/package_config_provider.dart';
 export 'package:dartdoc/src/package_meta.dart';
 
 const String programName = 'dartdoc';

lib/src/dartdoc_options.dart

@@ -671,8 +671,8 @@ abstract class DartdocOption<T> {
   String get _directoryCurrentPath => resourceProvider.pathContext.current;
 
   /// Calls [valueAt] on the directory this element is defined in.
-  T valueAtElement(Element element) => valueAt(resourceProvider.getFolder(
-      resourceProvider.pathContext.canonicalize(
+  T valueAtElement(Element element) =>
+      valueAt(resourceProvider.getFolder(resourceProvider.pathContext.normalize(
           resourceProvider.pathContext.basename(element.source.fullName))));
 
   /// Adds a DartdocOption to the children of this DartdocOption.
@@ -1685,7 +1685,7 @@ Future<List<DartdocOption<Object>>> createDartdocOptions(
         return resourceProvider.pathContext
             .join(flutterRoot, 'bin', 'cache', 'dart-sdk');
       }
-      return resourceProvider.defaultSdkDir.path;
+      return packageMetaProvider.defaultSdkDir.path;
     }, packageMetaProvider.resourceProvider,
         help: 'Path to the SDK directory.', isDir: true, mustExist: true),
     DartdocOptionArgFile<bool>(

lib/src/io_utils.dart

@@ -11,7 +11,8 @@ import 'dart:io' as io;
 
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
-import 'package:dartdoc/src/package_meta.dart';
+import 'package:analyzer/src/generated/sdk.dart';
+import 'package:analyzer/src/test_utilities/mock_sdk.dart';
 import 'package:path/path.dart' as path;
 
 Encoding utf8AllowMalformed = Utf8Codec(allowMalformed: true);
@@ -34,6 +35,14 @@ String resolveTildePath(String originalPath) {
   return path.join(homeDir, originalPath.substring(2));
 }
 
+bool isSdkLibraryDocumented(SdkLibrary library) {
+  if (library is MockSdkLibrary) {
+    // Not implemented in [MockSdkLibrary].
+    return true;
+  }
+  return library.isDocumented;
+}
+
 extension ResourceProviderExtensions on ResourceProvider {
   Folder createSystemTemp(String prefix) {
     if (this is PhysicalResourceProvider) {
@@ -43,20 +52,6 @@ extension ResourceProviderExtensions on ResourceProvider {
     }
   }
 
-  Folder get defaultSdkDir {
-    if (this is PhysicalResourceProvider) {
-      var sdkDir = getFile(pathContext.absolute(io.Platform.resolvedExecutable))
-          .parent
-          .parent;
-      assert(pathContext.equals(
-          sdkDir.path, PubPackageMeta.sdkDirParent(sdkDir, this).path));
-      return sdkDir;
-    } else {
-      // TODO(srawlins): Return what is needed for tests.
-      return null;
-    }
-  }
-
   String get resolvedExecutable {
     if (this is PhysicalResourceProvider) {
       return io.Platform.resolvedExecutable;
@@ -94,48 +89,6 @@ extension ResourceProviderExtensions on ResourceProvider {
   }
 }
 
-/// Lists the contents of [dir].
-///
-/// If [recursive] is `true`, lists subdirectory contents (defaults to `false`).
-///
-/// Excludes files and directories beginning with `.`
-///
-/// The returned paths are guaranteed to begin with [dir].
-Iterable<String> listDir(String dir,
-    {bool recursive = false,
-    Iterable<io.FileSystemEntity> Function(io.Directory dir) listDir}) {
-  listDir ??= (io.Directory dir) => dir.listSync();
-
-  return _doList(dir, <String>{}, recursive, listDir);
-}
-
-Iterable<String> _doList(
-    String dir,
-    Set<String> listedDirectories,
-    bool recurse,
-    Iterable<io.FileSystemEntity> Function(io.Directory dir) listDir) sync* {
-  // Avoid recursive symlinks.
-  var resolvedPath = io.Directory(dir).resolveSymbolicLinksSync();
-  if (!listedDirectories.contains(resolvedPath)) {
-    listedDirectories = Set<String>.from(listedDirectories);
-    listedDirectories.add(resolvedPath);
-
-    for (var entity in listDir(io.Directory(dir))) {
-      // Skip hidden files and directories
-      if (path.basename(entity.path).startsWith('.')) {
-        continue;
-      }
-
-      yield entity.path;
-      if (entity is io.Directory) {
-        if (recurse) {
-          yield* _doList(entity.path, listedDirectories, recurse, listDir);
-        }
-      }
-    }
-  }
-}
-
 /// Converts `.` and `:` into `-`, adding a ".html" extension.
 ///
 /// For example:

lib/src/model/library.dart

@@ -10,6 +10,7 @@ import 'package:analyzer/dart/element/visitor.dart';
 import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
 import 'package:analyzer/src/generated/sdk.dart';
+import 'package:dartdoc/src/io_utils.dart';
 import 'package:dartdoc/src/model/model.dart';
 import 'package:dartdoc/src/package_meta.dart' show PackageMeta;
 import 'package:dartdoc/src/quiver.dart' as quiver;
@@ -212,7 +213,8 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
   @override
   bool get isPublic {
     if (!super.isPublic) return false;
-    if (sdkLib != null && (sdkLib.isInternal || !sdkLib.isDocumented)) {
+    if (sdkLib != null &&
+        (sdkLib.isInternal || !isSdkLibraryDocumented(sdkLib))) {
       return false;
     }
     if (config.isLibraryExcluded(name) ||

lib/src/model/package.dart

@@ -12,7 +12,9 @@ import 'package:dartdoc/src/warnings.dart';
 import 'package:path/path.dart' as path;
 import 'package:pub_semver/pub_semver.dart';
 
-final RegExp substituteNameVersion = RegExp(r'%([bnv])%');
+@Deprecated('Public variable intended to be private; will be removed as early '
+    'as Dartdoc 1.0.0')
+RegExp get substituteNameVersion => Package._substituteNameVersion;
 
 // All hrefs are emitted as relative paths from the output root. We are unable
 // to compute them from the page we are generating, and many properties computed
@@ -224,51 +226,56 @@ class Package extends LibraryContainer
   String _baseHref;
 
   String get baseHref {
-    if (_baseHref == null) {
-      if (documentedWhere == DocumentLocation.remote) {
-        _baseHref =
-            config.linkToUrl.replaceAllMapped(substituteNameVersion, (m) {
-          switch (m.group(1)) {
-            // Return the prerelease tag of the release if a prerelease,
-            // or 'stable' otherwise. Mostly coded around
-            // the Dart SDK's use of dev/stable, but theoretically applicable
-            // elsewhere.
-            case 'b':
-              {
-                var version = Version.parse(packageMeta.version);
-                var tag = 'stable';
-                if (version.isPreRelease) {
-                  // version.preRelease is a List<dynamic> with a mix of
-                  // integers and strings.  Given this, handle
-                  // 2.8.0-dev.1.0, 2.9.0-1.0.dev, and similar
-                  // variations.
-                  tag = version.preRelease.whereType<String>().first;
-                  // Who knows about non-SDK packages, but assert that SDKs
-                  // must conform to the known format.
-                  assert(
-                      packageMeta.isSdk == false || int.tryParse(tag) == null,
-                      'Got an integer as string instead of the expected "dev" tag');
-                }
-                return tag;
-              }
-            case 'n':
-              return name;
-            // The full version string of the package.
-            case 'v':
-              return packageMeta.version;
-            default:
-              assert(false, 'Unsupported case: ${m.group(1)}');
-              return null;
-          }
-        });
-        if (!_baseHref.endsWith('/')) _baseHref = '${_baseHref}/';
-      } else {
-        _baseHref = config.useBaseHref ? '' : HTMLBASE_PLACEHOLDER;
-      }
+    if (_baseHref != null) {
+      return _baseHref;
     }
+
+    if (documentedWhere == DocumentLocation.remote) {
+      _baseHref = _remoteBaseHref;
+      if (!_baseHref.endsWith('/')) _baseHref = '${_baseHref}/';
+    } else {
+      _baseHref = config.useBaseHref ? '' : HTMLBASE_PLACEHOLDER;
+    }
+
     return _baseHref;
   }
 
+  String get _remoteBaseHref {
+    return config.linkToUrl.replaceAllMapped(_substituteNameVersion, (m) {
+      switch (m.group(1)) {
+        // Return the prerelease tag of the release if a prerelease, or 'stable'
+        // otherwise.  Mostly coded around the Dart SDK's use of dev/stable, but
+        // theoretically applicable elsewhere.
+        case 'b':
+          {
+            var version = Version.parse(packageMeta.version);
+            var tag = 'stable';
+            if (version.isPreRelease) {
+              // `version.preRelease` is a `List<dynamic>` with a mix of
+              // integers and strings.  Given this, handle
+              // "2.8.0-dev.1.0, 2.9.0-1.0.dev", and similar variations.
+              tag = version.preRelease.whereType<String>().first;
+              // Who knows about non-SDK packages, but SDKs must conform to the
+              // known format.
+              assert(packageMeta.isSdk == false || int.tryParse(tag) == null,
+                  'Got an integer as string instead of the expected "dev" tag');
+            }
+            return tag;
+          }
+        case 'n':
+          return name;
+        // The full version string of the package.
+        case 'v':
+          return packageMeta.version;
+        default:
+          assert(false, 'Unsupported case: ${m.group(1)}');
+          return null;
+      }
+    });
+  }
+
+  static final _substituteNameVersion = RegExp(r'%([bnv])%');
+
   @override
   String get href => '$baseHref$filePath';