dart migrate: Only opt out files with a comment if they appear in files-to-process

Change-Id: I3cb1bedaa4a03eca89e29528c554488955f2bb04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/180083
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

Author: srawlins

pkg/nnbd_migration/lib/migration_cli.dart

@@ -591,7 +591,10 @@ class MigrationCliRunner implements DartFixListenerClient {
   Set<String> computePathsToProcess(DriverBasedAnalysisContext context) =>
       context.contextRoot
           .analyzedFiles()
-          .where((s) => s.endsWith('.dart'))
+          .where((s) =>
+              s.endsWith('.dart') &&
+              // Any file may have been deleted since its initial analysis.
+              resourceProvider.getFile(s).exists)
           .toSet();
 
   NonNullableFix createNonNullableFix(
@@ -1159,6 +1162,8 @@ class _FixCodeProcessor extends Object {
     _progressBar.complete();
     _migrationCli.logger.stdout(_migrationCli.ansi
         .emphasized('Compiling instrumentation information...'));
+    // Update the tasks paths-to-process, in case of new or deleted files.
+    _task.pathsToProcess = pathsToProcess;
     var state = await _task.finish();
     _task.processPackage(context.contextRoot.root, state.neededPackages);
     if (_migrationCli.options.webPreview) {

pkg/nnbd_migration/lib/src/front_end/info_builder.dart

@@ -59,6 +59,9 @@ class InfoBuilder {
   /// migrated.
   final bool Function(String) shouldBeMigratedFunction;
 
+  /// The set of files which are being considered for migration.
+  final Iterable<String> _pathsToProcess;
+
   /// Initialize a newly created builder.
   InfoBuilder(
       this.provider,
@@ -68,7 +71,8 @@ class InfoBuilder {
       this.migration,
       this.nodeMapper,
       this._logger,
-      this.shouldBeMigratedFunction);
+      this.shouldBeMigratedFunction,
+      this._pathsToProcess);
 
   /// The provider used to get information about libraries.
   DriverProviderImpl get driverProvider => listener.server;
@@ -79,11 +83,18 @@ class InfoBuilder {
     var sourceInfoMap = info.sourceInformation;
     Set<UnitInfo> units =
         SplayTreeSet<UnitInfo>((u1, u2) => u1.path.compareTo(u2.path));
-    var progressBar = ProgressBar(_logger, sourceInfoMap.length);
 
-    for (var source in sourceInfoMap.keys) {
+    // Collect all of the sources for which we have [SourceInformation], as well
+    // as all files which are being "processed" during this migration, which may
+    // include already migrated files.
+    var sources = {
+      ...sourceInfoMap.keys.map((source) => source.fullName),
+      ..._pathsToProcess,
+    };
+    var progressBar = ProgressBar(_logger, sources.length);
+
+    for (var filePath in sources) {
       progressBar.tick();
-      var filePath = source.fullName;
       var session = driverProvider.getAnalysisSession(filePath);
       if (!session.getFile(filePath).isPart) {
         var result = await session.getResolvedLibrary(filePath);
@@ -102,7 +113,7 @@ class InfoBuilder {
           sourceInfo ??= SourceInformation();
           var edit = listener.sourceChange.getFileEdit(unitResult.path);
           var unit = _explainUnit(sourceInfo, unitResult, edit);
-          if (provider.pathContext.isWithin(includedPath, unitResult.path)) {
+          if (_pathsToProcess.contains(unitResult.path)) {
             units.add(unit);
           }
         }

pkg/nnbd_migration/lib/src/front_end/migration_state.dart

@@ -77,7 +77,7 @@ class MigrationState {
   }
 
   /// Refresh the state of the migration after the migration has been updated.
-  Future<void> refresh(Logger logger) async {
+  Future<void> refresh(Logger logger, Iterable<String> pathsToProcess) async {
     assert(!hasBeenApplied);
     var provider = listener.server.resourceProvider;
     var infoBuilder = InfoBuilder(
@@ -88,7 +88,8 @@ class MigrationState {
         migration,
         nodeMapper,
         logger,
-        shouldBeMigratedFunction);
+        shouldBeMigratedFunction,
+        pathsToProcess);
     var unitInfos = await infoBuilder.explainMigration();
     var pathContext = provider.pathContext;
     migrationInfo = MigrationInfo(

pkg/nnbd_migration/lib/src/front_end/non_nullable_fix.dart

@@ -79,6 +79,9 @@ class NonNullableFix {
   /// migrated.
   final bool Function(String) shouldBeMigratedFunction;
 
+  /// The set of files which are being considered for migration.
+  Iterable<String> pathsToProcess;
+
   /// Completes when the server has been shutdown.
   Completer<void> serverIsShutdown;
 
@@ -116,7 +119,7 @@ class NonNullableFix {
     var neededPackages = migration.finish();
     final state = MigrationState(migration, includedRoot, listener,
         instrumentationListener, neededPackages, shouldBeMigratedFunction);
-    await state.refresh(_logger);
+    await state.refresh(_logger, pathsToProcess);
     return state;
   }
 

pkg/nnbd_migration/test/front_end/nnbd_migration_test_base.dart

@@ -159,6 +159,8 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
     var includedRoot = resourceProvider.pathContext.dirname(testFile);
     await _buildMigrationInfo([testFile],
         includedRoot: includedRoot,
+        shouldBeMigratedFunction: (String path) => true,
+        pathsToProcess: [testFile],
         removeViaComments: removeViaComments,
         warnOnWeakCode: warnOnWeakCode);
   }
@@ -190,13 +192,20 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
   ///
   /// Returns the singular [UnitInfo] which was built.
   Future<List<UnitInfo>> buildInfoForTestFiles(Map<String, String> files,
-      {String includedRoot}) async {
+      {@required String includedRoot,
+      bool Function(String) shouldBeMigratedFunction,
+      Iterable<String> pathsToProcess}) async {
+    shouldBeMigratedFunction ??= (String path) => true;
     var testPaths = <String>[];
     files.forEach((String path, String content) {
       newFile(path, content: content);
       testPaths.add(path);
     });
-    await _buildMigrationInfo(testPaths, includedRoot: includedRoot);
+    pathsToProcess ??= testPaths;
+    await _buildMigrationInfo(testPaths,
+        includedRoot: includedRoot,
+        shouldBeMigratedFunction: shouldBeMigratedFunction,
+        pathsToProcess: pathsToProcess);
     // Ignore info for dart:core.
     var filteredInfos = [
       for (var info in infos)
@@ -212,8 +221,10 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
 
   /// Uses the InfoBuilder to build information for files at [testPaths], which
   /// should all share a common parent directory, [includedRoot].
-  Future<void> _buildMigrationInfo(List<String> testPaths,
-      {String includedRoot,
+  Future<void> _buildMigrationInfo(Iterable<String> testPaths,
+      {@required String includedRoot,
+      @required bool Function(String) shouldBeMigratedFunction,
+      @required Iterable<String> pathsToProcess,
       bool removeViaComments = true,
       bool warnOnWeakCode = false}) async {
     // Compute the analysis results.
@@ -243,8 +254,16 @@ class NnbdMigrationTestBase extends AbstractAnalysisTest {
     // Build the migration info.
     var info = instrumentationListener.data;
     var logger = TestLogger(false);
-    var builder = InfoBuilder(resourceProvider, includedRoot, info, listener,
-        migration, nodeMapper, logger, (String path) => true);
+    var builder = InfoBuilder(
+        resourceProvider,
+        includedRoot,
+        info,
+        listener,
+        migration,
+        nodeMapper,
+        logger,
+        shouldBeMigratedFunction,
+        pathsToProcess);
     infos = await builder.explainMigration();
   }
 }

pkg/nnbd_migration/test/preview/preview_site_test.dart

@@ -269,13 +269,20 @@ class PreviewSiteWithEngineTest extends NnbdMigrationTestBase
     with ResourceProviderMixin, PreviewSiteTestMixin {
   MigrationInfo migrationInfo;
 
-  Future<void> setUpMigrationInfo(Map<String, String> files) async {
-    await buildInfoForTestFiles(files, includedRoot: projectPath);
+  Future<void> setUpMigrationInfo(Map<String, String> files,
+      {bool Function(String) shouldBeMigratedFunction,
+      Iterable<String> pathsToProcess}) async {
+    shouldBeMigratedFunction ??= (String path) => true;
+    pathsToProcess ??= files.keys;
+    await buildInfoForTestFiles(files,
+        includedRoot: projectPath,
+        shouldBeMigratedFunction: shouldBeMigratedFunction,
+        pathsToProcess: pathsToProcess);
     dartfixListener = DartFixListener(null, ListenerClient());
     migrationInfo =
         MigrationInfo(infos, {}, resourceProvider.pathContext, projectPath);
     state = MigrationState(
-        null, null, dartfixListener, null, {}, (String path) => true);
+        null, null, dartfixListener, null, {}, shouldBeMigratedFunction);
     nodeMapper = state.nodeMapper;
     state.pathMapper = PathMapper(resourceProvider);
     state.migrationInfo = migrationInfo;
@@ -469,6 +476,32 @@ Opted 1 file out of null safety with a new Dart language version comment:
 '''));
   }
 
+  void test_applyMigration_doNotOptOutFileNotInPathsToProcess() async {
+    final pathA = convertPath('$projectPath/lib/a.dart');
+    final pathB = convertPath('$projectPath/lib/b.dart');
+    final content = 'void main() {}';
+    await setUpMigrationInfo({pathA: content, pathB: content},
+        // Neither [pathA] nor [[pathB] should be migrated.
+        shouldBeMigratedFunction: (String path) => false,
+        pathsToProcess: [pathA]);
+    site.unitInfoMap[pathA] = UnitInfo(pathA)
+      ..diskContent = content
+      ..wasExplicitlyOptedOut = false
+      ..migrationStatus = UnitMigrationStatus.optingOut;
+    site.unitInfoMap[pathB] = UnitInfo(pathB)
+      ..diskContent = content
+      ..wasExplicitlyOptedOut = false
+      ..migrationStatus = UnitMigrationStatus.optingOut;
+    var navigationTree =
+        NavigationTreeRenderer(migrationInfo, state.pathMapper).render();
+    site.performApply(navigationTree);
+    expect(getFile(pathA).readAsStringSync(), '''
+// @dart=2.9
+
+void main() {}''');
+    expect(getFile(pathB).readAsStringSync(), 'void main() {}');
+  }
+
   void test_applyMigration_optOutOne_migrateAnother() async {
     final pathA = convertPath('$projectPath/lib/a.dart');
     final pathB = convertPath('$projectPath/lib/b.dart');