Migration: improve error/warning text when there are analysis errors.

Fixes #44144

Bug: #44144
Change-Id: I54ef9ca8f38335df2082d730ba558fd407a07767
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171643
Commit-Queue: Kathy Walrath <[email protected]>
Reviewed-by: Kathy Walrath <[email protected]>
Auto-Submit: Paul Berry <[email protected]>

Author: stereotype441

pkg/dartdev/test/commands/migrate_test.dart

@@ -12,7 +12,8 @@ void main() {
 }
 
 void defineMigrateTests() {
-  final didYouForgetToRunPubGet = contains('Did you forget to run "pub get"?');
+  final runPubGet = contains('Run `dart pub get`');
+  final setLowerSdkConstraint = contains('Set the lower SDK constraint');
 
   TestProject p;
 
@@ -60,14 +61,16 @@ void defineMigrateTests() {
     var result = p.runSync('migrate', [p.dirPath]);
     expect(result.exitCode, 1);
     expect(result.stderr, isEmpty);
-    expect(result.stdout, didYouForgetToRunPubGet);
+    expect(result.stdout, runPubGet);
+    expect(result.stdout, isNot(setLowerSdkConstraint));
   });
 
   test('non-pub-related error', () {
     p = project(mainSrc: 'var missing = "semicolon"\n');
     var result = p.runSync('migrate', [p.dirPath]);
     expect(result.exitCode, 1);
     expect(result.stderr, isEmpty);
-    expect(result.stdout, isNot(didYouForgetToRunPubGet));
+    expect(result.stdout, runPubGet);
+    expect(result.stdout, setLowerSdkConstraint);
   });
 }

pkg/nnbd_migration/lib/migration_cli.dart

@@ -861,35 +861,41 @@ Exception details:
     for (AnalysisError error in analysisResult.errors) {
       renderer.render(error);
     }
-
     logger.stdout('');
-    logger.stdout('Note: analysis errors will result in erroneous migration '
-        'suggestions.');
-
     _hasAnalysisErrors = true;
+
     if (options.ignoreErrors) {
+      logger.stdout('Note: analysis errors will result in erroneous migration '
+          'suggestions.');
       logger.stdout('Continuing with migration suggestions due to the use of '
           '--${CommandLineOptions.ignoreErrorsFlag}.');
     } else {
       // Fail with how to continue.
+      logger.stdout("The migration tool didn't start, due to analysis errors.");
       logger.stdout('');
       if (analysisResult.hasImportErrors) {
-        logger
-            .stdout('Unresolved URIs found.  Did you forget to run "pub get"?');
-        logger.stdout('');
-      }
-      if (analysisResult.allSourcesAlreadyMigrated) {
         logger.stdout('''
-All files appear to have null safety already enabled.  Did you update the
-language version prior to running "dart migrate"?  If so, you need to un-do this
-(and re-run "pub get") prior to performing the migration.
+The following steps might fix your problem:
+1. Run `dart pub get`.
+2. Try running `dart migrate` again.
+''');
+      } else if (analysisResult.allSourcesAlreadyMigrated) {
+        logger.stdout('''
+The following steps might fix your problem:
+1. Set the lower SDK constraint (in pubspec.yaml) to a version before 2.12.
+2. Run `dart pub get`.
+3. Try running `dart migrate` again.
+''');
+      } else {
+        const ignoreErrors = CommandLineOptions.ignoreErrorsFlag;
+        logger.stdout('''
+We recommend fixing the analysis issues before running `dart migrate`.
+Alternatively, you can run `dart migrate --$ignoreErrors`, but you might
+get erroneous migration suggestions.
 ''');
-        logger.stdout('');
       }
       logger.stdout(
-          'Please fix the analysis issues (or, force generation of migration '
-          'suggestions by re-running with '
-          '--${CommandLineOptions.ignoreErrorsFlag}).');
+          'More information: https://dart.dev/go/null-safety-migration');
     }
   }
 

pkg/nnbd_migration/test/migration_cli_test.dart

@@ -745,15 +745,9 @@ int f() => null
         output,
         contains("error • Expected to find ';' at lib${sep}test.dart:1:12 • "
             '(expected_token)'));
-    expect(
-        output,
-        contains(
-            'analysis errors will result in erroneous migration suggestions'));
-    expect(output, contains('Please fix the analysis issues'));
-    expect(
-        output,
-        isNot(
-            contains('All files appear to have null safety already enabled')));
+    expect(output, contains('erroneous migration suggestions'));
+    expect(output, contains('We recommend fixing the analysis issues'));
+    expect(output, isNot(contains('Set the lower SDK constraint')));
   }
 
   test_lifecycle_ignore_errors_enable() async {
@@ -1488,17 +1482,10 @@ int f() => null;
     var output = logger.stdoutBuffer.toString();
     expect(output, contains('1 analysis issue found'));
     expect(output, contains('uri_does_not_exist'));
-    expect(
-        output,
-        contains(
-            'analysis errors will result in erroneous migration suggestions'));
-    expect(output,
-        contains('Unresolved URIs found.  Did you forget to run "pub get"?'));
-    expect(output, contains('Please fix the analysis issues'));
-    expect(
-        output,
-        isNot(
-            contains('All files appear to have null safety already enabled')));
+    expect(output, isNot(contains('erroneous migration suggestions')));
+    expect(output, contains('Run `dart pub get`'));
+    expect(output, contains('Try running `dart migrate` again'));
+    expect(output, isNot(contains('Set the lower SDK constraint')));
   }
 
   test_migrate_path_absolute() {
@@ -1912,9 +1899,7 @@ environment:
         errorOutput,
         contains("A value of type 'Null' can't be returned from function 'f' "
             "because it has a return type of 'int'"));
-    expect(errorOutput, contains('''
-All files appear to have null safety already enabled.  Did you update the
-language version prior to running "dart migrate"?'''));
+    expect(errorOutput, contains('Set the lower SDK constraint'));
   }
 
   String _getHelpText({@required bool verbose}) {