Refactor BuildCacheReader to BuildCacheAssetPathProvider.

Author: davidmorgan

_test_common/lib/common.dart

@@ -20,13 +20,11 @@ export 'sdk.dart';
 export 'test_phases.dart';
 
 Digest computeDigest(AssetId id, String contents) {
-  // Special handling for `$$` assets, these are generated under the .dart_tool
-  // dir and unfortunately that leaks into the digest computations.
-  if (id.package.startsWith(r'$$')) {
-    var package = id.package.substring(2);
-    id = AssetId(package, '.dart_tool/build/generated/$package/${id.path}');
-  }
-  return md5.convert([...utf8.encode(contents), ...id.toString().codeUnits]);
+  // Tests use `$$` at the start of an ID to signal "generated", remove it.
+  var idString = id.toString();
+  if (idString.startsWith(r'$$')) idString = idString.substring(2);
+
+  return md5.convert([...utf8.encode(contents), ...idString.codeUnits]);
 }
 
 class PlaceholderBuilder extends Builder {

_test_common/lib/matchers.dart

@@ -189,10 +189,13 @@ class _AssetGraphMatcher extends Matcher {
         }
       }
     }
-    if (!equals(_expected.packageLanguageVersions).matches(
-      item.packageLanguageVersions,
-      matchState['packageLanguageVersions'] = <String, LanguageVersion?>{},
-    )) {
+    if (!equals(
+      _expected.packageLanguageVersions,
+    ).matches(item.packageLanguageVersions, matchState)) {
+      matchState['packageLanguageVersions'] = [
+        item.packageLanguageVersions,
+        _expected.packageLanguageVersions,
+      ];
       matches = false;
     }
     return matches;

build/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 2.4.3-wip
 
+- `AssetNotFoundException` now also reports the missing `path`.
 - Bump the min sdk to 3.7.0.
 - Use `build_test` 3.0.0.
 - Add `package:build/src/internal.dart` for use by `build_resolvers`,
@@ -9,6 +10,7 @@
 - Add internal `Filesystem` that backs `AssetReader` and `AssetWriter`
   implementations.
 - Refactor `CachingAssetReader` to `FilesystemCache`.
+- Refactor `BuildCacheReader` to `BuildCacheAssetPathProvider`.
 
 ## 2.4.2
 

build/lib/src/asset/exceptions.dart

@@ -5,11 +5,15 @@ import 'id.dart';
 
 class AssetNotFoundException implements Exception {
   final AssetId assetId;
+  final String? path;
 
-  AssetNotFoundException(this.assetId);
+  AssetNotFoundException(this.assetId, {this.path});
 
   @override
-  String toString() => 'AssetNotFoundException: $assetId';
+  String toString() {
+    if (path == null) return 'AssetNotFoundException: $assetId';
+    return 'AssetNotFoundException: $assetId ($path)';
+  }
 }
 
 class PackageNotFoundException implements Exception {

build/lib/src/builder/build_step_impl.dart

@@ -85,10 +85,13 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
        _reportUnusedAssets = reportUnusedAssets;
 
   @override
-  BuildStepImpl copyWith({FilesystemCache? cache}) => BuildStepImpl(
+  BuildStepImpl copyWith({
+    AssetPathProvider? assetPathProvider,
+    FilesystemCache? cache,
+  }) => BuildStepImpl(
     inputId,
     allowedOutputs,
-    _reader.copyWith(cache: cache),
+    _reader.copyWith(assetPathProvider: assetPathProvider, cache: cache),
     _writer,
     _resolvers,
     _resourceManager,
@@ -107,7 +110,7 @@ class BuildStepImpl implements BuildStep, AssetReaderState {
   AssetFinder get assetFinder => _reader.assetFinder;
 
   @override
-  AssetPathProvider? get assetPathProvider => _reader.assetPathProvider;
+  AssetPathProvider get assetPathProvider => _reader.assetPathProvider;
 
   @override
   InputTracker? get inputTracker => _reader.inputTracker;

build/lib/src/state/asset_path_provider.dart

@@ -4,18 +4,7 @@
 
 import '../asset/id.dart';
 
-/// Converts [AssetId] to paths.
+/// Converts [AssetId]s to paths.
 abstract interface class AssetPathProvider {
   String pathFor(AssetId id);
 }
-
-/// Applies a function to an existing [AssetPathProvider].
-class OverlayAssetPathProvider implements AssetPathProvider {
-  AssetPathProvider delegate;
-  AssetId Function(AssetId) overlay;
-
-  OverlayAssetPathProvider({required this.delegate, required this.overlay});
-
-  @override
-  String pathFor(AssetId id) => delegate.pathFor(overlay(id));
-}

build/lib/src/state/reader_state.dart

@@ -12,9 +12,15 @@ import 'input_tracker.dart';
 /// Provides access to the state backing an [AssetReader].
 extension AssetReaderStateExtension on AssetReader {
   /// Returns a new instance with optionally updated [cache].
-  AssetReader copyWith({FilesystemCache? cache}) {
+  AssetReader copyWith({
+    AssetPathProvider? assetPathProvider,
+    FilesystemCache? cache,
+  }) {
     _requireIsAssetReaderState();
-    return (this as AssetReaderState).copyWith(cache: cache);
+    return (this as AssetReaderState).copyWith(
+      assetPathProvider: assetPathProvider,
+      cache: cache,
+    );
   }
 
   Filesystem get filesystem {
@@ -35,6 +41,11 @@ extension AssetReaderStateExtension on AssetReader {
   InputTracker? get inputTracker =>
       this is AssetReaderState ? (this as AssetReaderState).inputTracker : null;
 
+  AssetPathProvider get assetPathProvider {
+    _requireIsAssetReaderState();
+    return (this as AssetReaderState).assetPathProvider;
+  }
+
   /// Gets [inputTracker] or throws a descriptive error if it is `null`.
   InputTracker get requireInputTracker {
     final result = inputTracker;
@@ -47,11 +58,6 @@ extension AssetReaderStateExtension on AssetReader {
     return result;
   }
 
-  AssetPathProvider? get assetPathProvider =>
-      this is AssetReaderState
-          ? (this as AssetReaderState).assetPathProvider
-          : null;
-
   /// Throws if `this` is not an [AssetReaderState].
   void _requireIsAssetReaderState() {
     if (this is! AssetReaderState) {
@@ -64,8 +70,11 @@ extension AssetReaderStateExtension on AssetReader {
 
 /// The state backing an [AssetReader].
 abstract interface class AssetReaderState {
-  /// Returns a new instance with optionally updated [cache].
-  AssetReader copyWith({FilesystemCache? cache});
+  /// Returns a new instance with optionally updated [assetPathProvider] and/or [cache].
+  AssetReader copyWith({
+    AssetPathProvider? assetPathProvider,
+    FilesystemCache? cache,
+  });
 
   /// The [Filesystem] that this reader reads from.
   ///
@@ -86,7 +95,6 @@ abstract interface class AssetReaderState {
   /// not have one.
   InputTracker? get inputTracker;
 
-  /// The [AssetPathProvider] associated with this reader, or `null` if it does
-  /// not have one.
-  AssetPathProvider? get assetPathProvider;
+  /// The [AssetPathProvider] associated with this reader.
+  AssetPathProvider get assetPathProvider;
 }

build_runner_core/CHANGELOG.md

@@ -10,6 +10,7 @@
 - Add internal `Filesystem` that backs `AssetReader` and `AssetWriter`
   implementations.
 - Refactor `CachingAssetReader` to `FilesystemCache`.
+- Refactor `BuildCacheReader` to `BuildCacheAssetPathProvider`.
 
 ## 8.0.0
 

build_runner_core/lib/build_runner_core.dart

@@ -4,7 +4,6 @@
 
 export 'package:build/build.dart' show PostProcessBuildStep, PostProcessBuilder;
 
-export 'src/asset/batch.dart' show wrapInBatch;
 export 'src/asset/file_based.dart';
 export 'src/asset/finalized_reader.dart';
 export 'src/asset/writer.dart';

build_runner_core/lib/src/asset/batch.dart

@@ -25,6 +25,8 @@ import 'writer.dart';
 ///
 /// The default [IOEnvironment] uses readers and writes that are batch-aware
 /// outside of low-memory mode.
+///
+/// TODO(davidmorgan): this is not currently used, refactor into `Filesystem`.
 final class _FileSystemWriteBatch {
   final Map<AssetId, _PendingFileState> _pendingWrites = {};
 
@@ -54,7 +56,7 @@ final class _FileSystemWriteBatch {
 ///
 /// The returned reader will see pending writes by the returned writer before
 /// they are flushed to the file system.
-(AssetReader, RunnerAssetWriter) wrapInBatch({
+(BatchReader, BatchWriter) wrapInBatch({
   required AssetReader reader,
   required RunnerAssetWriter writer,
 }) {
@@ -72,35 +74,18 @@ final class _PendingFileState {
 }
 
 @internal
-final class BatchReader extends AssetReader implements AssetReaderState {
-  @override
+final class BatchReader {
   late final AssetFinder assetFinder = FunctionAssetFinder(_findAssets);
+
   final AssetReader _inner;
   final _FileSystemWriteBatch _batch;
 
   BatchReader(this._inner, this._batch);
 
-  @override
-  BatchReader copyWith({FilesystemCache? cache}) =>
-      BatchReader(_inner.copyWith(cache: cache), _batch);
-
-  @override
-  Filesystem get filesystem => _inner.filesystem;
-
-  @override
-  FilesystemCache get cache => _inner.cache;
-
-  @override
-  AssetPathProvider? get assetPathProvider => _inner.assetPathProvider;
-
-  @override
-  InputTracker? get inputTracker => _inner.inputTracker;
-
   _PendingFileState? _stateFor(AssetId id) {
     return _batch._pendingWrites[id];
   }
 
-  @override
   Future<bool> canRead(AssetId id) async {
     if (_stateFor(id) case final state?) {
       return !state.isDeleted;
@@ -110,7 +95,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
   }
 
   // This is only for generators, so only `BuildStep` needs to implement it.
-  @override
   Stream<AssetId> findAssets(Glob glob) => throw UnimplementedError();
 
   Stream<AssetId> _findAssets(Glob glob, String? package) {
@@ -119,7 +103,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
         .where((asset) => _stateFor(asset)?.isDeleted != true);
   }
 
-  @override
   Future<List<int>> readAsBytes(AssetId id) async {
     if (_stateFor(id) case final state?) {
       if (state.isDeleted) {
@@ -132,7 +115,6 @@ final class BatchReader extends AssetReader implements AssetReaderState {
     }
   }
 
-  @override
   Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) async {
     if (_stateFor(id) case final state?) {
       if (state.isDeleted) {

build_runner_core/lib/src/asset/build_cache.dart

@@ -4,84 +4,37 @@ import 'dart:convert';
 import 'package:build/build.dart';
 // ignore: implementation_imports
 import 'package:build/src/internal.dart';
-import 'package:crypto/crypto.dart';
-import 'package:glob/glob.dart';
 
 import '../asset_graph/graph.dart';
 import '../asset_graph/node.dart';
 import '../util/constants.dart';
 import 'writer.dart';
 
-/// Wraps an [AssetReader] and translates reads for generated files into reads
-/// from the build cache directory
-class BuildCacheReader implements AssetReader, AssetReaderState {
-  @override
-  final AssetPathProvider? assetPathProvider;
-
-  final AssetReader _delegate;
+/// Wraps an [AssetPathProvider] to place hidden generated files in the build
+/// cache directory.
+///
+/// Assets that are in the provided [AssetGraph], have node type
+/// [GeneratedAssetNode] and have `isHidden == true` are mapped to
+/// [generatedOutputDirectory].
+class BuildCacheAssetPathProvider implements AssetPathProvider {
+  final AssetPathProvider _delegate;
   final AssetGraph _assetGraph;
   final String _rootPackage;
 
-  BuildCacheReader(
-    AssetReader delegate,
-    AssetGraph assetGraph,
-    String rootPackage,
-  ) : _delegate = delegate,
-      _assetGraph = assetGraph,
-      _rootPackage = rootPackage,
-      assetPathProvider =
-          delegate.assetPathProvider == null
-              ? null
-              : OverlayAssetPathProvider(
-                delegate: delegate.assetPathProvider!,
-                overlay: (id) => _cacheLocation(id, assetGraph, rootPackage),
-              );
-
-  @override
-  BuildCacheReader copyWith({FilesystemCache? cache}) => BuildCacheReader(
-    _delegate.copyWith(cache: cache),
-    _assetGraph,
-    _rootPackage,
-  );
-
-  @override
-  Filesystem get filesystem => _delegate.filesystem;
-
-  @override
-  FilesystemCache get cache => _delegate.cache;
-
-  @override
-  AssetFinder get assetFinder => _delegate.assetFinder;
-
-  @override
-  InputTracker? get inputTracker => _delegate.inputTracker;
-
-  @override
-  Future<bool> canRead(AssetId id) =>
-      _delegate.canRead(_cacheLocation(id, _assetGraph, _rootPackage));
-
-  @override
-  Future<Digest> digest(AssetId id) =>
-      _delegate.digest(_cacheLocation(id, _assetGraph, _rootPackage));
-
-  @override
-  Future<List<int>> readAsBytes(AssetId id) =>
-      _delegate.readAsBytes(_cacheLocation(id, _assetGraph, _rootPackage));
-
-  @override
-  Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) =>
-      _delegate.readAsString(
-        _cacheLocation(id, _assetGraph, _rootPackage),
-        encoding: encoding,
-      );
+  BuildCacheAssetPathProvider({
+    required AssetPathProvider delegate,
+    required AssetGraph assetGraph,
+    required String rootPackage,
+  }) : _delegate = delegate,
+       _assetGraph = assetGraph,
+       _rootPackage = rootPackage;
 
   @override
-  Stream<AssetId> findAssets(Glob glob) =>
-      throw UnimplementedError(
-        'Asset globbing should be done per phase with the AssetGraph',
-      );
+  String pathFor(AssetId id) =>
+      _delegate.pathFor(_cacheLocation(id, _assetGraph, _rootPackage));
 }
 
+// TODO(davidmorgan): refactor in the same way as the reader.
 class BuildCacheWriter implements RunnerAssetWriter {
   final AssetGraph _assetGraph;
   final RunnerAssetWriter _delegate;