move valid input/output checks to the BuildStepImpl and build

lib/src/asset/file_based.dart

@@ -26,29 +26,18 @@ class FileBasedAssetReader implements AssetReader {
 
   @override
   Future<bool> hasInput(AssetId id) async {
-    _checkInput(id);
     return _fileFor(id, packageGraph).exists();
   }
 
   @override
   Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) async {
-    _checkInput(id);
-
     var file = await _fileFor(id, packageGraph);
     if (!await file.exists()) {
       throw new AssetNotFoundException(id);
     }
     return file.readAsString(encoding: encoding);
   }
 
-  /// Checks that [id] is a valid input, and throws an [InvalidInputException]
-  /// if its not.
-  void _checkInput(AssetId id) {
-    if (id.package != packageGraph.root.name && !id.path.startsWith('lib/')) {
-      throw new InvalidInputException(id);
-    }
-  }
-
   /// Searches for all [AssetId]s matching [inputSet]s.
   Stream<AssetId> listAssetIds(Iterable<InputSet> inputSets) async* {
     var seenAssets = new Set<AssetId>();
@@ -88,10 +77,6 @@ class FileBasedAssetWriter implements AssetWriter {
 
   @override
   Future writeAsString(Asset asset, {Encoding encoding: UTF8}) async {
-    if (asset.id.package != packageGraph.root.name) {
-      throw new InvalidOutputException(asset);
-    }
-
     var file = _fileFor(asset.id, packageGraph);
     await file.create(recursive: true);
     await file.writeAsString(asset.stringContents, encoding: encoding);

lib/src/builder/build_step_impl.dart

@@ -10,6 +10,7 @@ import 'package:logging/logging.dart';
 
 import '../analyzer/resolver.dart';
 import '../asset/asset.dart';
+import '../asset/exceptions.dart';
 import '../asset/id.dart';
 import '../asset/reader.dart';
 import '../asset/writer.dart';
@@ -20,6 +21,7 @@ import 'exceptions.dart';
 /// A single step in the build processes. This represents a single input and
 /// its expected and real outputs. It also handles tracking of dependencies.
 class BuildStepImpl implements BuildStep {
+  /// Single `_resolvers` instance for all [BuildStepImpl]s
   static code_transformers.Resolvers _resolvers =
       new code_transformers.Resolvers(code_transformers.dartSdkDirectory);
 
@@ -36,6 +38,7 @@ class BuildStepImpl implements BuildStep {
     _logger ??= new Logger(input.id.toString());
     return _logger;
   }
+
   Logger _logger;
 
   /// The actual outputs of this build step.
@@ -56,22 +59,27 @@ class BuildStepImpl implements BuildStep {
   /// Used internally for writing files.
   final AssetWriter _writer;
 
-  BuildStepImpl(
-      this.input, Iterable<AssetId> expectedOutputs, this._reader, this._writer)
+  /// The current root package, used for input/output validation.
+  final String _rootPackage;
+
+  BuildStepImpl(this.input, Iterable<AssetId> expectedOutputs, this._reader,
+      this._writer, this._rootPackage)
       : expectedOutputs = new List.unmodifiable(expectedOutputs) {
     /// The [input] is always a dependency.
     _dependencies.add(input.id);
   }
 
   /// Checks if an [Asset] by [id] exists as an input for this [BuildStep].
   Future<bool> hasInput(AssetId id) {
+    _checkInput(id);
     _dependencies.add(id);
     return _reader.hasInput(id);
   }
 
   /// Reads an [Asset] by [id] as a [String] using [encoding].
   @override
   Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) {
+    _checkInput(id);
     _dependencies.add(id);
     return _reader.readAsString(id, encoding: encoding);
   }
@@ -83,9 +91,7 @@ class BuildStepImpl implements BuildStep {
   /// [expectedOutputs].
   @override
   void writeAsString(Asset asset, {Encoding encoding: UTF8}) {
-    if (!expectedOutputs.any((id) => id == asset.id)) {
-      throw new UnexpectedOutputException(asset);
-    }
+    _checkOutput(asset);
     _outputs.add(asset);
     var done = _writer.writeAsString(asset, encoding: encoding);
     _outputsCompleted = _outputsCompleted.then((_) => done);
@@ -101,4 +107,23 @@ class BuildStepImpl implements BuildStep {
     await _outputsCompleted;
     await _logger?.clearListeners();
   }
+
+  /// Checks that [id] is a valid input, and throws an [InvalidInputException]
+  /// if its not.
+  void _checkInput(AssetId id) {
+    if (id.package != _rootPackage && !id.path.startsWith('lib/')) {
+      throw new InvalidInputException(id);
+    }
+  }
+
+  /// Checks that [asset] is a valid output, and throws an
+  /// [InvalidOutputException] or [UnexcpectedOutputException] if it's not.
+  void _checkOutput(Asset asset) {
+    if (asset.id.package != _rootPackage) {
+      throw new InvalidOutputException(asset);
+    }
+    if (!expectedOutputs.any((id) => id == asset.id)) {
+      throw new UnexpectedOutputException(asset);
+    }
+  }
 }

lib/src/generate/build.dart

@@ -9,6 +9,7 @@ import 'package:logging/logging.dart';
 import 'package:path/path.dart' as path;
 
 import '../asset/asset.dart';
+import '../asset/exceptions.dart';
 import '../asset/cache.dart';
 import '../asset/file_based.dart';
 import '../asset/id.dart';
@@ -131,6 +132,7 @@ Future _deletePreviousOutputs(List<List<Phase>> phaseGroups) async {
         }
       }
     }
+
     /// Once the group is done, add all outputs so they can be used in the next
     /// phase.
     for (var outputId in groupOutputIds) {
@@ -146,10 +148,10 @@ Future _deletePreviousOutputs(List<List<Phase>> phaseGroups) async {
       'which already exist on disk. This is likely because the `.build` '
       'folder was deleted.');
   var done = false;
-  while(!done) {
+  while (!done) {
     stdout.write('Delete these files (y/n) (or list them (l))?: ');
     var input = stdin.readLineSync();
-    switch(input) {
+    switch (input) {
       case 'y':
         stdout.writeln('Deleting files...');
         await Future.wait(conflictingOutputs.map((output) {
@@ -184,7 +186,7 @@ Future<BuildResult> _runPhases(List<List<Phase>> phaseGroups) async {
       for (var builder in phase.builders) {
         // TODO(jakemac): Optimize, we can run all the builders in a phase
         // at the same time instead of sequentially.
-        await for (var output in _runBuilder(builder, inputs)) {
+        await for (var output in _runBuilder(builder, inputs, allInputs)) {
           groupOutputs.add(output);
           outputs.add(output);
         }
@@ -251,12 +253,23 @@ bool _isValidInput(AssetId input) {
 }
 
 /// Runs [builder] with [inputs] as inputs.
-Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> inputs) async* {
-  for (var input in inputs) {
+Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> primaryInputs,
+    Map<String, Set<AssetId>> allInputs) async* {
+  for (var input in primaryInputs) {
     var expectedOutputs = builder.declareOutputs(input);
+    /// Validate [expectedOutputs].
+    for (var output in expectedOutputs) {
+      if (output.package != _packageGraph.root.name) {
+        throw new InvalidOutputException(new Asset(output, ''));
+      }
+      if (allInputs[output.package]?.contains(output) == true) {
+        throw new InvalidOutputException(new Asset(output, ''));
+      }
+    }
+
     var inputAsset = new Asset(input, await _reader.readAsString(input));
-    var buildStep =
-        new BuildStepImpl(inputAsset, expectedOutputs, _reader, _writer);
+    var buildStep = new BuildStepImpl(
+        inputAsset, expectedOutputs, _reader, _writer, _packageGraph.root.name);
     await builder.build(buildStep);
     await buildStep.complete();
     for (var output in buildStep.outputs) {

lib/src/transformer/transformer.dart

@@ -70,7 +70,8 @@ abstract class BuilderTransformer implements Transformer, DeclaringTransformer {
       }
 
       // Run the build step.
-      var buildStep = new BuildStepImpl(input, expected, reader, writer);
+      var buildStep =
+          new BuildStepImpl(input, expected, reader, writer, input.id.package);
       await builder.build(buildStep);
       await buildStep.complete();
     }));

test/analyzer/resolver_test.dart

@@ -17,14 +17,17 @@ import '../common/common.dart';
 main() {
   var entryPoint = makeAssetId('a|web/main.dart');
   Future validateResolver(
-      {Map<String, String> inputs, validator(Resolver), List messages: const []}) async {
+      {Map<String, String> inputs,
+      validator(Resolver),
+      List messages: const []}) async {
     var writer = new InMemoryAssetWriter();
     var reader = new InMemoryAssetReader(writer.assets);
     var assets = makeAssets(inputs);
     addAssets(assets.values, writer);
 
     var builder = new TestBuilder(validator);
-    var buildStep = new BuildStepImpl(assets[entryPoint], [], reader, writer);
+    var buildStep =
+        new BuildStepImpl(assets[entryPoint], [], reader, writer, 'a');
     var logs = [];
     if (messages != null) {
       buildStep.logger.onRecord.listen(logs.add);
@@ -175,8 +178,7 @@ main() {
         // First from the AST walker
         '[WARNING] a|web/main.dart: $warningMessage "/b.dart"',
         '[WARNING] a|web/main.dart: $warningMessage "/b.dart"',
-      ],
-          validator: (resolver) {
+      ], validator: (resolver) {
         var lib = resolver.getLibrary(entryPoint);
         expect(lib.importedLibraries.length, 1);
       });

test/asset/file_based_test.dart

@@ -26,12 +26,8 @@ main() {
           'world\n');
     });
 
-    test('can only read package dependency files in the lib dir', () async {
+    test('can read package dependency files in the lib dir', () async {
       expect(await reader.readAsString(makeAssetId('a|lib/a.txt')), 'A\n');
-      expect(reader.readAsString(makeAssetId('a|web/a.txt')),
-          throwsA(invalidInputException));
-      expect(reader.readAsString(makeAssetId('a|a.txt')),
-          throwsA(invalidInputException));
     });
 
     test('can check for existence of any application package files', () async {
@@ -46,16 +42,10 @@ main() {
           await reader.hasInput(makeAssetId('basic_pkg|lib/a.txt')), isFalse);
     });
 
-    test('can only check for existence of package dependency files in lib',
+    test('can check for existence of package dependency files in lib',
         () async {
       expect(await reader.hasInput(makeAssetId('a|lib/a.txt')), isTrue);
       expect(await reader.hasInput(makeAssetId('a|lib/b.txt')), isFalse);
-      expect(reader.hasInput(makeAssetId('a|web/a.txt')),
-          throwsA(invalidInputException));
-      expect(reader.hasInput(makeAssetId('a|a.txt')),
-          throwsA(invalidInputException));
-      expect(reader.hasInput(makeAssetId('foo|bar.txt')),
-          throwsA(invalidInputException));
     });
 
     test('throws when attempting to read a non-existent file', () async {
@@ -105,25 +95,5 @@ main() {
       await writer.delete(asset.id);
       expect(await file.exists(), isFalse);
     });
-
-    test('can\'t output files in package dependencies', () async {
-      var asset = makeAsset('a|test.txt');
-      expect(writer.writeAsString(asset), throwsA(invalidOutputException));
-    });
-
-    test('can\'t output files in arbitrary packages', () async {
-      var asset = makeAsset('foo|bar.txt');
-      expect(writer.writeAsString(asset), throwsA(invalidOutputException));
-    });
-
-    test('can\'t delete files in package dependencies', () async {
-      var id = makeAssetId('a|test.txt');
-      expect(writer.delete(id), throws);
-    });
-
-    test('can\'t delete files in arbitrary dependencies', () async {
-      var id = makeAssetId('foo|test.txt');
-      expect(writer.delete(id), throws);
-    });
   });
 }

test/builder/build_step_impl_test.dart

@@ -23,7 +23,8 @@ main() {
         reader = new StubAssetReader();
         writer = new StubAssetWriter();
         primary = makeAsset();
-        buildStep = new BuildStepImpl(primary, [], reader, writer);
+        buildStep =
+            new BuildStepImpl(primary, [], reader, writer, primary.id.package);
       });
 
       test('tracks dependencies on read', () async {
@@ -53,7 +54,8 @@ main() {
       test('tracks outputs', () async {
         var a1 = makeAsset();
         var a2 = makeAsset();
-        buildStep = new BuildStepImpl(primary, [a1.id, a2.id], reader, writer);
+        buildStep = new BuildStepImpl(
+            primary, [a1.id, a2.id], reader, writer, primary.id.package);
 
         buildStep.writeAsString(a1);
         expect(buildStep.outputs, [a1]);
@@ -86,6 +88,33 @@ main() {
           '[SEVERE] ${primary.id}: goodbye',
         ]);
       });
+
+      test('hasInput throws invalidInputExceptions', () async {
+        expect(() => buildStep.hasInput(makeAssetId('b|web/a.txt')),
+            throwsA(invalidInputException));
+        expect(() => buildStep.hasInput(makeAssetId('b|a.txt')),
+            throwsA(invalidInputException));
+        expect(() => buildStep.hasInput(makeAssetId('foo|bar.txt')),
+            throwsA(invalidInputException));
+      });
+
+      test('readAsString throws InvalidInputExceptions', () async {
+        expect(() => buildStep.readAsString(makeAssetId('b|web/a.txt')),
+            throwsA(invalidInputException));
+        expect(() => buildStep.readAsString(makeAssetId('b|a.txt')),
+            throwsA(invalidInputException));
+        expect(() => buildStep.readAsString(makeAssetId('foo|bar.txt')),
+            throwsA(invalidInputException));
+      });
+
+      test('writeAsString throws InvalidOutputExceptions', () async {
+        var a1 = makeAsset('b|test.txt');
+        expect(
+            () => buildStep.writeAsString(a1), throwsA(invalidOutputException));
+        var a2 = makeAsset('foo|bar.txt');
+        expect(
+            () => buildStep.writeAsString(a2), throwsA(invalidOutputException));
+      });
     });
 
     group('with in memory file system', () {
@@ -109,8 +138,8 @@ main() {
         });
         addAssets(inputs.values, writer);
         var outputId = new AssetId.parse('$primary.combined');
-        var buildStep = new BuildStepImpl(
-            inputs[new AssetId.parse(primary)], [outputId], reader, writer);
+        var buildStep = new BuildStepImpl(inputs[new AssetId.parse(primary)],
+            [outputId], reader, writer, 'a');
 
         await fileCombiner.build(buildStep);
         await buildStep.complete();
@@ -140,8 +169,8 @@ main() {
           addAssets(inputs.values, writer);
 
           var primary = makeAssetId('a|web/a.dart');
-          var buildStep =
-              new BuildStepImpl(inputs[primary], [], reader, writer);
+          var buildStep = new BuildStepImpl(
+              inputs[primary], [], reader, writer, primary.package);
           var resolver = await buildStep.resolve(primary);
 
           var aLib = resolver.getLibrary(primary);