[ CLI ] Lockdown --no-sound-null-safety in dart compile subcommands

Related to #51513

Change-Id: Ie798af562133b933a76351b9e7244870ea49e6dd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/285380
Commit-Queue: Ben Konyi <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>

Author: bkonyi

pkg/dartdev/lib/src/commands/compile.dart

@@ -25,6 +25,7 @@ class Option {
   final String help;
   final String? abbr;
   final String? defaultsTo;
+  final bool? flagDefaultsTo;
   final String? valueHelp;
   final List<String>? allowed;
   final Map<String, String>? allowedHelp;
@@ -34,6 +35,7 @@ class Option {
     required this.help,
     this.abbr,
     this.defaultsTo,
+    this.flagDefaultsTo,
     this.valueHelp,
     this.allowed,
     this.allowedHelp,
@@ -127,9 +129,10 @@ class CompileSnapshotCommand extends CompileSubcommandCommand {
         abbr: defineOption.abbr,
         valueHelp: defineOption.valueHelp,
       )
-      ..addFlag('sound-null-safety',
-          help: 'Respect the nullability of types at runtime.',
-          defaultsTo: true)
+      ..addFlag(soundNullSafetyOption.flag,
+          help: soundNullSafetyOption.help,
+          defaultsTo: soundNullSafetyOption.flagDefaultsTo,
+          hide: true)
       ..addExperimentalFlags(verbose: verbose);
   }
 
@@ -188,6 +191,9 @@ class CompileSnapshotCommand extends CompileSubcommandCommand {
 
     final bool soundNullSafety = args['sound-null-safety'];
     if (!soundNullSafety) {
+      if (!shouldAllowNoSoundNullSafety()) {
+        return compileErrorExitCode;
+      }
       buildArgs.add('--no-sound-null-safety');
     }
 
@@ -269,9 +275,10 @@ class CompileNativeCommand extends CompileSubcommandCommand {
         valueHelp: packagesOption.valueHelp,
         help: packagesOption.help,
       )
-      ..addFlag('sound-null-safety',
-          help: 'Respect the nullability of types at runtime.',
-          defaultsTo: true)
+      ..addFlag(soundNullSafetyOption.flag,
+          help: soundNullSafetyOption.help,
+          defaultsTo: soundNullSafetyOption.flagDefaultsTo,
+          hide: true)
       ..addOption('save-debugging-info', abbr: 'S', valueHelp: 'path', help: '''
 Remove debugging information from the output and save it separately to the specified file.
 <path> can be relative or absolute.''')
@@ -315,6 +322,10 @@ Remove debugging information from the output and save it separately to the speci
       return -1;
     }
 
+    if (!args['sound-null-safety'] && !shouldAllowNoSoundNullSafety()) {
+      return compileErrorExitCode;
+    }
+
     try {
       await generateNative(
         kind: format,
@@ -362,6 +373,11 @@ Sets the verbosity level of the compilation.
     allowed: Verbosity.allowedValues,
     allowedHelp: Verbosity.allowedValuesHelp,
   );
+  final soundNullSafetyOption = Option(
+    flag: 'sound-null-safety',
+    help: 'DEPRECATED: Respect the nullability of types at runtime.',
+    flagDefaultsTo: true,
+  );
 
   late final Option defineOption;
   late final Option packagesOption;
@@ -385,6 +401,23 @@ For example: dart compile $name -Da=1,b=2 main.dart''',
 <path> can be relative or absolute.
 For example: dart compile $name --packages=/tmp/pkgs.json main.dart'''),
         super(name, description, verbose, hidden: hidden);
+
+  bool shouldAllowNoSoundNullSafety() {
+    // We need to maintain support for generating AOT snapshots and kernel
+    // files with no-sound-null-safety internal Flutter aplications are
+    // fully null-safe.
+    //
+    // See https://github.com/dart-lang/sdk/issues/51513 for context.
+    if (name == CompileNativeCommand.aotSnapshotCmdName ||
+        name == CompileSnapshotCommand.kernelCmdName) {
+      log.stdout(
+          'Warning: the flag --no-sound-null-safety is deprecated and pending removal.');
+      return true;
+    }
+    log.stdout(
+        'Error: the flag --no-sound-null-safety is not supported in Dart 3.');
+    return false;
+  }
 }
 
 class CompileCommand extends DartdevCommand {

pkg/dartdev/test/commands/compile_test.dart

@@ -23,6 +23,10 @@ void main() {
 const String soundNullSafetyMessage = 'Info: Compiling with sound null safety';
 const String unsoundNullSafetyMessage =
     'Info: Compiling without sound null safety';
+const String unsoundNullSafetyError =
+    'Error: the flag --no-sound-null-safety is not supported in Dart 3.';
+const String unsoundNullSafetyWarning =
+    'Warning: the flag --no-sound-null-safety is deprecated and pending removal.';
 
 void defineCompileTests() {
   final isRunningOnIA32 = Platform.version.contains('ia32');
@@ -538,11 +542,10 @@ void main() {}
       ],
     );
 
-    expect(result.stdout, contains(unsoundNullSafetyMessage));
+    expect(result.stdout, contains(unsoundNullSafetyError));
     expect(result.stderr, isEmpty);
-    expect(result.exitCode, 0);
-    expect(File(outFile).existsSync(), true,
-        reason: 'File not found: $outFile');
+    expect(result.exitCode, 64);
+    expect(File(outFile).existsSync(), false, reason: 'File found: $outFile');
   }, skip: isRunningOnIA32);
 
   test('Compile and run exe with --sound-null-safety', () async {
@@ -579,40 +582,6 @@ void main() {}
     expect(result.stdout, contains('sound'));
   }, skip: isRunningOnIA32);
 
-  test('Compile and run exe with --no-sound-null-safety', () async {
-    final p = project(mainSrc: '''void main() {
-      print((<int?>[] is List<int>) ? 'unsound' : 'oh no');
-    }''');
-    final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath));
-    final outFile = path.canonicalize(path.join(p.dirPath, 'myexe'));
-
-    var result = await p.run(
-      [
-        'compile',
-        'exe',
-        '--no-sound-null-safety',
-        '-o',
-        outFile,
-        inFile,
-      ],
-    );
-
-    expect(result.stdout, contains(unsoundNullSafetyMessage));
-    expect(result.stderr, isEmpty);
-    expect(result.exitCode, 0);
-    expect(File(outFile).existsSync(), true,
-        reason: 'File not found: $outFile');
-
-    result = Process.runSync(
-      outFile,
-      [],
-    );
-
-    expect(result.stdout, contains('unsound'));
-    expect(result.stderr, isEmpty);
-    expect(result.exitCode, 0);
-  }, skip: isRunningOnIA32);
-
   test('Compile exe without info', () async {
     final p = project(mainSrc: '''void main() {}''');
     final inFile = path.canonicalize(path.join(p.dirPath, p.relativeFilePath));
@@ -804,6 +773,7 @@ void main() {}
     );
 
     expect(result.stdout, contains(unsoundNullSafetyMessage));
+    expect(result.stdout, contains(unsoundNullSafetyWarning));
     expect(result.stderr, isEmpty);
     expect(result.exitCode, 0);
     expect(File(outFile).existsSync(), true,
@@ -958,6 +928,7 @@ void main() {}
     );
 
     expect(result.stderr, contains(unsoundNullSafetyMessage));
+    expect(result.stdout, contains(unsoundNullSafetyWarning));
     expect(result.exitCode, 0);
     expect(File(outFile).existsSync(), true,
         reason: 'File not found: $outFile');
@@ -1006,6 +977,7 @@ void main() {}
     );
 
     expect(result.stderr, isNot(contains(soundNullSafetyMessage)));
+    expect(result.stdout, contains(unsoundNullSafetyWarning));
     expect(result.exitCode, 0);
     expect(File(outFile).existsSync(), true,
         reason: 'File not found: $outFile');
@@ -1126,10 +1098,9 @@ void main() {}
       ],
     );
 
-    expect(result.stderr, contains(unsoundNullSafetyMessage));
-    expect(result.exitCode, 0);
-    expect(File(outFile).existsSync(), true,
-        reason: 'File not found: $outFile');
+    expect(result.stdout, contains(unsoundNullSafetyError));
+    expect(result.exitCode, 64);
+    expect(File(outFile).existsSync(), false, reason: 'File found: $outFile');
   });
 
   test('Compile JIT snapshot with --sound-null-safety', () async {
@@ -1175,9 +1146,9 @@ void main() {}
     );
 
     expect(result.stderr, isNot(contains(soundNullSafetyMessage)));
-    expect(result.exitCode, 0);
-    expect(File(outFile).existsSync(), true,
-        reason: 'File not found: $outFile');
+    expect(result.stdout, contains(unsoundNullSafetyError));
+    expect(result.exitCode, 64);
+    expect(File(outFile).existsSync(), false, reason: 'File found: $outFile');
   });
 
   test('Compile JIT snapshot with training args', () async {