Fix handling of checked in outputs. (#4007)

Author: davidmorgan

build_runner_core/lib/src/asset_graph/graph.dart

@@ -261,6 +261,8 @@ class AssetGraph implements GeneratedAssetHider {
   }
 
   /// Changes [id] and its transitive`primaryOutput`s to `missingSource` nodes.
+  ///
+  /// Removes post build applications with removed assets as inputs.
   void _setMissingRecursive(AssetId id, {Set<AssetId>? removedIds}) {
     removedIds ??= <AssetId>{};
     var node = get(id);
@@ -279,6 +281,19 @@ class AssetGraph implements GeneratedAssetHider {
     }
   }
 
+  /// Removes [id] and its transitive`primaryOutput`s from the graph.
+  void _removeRecursive(AssetId id, {Set<AssetId>? removedIds}) {
+    removedIds ??= <AssetId>{};
+    var node = get(id);
+    if (node == null) return;
+    if (removedIds.add(id)) {
+      for (var output in node.primaryOutputs.toList()) {
+        _removeRecursive(output, removedIds: removedIds);
+      }
+      _nodesByPackage[id.package]!.remove(id.path);
+    }
+  }
+
   /// Computes node outputs: the inverse of the graph described by the `inputs`
   /// fields on glob and generated nodes.
   ///
@@ -557,8 +572,9 @@ class AssetGraph implements GeneratedAssetHider {
   /// If there are existing [AssetNode.source]s or [AssetNode.missingSource]s
   /// that overlap the [AssetNode.generated]s, then they will be replaced with
   /// [AssetNode.generated]s, and their transitive `primaryOutputs` will be
-  /// changed to `missingSource` nodes. The return value is the set of assets
-  /// that were removed from the graph.
+  /// removed from the graph.
+  ///
+  /// The return value is the set of assets that were removed from the graph.
   Set<AssetId> _addGeneratedOutputs(
     Iterable<AssetId> outputs,
     int phaseNumber,
@@ -588,7 +604,7 @@ class AssetGraph implements GeneratedAssetHider {
             buildPhases.inBuildPhases[phaseNumber].builderLabel,
           );
         }
-        _setMissingRecursive(output, removedIds: removed);
+        _removeRecursive(output, removedIds: removed);
       }
 
       var newNode = AssetNode.generated(

build_runner_core/test/invalidation/checked_in_output_test.dart

@@ -0,0 +1,55 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:test/test.dart';
+
+import 'invalidation_tester.dart';
+
+void main() {
+  late InvalidationTester tester;
+
+  setUp(() {
+    tester = InvalidationTester();
+
+    // Start with output source on disk.
+    //
+    // The ordering of sources matters because a different codepath in
+    // `graph.dart` is triggered depending on whether a source is first
+    // processed as an input or as a generated output of another input.
+    //
+    // So for `a` have the output come first, and for `b` the input come
+    // first.
+    tester.sources(['a.g', 'a', 'b', 'b.g']);
+  });
+
+  group('a <== a.g, a <== a.other', () {
+    setUp(() {
+      tester.builder(from: '', to: '.g', outputIsVisible: true)
+        ..reads('')
+        ..writes('.g');
+
+      tester.builder(from: '', to: '.other', outputIsVisible: true)
+        ..reads('')
+        ..writes('.other');
+    });
+
+    test('checked in outputs are not treated as inputs', () async {
+      expect(
+        await tester.build(),
+        // If outputs were treated by inputs there would be outputs created like
+        // `a.g.g.other`.
+        Result(
+          written: [
+            'a.g',
+            'a.other',
+            'a.g.other',
+            'b.g',
+            'b.other',
+            'b.g.other',
+          ],
+        ),
+      );
+    });
+  });
+}

build_runner_core/test/invalidation/invalidation_tester.dart

@@ -83,8 +83,9 @@ class InvalidationTester {
     required String from,
     required String to,
     bool isOptional = false,
+    bool outputIsVisible = true,
   }) {
-    final builder = TestBuilder(this, from, [to], isOptional);
+    final builder = TestBuilder(this, from, [to], isOptional, outputIsVisible);
     _builders.add(builder);
     return TestBuilderBuilder(builder);
   }
@@ -185,6 +186,7 @@ class InvalidationTester {
       assets.map((id, content) => MapEntry(id.toString(), content)),
       rootPackage: 'pkg',
       optionalBuilders: _builders.where((b) => b.isOptional).toSet(),
+      visibleOutputBuilders: _builders.where((b) => b.outputIsVisible).toSet(),
       testingBuilderConfig: false,
     );
     final logString = log.toString();
@@ -309,12 +311,16 @@ class TestBuilder implements Builder {
 
   final bool isOptional;
 
+  final bool outputIsVisible;
+
   final InvalidationTester _tester;
 
+  final String from;
+
   /// Extensions of assets that the builder will read.
   ///
   /// The extensions are applied to the primary input asset ID with
-  /// [AssetIdExtension.replaceAllPathExtensions].
+  /// [AssetIdExtension.replaceExtensions].
   List<String> reads = [];
 
   /// Names of assets that the builder will read.
@@ -326,17 +332,22 @@ class TestBuilder implements Builder {
   /// Extensions of assets that the builder will write.
   ///
   /// The extensions are applied to the primary input asset ID with
-  /// [AssetIdExtension.replaceAllPathExtensions].
+  /// [AssetIdExtension.replaceExtensions].
   List<String> writes = [];
 
-  TestBuilder(this._tester, String from, Iterable<String> to, this.isOptional)
-    : buildExtensions = {'$from.dart': to.map((to) => '$to.dart').toList()};
+  TestBuilder(
+    this._tester,
+    this.from,
+    Iterable<String> to,
+    this.isOptional,
+    this.outputIsVisible,
+  ) : buildExtensions = {'$from.dart': to.map((to) => '$to.dart').toList()};
 
   @override
   Future<void> build(BuildStep buildStep) async {
     final content = <String>[];
     for (final read in reads) {
-      final readId = buildStep.inputId.replaceAllPathExtensions(read);
+      final readId = buildStep.inputId.replaceExtensions('$from.dart', read);
       content.add(read.toString());
       if (await buildStep.canRead(readId)) {
         content.add(await buildStep.readAsString(readId));
@@ -353,7 +364,7 @@ class TestBuilder implements Builder {
       await buildStep.resolver.libraryFor(resolve.assetId);
     }
     for (final write in writes) {
-      final writeId = buildStep.inputId.replaceAllPathExtensions(write);
+      final writeId = buildStep.inputId.replaceExtensions('$from.dart', write);
       final outputStrategy =
           _tester._outputStrategies[writeId] ?? OutputStrategy.inputDigest;
       final inputHash = base64.encode(
@@ -371,7 +382,7 @@ class TestBuilder implements Builder {
       }
     }
     for (final write in writes) {
-      final writeId = buildStep.inputId.replaceAllPathExtensions(write);
+      final writeId = buildStep.inputId.replaceExtensions('$from.dart', write);
       if (_tester._failureStrategies[writeId] == FailureStrategy.fail) {
         throw StateError('Failing as requested by test setup.');
       }
@@ -417,8 +428,6 @@ String _assetIdToName(AssetId id) {
 }
 
 extension AssetIdExtension on AssetId {
-  static final extensionsRegexp = RegExp(r'\..*');
-
-  AssetId replaceAllPathExtensions(String extension) =>
-      AssetId(package, path.replaceAll(extensionsRegexp, '') + extension);
+  AssetId replaceExtensions(String from, String to) =>
+      AssetId(package, path.replaceAll(RegExp('$from\$'), to));
 }

build_test/CHANGELOG.md

@@ -5,7 +5,9 @@
 - `resolveSources` and `testBuilder` now do a full `build_runner` build, with
   configuration as much as possible based on the some parameters.
 - Add `testBuilders` to run a test build with multiple builders.
-- Add `optionalBuilder` to `testBuilders` to have some builders be optional.
+- Add `optionalBuilders` to `testBuilders` to have some builders be optional.
+- Add `visibleOutputBuilders` to `testBuilders` to have some builders write
+  their output next to their inputs.
 - Add `testingBuilderConfig` to `testBuilders` to control builder config
   override.
 - Add `resolvers` parameter to `testBuild` and `testBuilders`.

build_test/lib/src/test_builder.dart

@@ -196,6 +196,10 @@ Future<TestBuilderResult> testBuilder(
 /// To mark a builder as optional, add it to [optionalBuilders]. Optional
 /// builders only run if their output is used by a non-optional builder.
 ///
+/// To mark a builder's output as visible, add it to [visibleOutputBuilders].
+/// The builder then writes its outputs next to its input, instead of hidden
+/// under `.dart_tool`.
+///
 /// The default builder config will be overwritten with one that causes the
 /// builder to run for all inputs. To use the default builder config instead,
 /// set [testingBuilderConfig] to `false`.
@@ -221,6 +225,7 @@ Future<TestBuilderResult> testBuilders(
   PackageConfig? packageConfig,
   Resolvers? resolvers,
   Set<Builder> optionalBuilders = const {},
+  Set<Builder> visibleOutputBuilders = const {},
   bool testingBuilderConfig = true,
   TestReaderWriter? readerWriter,
   bool enableLowResourceMode = false,
@@ -322,6 +327,9 @@ Future<TestBuilderResult> testBuilders(
     // didn't change. Skip it to allow testing with preserved state.
     skipBuildScriptCheck: true,
     enableLowResourcesMode: enableLowResourceMode,
+    // If a builder has visible output, it might need to be deleted. Do so
+    // without prompting.
+    deleteFilesByDefault: true,
   );
 
   final buildSeries = await BuildSeries.create(buildOptions, environment, [
@@ -331,6 +339,7 @@ Future<TestBuilderResult> testBuilders(
         [(_) => builder],
         (package) => package.name != r'$sdk',
         isOptional: optionalBuilders.contains(builder),
+        hideOutput: !visibleOutputBuilders.contains(builder),
       ),
   ], {});