Migration: allow the client to specify which files to analyze/migrate.

Change-Id: Ie49f003640342a8c0d26b4f509a9268200955dda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151302
Reviewed-by: Janice Collins <[email protected]>

Author: stereotype441

pkg/nnbd_migration/lib/migration_cli.dart

@@ -498,6 +498,22 @@ class MigrationCliRunner {
     return stream.first;
   }
 
+  /// Computes the set of file paths that should be analyzed by the migration
+  /// engine.  May be overridden by a derived class.
+  ///
+  /// All files to be migrated must be included in the returned set.  It is
+  /// permissible for the set to contain additional files that could help the
+  /// migration tool build up a more complete nullability graph (for example
+  /// generated files, or usages of the code-to-be-migrated by one one of its
+  /// clients).
+  ///
+  /// By default returns the set of all `.dart` files contained in the context.
+  Set<String> computePathsToProcess(DriverBasedAnalysisContext context) =>
+      context.contextRoot
+          .analyzedFiles()
+          .where((s) => s.endsWith('.dart'))
+          .toSet();
+
   NonNullableFix createNonNullableFix(DartFixListener listener,
       ResourceProvider resourceProvider, LineInfo getLineInfo(String path),
       {List<String> included = const <String>[],
@@ -609,6 +625,22 @@ Use this interactive web view to review, improve, or apply the results.
     }
   }
 
+  /// Determines whether a migrated version of the file at [path] should be
+  /// output by the migration too.  May be overridden by a derived class.
+  ///
+  /// This method should return `false` for files that are being considered by
+  /// the migration tool for information only (for example generated files, or
+  /// usages of the code-to-be-migrated by one one of its clients).
+  ///
+  /// By default returns `true` if the file is contained within the context
+  /// root.  This means that if a client overrides [computePathsToProcess] to
+  /// return additional paths that aren't inside the user's project, but doesn't
+  /// override this method, then those additional paths will be analyzed but not
+  /// migrated.
+  bool shouldBeMigrated(DriverBasedAnalysisContext context, String path) {
+    return context.contextRoot.isAnalyzed(path);
+  }
+
   /// Perform the indicated source edits to the given source, returning the
   /// resulting transformed text.
   String _applyEdits(SourceFileEdit sourceFileEdit, String source) {
@@ -867,7 +899,7 @@ class _FixCodeProcessor extends Object {
   NonNullableFix nonNullableFixTask;
 
   _FixCodeProcessor(this.context, this._migrationCli)
-      : pathsToProcess = _computePathsToProcess(context);
+      : pathsToProcess = _migrationCli.computePathsToProcess(context);
 
   bool get isPreviewServerRunnning =>
       nonNullableFixTask?.isPreviewServerRunning ?? false;
@@ -877,7 +909,7 @@ class _FixCodeProcessor extends Object {
 
   void prepareToRerun() {
     var driver = context.driver;
-    pathsToProcess = _computePathsToProcess(context);
+    pathsToProcess = _migrationCli.computePathsToProcess(context);
     pathsToProcess.forEach(driver.changeFile);
   }
 
@@ -898,8 +930,7 @@ class _FixCodeProcessor extends Object {
           var result = await driver.getResolvedLibrary(path);
           if (result != null) {
             for (var unit in result.units) {
-              if (pathsToProcess.contains(unit.path) &&
-                  !pathsProcessed.contains(unit.path)) {
+              if (!pathsProcessed.contains(unit.path)) {
                 await process(unit);
                 pathsProcessed.add(unit.path);
               }
@@ -960,7 +991,9 @@ class _FixCodeProcessor extends Object {
     });
     await processResources((ResolvedUnitResult result) async {
       _progressBar.tick();
-      await _task.finalizeUnit(result);
+      if (_migrationCli.shouldBeMigrated(context, result.path)) {
+        await _task.finalizeUnit(result);
+      }
     });
     var state = await _task.finish();
     if (_migrationCli.options.webPreview) {
@@ -970,13 +1003,6 @@ class _FixCodeProcessor extends Object {
 
     return nonNullableFixTask.previewUrls;
   }
-
-  static Set<String> _computePathsToProcess(
-          DriverBasedAnalysisContext context) =>
-      context.contextRoot
-          .analyzedFiles()
-          .where((s) => s.endsWith('.dart'))
-          .toSet();
 }
 
 /// Given a Logger and an analysis issue, render the issue to the logger.

pkg/nnbd_migration/test/migration_cli_test.dart

@@ -11,6 +11,7 @@ import 'package:analyzer/file_system/file_system.dart' show ResourceProvider;
 import 'package:analyzer/file_system/memory_file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
 import 'package:analyzer/source/line_info.dart';
+import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/test_utilities/mock_sdk.dart' as mock_sdk;
 import 'package:args/args.dart';
 import 'package:cli_util/cli_logging.dart';
@@ -112,6 +113,10 @@ class _MigrationCliRunner extends MigrationCliRunner {
     _runWhilePreviewServerActive = null;
   }
 
+  @override
+  Set<String> computePathsToProcess(DriverBasedAnalysisContext context) =>
+      cli._test.overridePathsToProcess ?? super.computePathsToProcess(context);
+
   @override
   NonNullableFix createNonNullableFix(DartFixListener listener,
       ResourceProvider resourceProvider, LineInfo getLineInfo(String path),
@@ -139,6 +144,11 @@ class _MigrationCliRunner extends MigrationCliRunner {
       fail('Preview server never started');
     }
   }
+
+  @override
+  bool shouldBeMigrated(DriverBasedAnalysisContext context, String path) =>
+      cli._test.overrideShouldBeMigrated?.call(path) ??
+      super.shouldBeMigrated(context, path);
 }
 
 abstract class _MigrationCliTestBase {
@@ -148,6 +158,12 @@ abstract class _MigrationCliTestBase {
   /// encounters a reference to the `print` function.
   bool injectArtificialException = false;
 
+  /// If non-null, this is injected as the return value for
+  /// [_MigrationCliRunner.computePathsToProcess].
+  Set<String> overridePathsToProcess;
+
+  bool Function(String) overrideShouldBeMigrated;
+
   void set logger(_TestLogger logger);
 
   _MockProcessManager get processManager;
@@ -690,6 +706,52 @@ int? f() => null
     assertProjectContents(projectDir, projectContents);
   }
 
+  test_lifecycle_override_paths() async {
+    Map<String, String> makeProject({bool migrated = false}) {
+      var projectContents = simpleProject(migrated: migrated);
+      projectContents['lib/test.dart'] = '''
+import 'skip.dart';
+import 'analyze_but_do_not_migrate.dart';
+void f(int x) {}
+void g(int${migrated ? '?' : ''} x) {}
+void h(int${migrated ? '?' : ''} x) {}
+void call_h() => h(null);
+''';
+      projectContents['lib/skip.dart'] = '''
+import 'test.dart';
+void call_f() => f(null);
+''';
+      projectContents['lib/analyze_but_do_not_migrate.dart'] = '''
+import 'test.dart';
+void call_g() => g(null);
+''';
+      return projectContents;
+    }
+
+    var projectContents = makeProject();
+    var projectDir = await createProjectDir(projectContents);
+    var testPath =
+        resourceProvider.pathContext.join(projectDir, 'lib', 'test.dart');
+    var analyzeButDoNotMigratePath = resourceProvider.pathContext
+        .join(projectDir, 'lib', 'analyze_but_do_not_migrate.dart');
+    overridePathsToProcess = {testPath, analyzeButDoNotMigratePath};
+    overrideShouldBeMigrated = (path) => path == testPath;
+    var cliRunner = _createCli().decodeCommandLineArgs(
+        _parseArgs(['--no-web-preview', '--apply-changes', projectDir]));
+    await cliRunner.run();
+    assertNormalExit(cliRunner);
+    // Check that a summary was printed
+    expect(logger.stdoutBuffer.toString(), contains('Applying changes'));
+    // And that it refers to test.dart and pubspec.yaml
+    expect(logger.stdoutBuffer.toString(), contains('test.dart'));
+    expect(logger.stdoutBuffer.toString(), contains('pubspec.yaml'));
+    // And that it does not tell the user they can rerun with `--apply-changes`
+    expect(logger.stdoutBuffer.toString(), isNot(contains('--apply-changes')));
+    // Changes should have been made only to test.dart, and only accounting for
+    // the calls coming from analyze_but_do_not_migrate.dart and test.dart
+    assertProjectContents(projectDir, makeProject(migrated: true));
+  }
+
   test_lifecycle_preview() async {
     var projectContents = simpleProject();
     var projectDir = await createProjectDir(projectContents);