Removed unsound null safety options from classes ModuleMetadata and M…

…etadataProvider (#2510)

* Remove unsound null safety options from classes ModuleMetadata and MetadataProvider

* updated changelog

* Removed unsound null safety options from classes ModuleMetadata, MetadataProvider & Metadata_test.

* deprecated metadataProvider's soundNullSafety and associated tests

* Deprecated CompilerOptions' soundNullSafety and related usage

* deprecated sdk_configuration_test's soundNullSafety

* deprecated test_sdk_layout's soundNullSafety

* removed remaining use of sound/weak null safety

* deprecated sound/weak sdkSummaryPath but kept variables in constructor to avoid breaking changes

* deprecated sound/weak sdkSummaryPath for SdkLayout class but kept variables in constructor to avoid breaking changes

* formatted code

* formatted code

* Deprecated public getters soundSdkSummaryUri & weakSdkSummaryUri

dwds/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 - Replace deprecated JS code `this.__proto__` with `Object.getPrototypeOf(this)`. - [#2500](https://github.com/dart-lang/webdev/pull/2500)
 - Migrate injected client code to `package:web`. - [#2491](https://github.com/dart-lang/webdev/pull/2491)
+- Deprecated MetadataProvider's, CompilerOptions', SdkConfiguration's & SdkLayout's soundNullSafety. - [#2427](https://github.com/dart-lang/webdev/issues/2427)
 
 ## 24.1.0
 

dwds/lib/src/debugging/metadata/module_metadata.dart

@@ -114,18 +114,13 @@ class ModuleMetadata {
   /// Module uri
   final String moduleUri;
 
-  /// True if the module corresponding to this metadata was compiled with sound
-  /// null safety enabled.
-  final bool soundNullSafety;
-
   final Map<String, LibraryMetadata> libraries = {};
 
   ModuleMetadata(
     this.name,
     this.closureName,
     this.sourceMapUri,
-    this.moduleUri,
-    this.soundNullSafety, {
+    this.moduleUri, {
     String? ver,
   }) {
     version = ver ?? ModuleMetadataVersion.current.version;
@@ -151,8 +146,7 @@ class ModuleMetadata {
         name = _readRequiredField(json, 'name'),
         closureName = _readRequiredField(json, 'closureName'),
         sourceMapUri = _readRequiredField(json, 'sourceMapUri'),
-        moduleUri = _readRequiredField(json, 'moduleUri'),
-        soundNullSafety = _readOptionalField(json, 'soundNullSafety') ?? false {
+        moduleUri = _readRequiredField(json, 'moduleUri') {
     if (!ModuleMetadataVersion.current.isCompatibleWith(version) &&
         !ModuleMetadataVersion.previous.isCompatibleWith(version)) {
       throw Exception('Unsupported metadata version $version. '
@@ -174,7 +168,6 @@ class ModuleMetadata {
       'sourceMapUri': sourceMapUri,
       'moduleUri': moduleUri,
       'libraries': [for (final lib in libraries.values) lib.toJson()],
-      'soundNullSafety': soundNullSafety,
     };
   }
 }

dwds/lib/src/debugging/metadata/provider.dart

@@ -15,7 +15,6 @@ class MetadataProvider {
   final AssetReader _assetReader;
   final _logger = Logger('MetadataProvider');
   final String entrypoint;
-  bool _soundNullSafety;
   final List<String> _libraries = [];
   final Map<String, String> _scriptToModule = {};
   final Map<String, String> _moduleToSourceMap = {};
@@ -65,15 +64,15 @@ class MetadataProvider {
         'dart:ui',
       ];
 
-  MetadataProvider(this.entrypoint, this._assetReader)
-      : _soundNullSafety = false;
+  MetadataProvider(this.entrypoint, this._assetReader);
 
   /// A sound null safety mode for the whole app.
   ///
   /// All libraries have to agree on null safety mode.
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   Future<bool> get soundNullSafety async {
     await _initialize();
-    return _soundNullSafety;
+    return true;
   }
 
   /// A list of all libraries in the Dart application.
@@ -178,8 +177,6 @@ class MetadataProvider {
 
   Future<void> _initialize() async {
     await _metadataMemoizer.runOnce(() async {
-      var hasSoundNullSafety = true;
-      var hasUnsoundNullSafety = true;
       // The merged metadata resides next to the entrypoint.
       // Assume that <name>.bootstrap.js has <name>.ddc_merged_metadata
       if (entrypoint.endsWith('.bootstrap.js')) {
@@ -199,22 +196,14 @@ class MetadataProvider {
               final metadata =
                   ModuleMetadata.fromJson(moduleJson as Map<String, dynamic>);
               _addMetadata(metadata);
-              hasUnsoundNullSafety &= !metadata.soundNullSafety;
-              hasSoundNullSafety &= metadata.soundNullSafety;
               _logger
                   .fine('Loaded debug metadata for module: ${metadata.name}');
             } catch (e) {
               _logger.warning('Failed to read metadata: $e');
               rethrow;
             }
           }
-          if (!hasSoundNullSafety && !hasUnsoundNullSafety) {
-            throw Exception('Metadata contains modules with mixed null safety');
-          }
-          _soundNullSafety = hasSoundNullSafety;
         }
-        _logger.info('Loaded debug metadata '
-            '(${_soundNullSafety ? "sound" : "weak"} null safety)');
       }
     });
   }

dwds/lib/src/services/chrome_proxy_service.dart

@@ -205,16 +205,10 @@ class ChromeProxyService implements VmServiceInterface {
     final canaryFeatures = loadStrategy.buildSettings.canaryFeatures;
     final experiments = loadStrategy.buildSettings.experiments;
 
-    // TODO(annagrin): Read null safety setting from the build settings.
-    final metadataProvider = loadStrategy.metadataProviderFor(entrypoint);
-    final soundNullSafety = await metadataProvider.soundNullSafety;
-
-    _logger.info('Initializing expression compiler for $entrypoint '
-        'with sound null safety: $soundNullSafety');
+    _logger.info('Initializing expression compiler for $entrypoint');
 
     final compilerOptions = CompilerOptions(
       moduleFormat: ModuleFormat.values.byName(moduleFormat),
-      soundNullSafety: soundNullSafety,
       canaryFeatures: canaryFeatures,
       experiments: experiments,
     );

dwds/lib/src/services/expression_compiler.dart

@@ -5,13 +5,16 @@
 /// Options passed to DDC and the expression compiler.
 class CompilerOptions {
   final ModuleFormat moduleFormat;
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   final bool soundNullSafety;
+
   final bool canaryFeatures;
   final List<String> experiments;
 
   CompilerOptions({
     required this.moduleFormat,
-    required this.soundNullSafety,
+    this.soundNullSafety = true,
     required this.canaryFeatures,
     required this.experiments,
   });

dwds/lib/src/services/expression_compiler_service.dart

@@ -53,9 +53,6 @@ class _Compiler {
   /// expression compilation (summaries, libraries spec, compiler worker
   /// snapshot).
   ///
-  /// [soundNullSafety] indicates if the compiler should support sound
-  /// null safety.
-  ///
   /// Performs handshake with the isolate running expression compiler
   /// worker to establish communication via send/receive ports, returns
   /// the service after the communication is established.
@@ -69,16 +66,10 @@ class _Compiler {
     bool verbose,
   ) async {
     sdkConfiguration.validateSdkDir();
-    if (compilerOptions.soundNullSafety) {
-      sdkConfiguration.validateSoundSummaries();
-    } else {
-      sdkConfiguration.validateWeakSummaries();
-    }
+    sdkConfiguration.validateSummaries();
 
     final workerUri = sdkConfiguration.compilerWorkerUri!;
-    final sdkSummaryUri = compilerOptions.soundNullSafety
-        ? sdkConfiguration.soundSdkSummaryUri!
-        : sdkConfiguration.weakSdkSummaryUri!;
+    final sdkSummaryUri = sdkConfiguration.sdkSummaryUri!;
 
     final args = [
       '--experimental-expression-compiler',
@@ -91,9 +82,7 @@ class _Compiler {
       '--module-format',
       compilerOptions.moduleFormat.name,
       if (verbose) '--verbose',
-      compilerOptions.soundNullSafety
-          ? '--sound-null-safety'
-          : '--no-sound-null-safety',
+      '--sound-null-safety',
       for (final experiment in compilerOptions.experiments)
         '--enable-experiment=$experiment',
       if (compilerOptions.canaryFeatures) '--canary',

dwds/lib/src/utilities/sdk_configuration.dart

@@ -42,25 +42,24 @@ class SdkLayout {
       SdkLayout.createDefault(defaultSdkDirectory);
 
   final String sdkDirectory;
+  final String summaryPath;
+  final String dartdevcSnapshotPath;
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   final String soundSummaryPath;
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   final String weakSummaryPath;
-  final String dartdevcSnapshotPath;
 
   SdkLayout.createDefault(String sdkDirectory)
       : this(
           sdkDirectory: sdkDirectory,
-          soundSummaryPath: p.join(
+          summaryPath: p.join(
             sdkDirectory,
             'lib',
             '_internal',
             'ddc_outline.dill',
           ),
-          weakSummaryPath: p.join(
-            sdkDirectory,
-            'lib',
-            '_internal',
-            'ddc_outline_unsound.dill',
-          ),
           dartdevcSnapshotPath: p.join(
             sdkDirectory,
             'bin',
@@ -71,8 +70,9 @@ class SdkLayout {
 
   const SdkLayout({
     required this.sdkDirectory,
-    required this.soundSummaryPath,
-    required this.weakSummaryPath,
+    required this.summaryPath,
+    this.soundSummaryPath = '',
+    this.weakSummaryPath = '',
     required this.dartdevcSnapshotPath,
   });
 }
@@ -87,12 +87,18 @@ class SdkConfiguration {
       SdkConfiguration.fromSdkLayout(SdkLayout.defaultSdkLayout);
 
   final String? sdkDirectory;
+  final String? sdkSummaryPath;
+  final String? compilerWorkerPath;
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   final String? weakSdkSummaryPath;
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   final String? soundSdkSummaryPath;
-  final String? compilerWorkerPath;
 
   const SdkConfiguration({
     this.sdkDirectory,
+    this.sdkSummaryPath,
     this.weakSdkSummaryPath,
     this.soundSdkSummaryPath,
     this.compilerWorkerPath,
@@ -103,8 +109,7 @@ class SdkConfiguration {
   SdkConfiguration.fromSdkLayout(SdkLayout sdkLayout)
       : this(
           sdkDirectory: sdkLayout.sdkDirectory,
-          weakSdkSummaryPath: sdkLayout.weakSummaryPath,
-          soundSdkSummaryPath: sdkLayout.soundSummaryPath,
+          sdkSummaryPath: sdkLayout.summaryPath,
           compilerWorkerPath: sdkLayout.dartdevcSnapshotPath,
         );
 
@@ -113,7 +118,12 @@ class SdkConfiguration {
       path == null ? null : p.toUri(p.absolute(path));
 
   Uri? get sdkDirectoryUri => _toUri(sdkDirectory);
+  Uri? get sdkSummaryUri => _toUri(sdkSummaryPath);
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   Uri? get soundSdkSummaryUri => _toUri(soundSdkSummaryPath);
+
+  @Deprecated('Only sound null safety is supported as of Dart 3.0')
   Uri? get weakSdkSummaryUri => _toUri(weakSdkSummaryPath);
 
   /// Note: has to be ///file: Uri to run in an isolate.
@@ -139,28 +149,10 @@ class SdkConfiguration {
   }
 
   void validateSummaries({FileSystem fileSystem = const LocalFileSystem()}) {
-    validateSoundSummaries(fileSystem: fileSystem);
-    validateWeakSummaries(fileSystem: fileSystem);
-  }
-
-  void validateWeakSummaries({
-    FileSystem fileSystem = const LocalFileSystem(),
-  }) {
-    if (weakSdkSummaryPath == null ||
-        !fileSystem.file(weakSdkSummaryPath).existsSync()) {
-      throw InvalidSdkConfigurationException(
-        'Sdk summary $weakSdkSummaryPath does not exist',
-      );
-    }
-  }
-
-  void validateSoundSummaries({
-    FileSystem fileSystem = const LocalFileSystem(),
-  }) {
-    if (soundSdkSummaryPath == null ||
-        !fileSystem.file(soundSdkSummaryPath).existsSync()) {
+    if (sdkSummaryPath == null ||
+        !fileSystem.file(sdkSummaryPath).existsSync()) {
       throw InvalidSdkConfigurationException(
-        'Sdk summary $soundSdkSummaryPath does not exist',
+        'Sdk summary $sdkSummaryPath does not exist',
       );
     }
   }

dwds/test/expression_compiler_service_ddc_and_canary_test.dart

@@ -16,7 +16,6 @@ void main() async {
   testAll(
     compilerOptions: CompilerOptions(
       moduleFormat: ModuleFormat.ddc,
-      soundNullSafety: true,
       canaryFeatures: true,
       experiments: const <String>[],
     ),

dwds/test/expression_compiler_service_test.dart

@@ -16,7 +16,6 @@ void main() async {
   testAll(
     compilerOptions: CompilerOptions(
       moduleFormat: ModuleFormat.amd,
-      soundNullSafety: true,
       canaryFeatures: false,
       experiments: const <String>[],
     ),

dwds/test/fixtures/utilities.dart

@@ -300,5 +300,5 @@ class TestCompilerOptions extends CompilerOptions {
     required super.canaryFeatures,
     super.experiments = const [],
     super.moduleFormat = ModuleFormat.amd,
-  }) : super(soundNullSafety: true);
+  });
 }

dwds/test/metadata_test.dart

@@ -13,16 +13,6 @@ import 'fixtures/fakes.dart';
 import 'fixtures/utilities.dart';
 
 const _emptySourceMetadata =
-    '{"version":"1.0.0","name":"web/main","closureName":"load__web__main",'
-    '"sourceMapUri":"foo/web/main.ddc.js.map",'
-    '"moduleUri":"foo/web/main.ddc.js",'
-    '"soundNullSafety":true,'
-    '"libraries":[{"name":"main",'
-    '"importUri":"org-dartlang-app:///web/main.dart",'
-    '"fileUri":"org-dartlang-app:///web/main.dart","partUris":[]}]}\n'
-    '// intentionally empty: package blah has no dart sources';
-
-const _noNullSafetyMetadata =
     '{"version":"1.0.0","name":"web/main","closureName":"load__web__main",'
     '"sourceMapUri":"foo/web/main.ddc.js.map",'
     '"moduleUri":"foo/web/main.ddc.js",'
@@ -35,7 +25,6 @@ const _fileUriMetadata =
     '{"version":"1.0.0","name":"web/main","closureName":"load__web__main",'
     '"sourceMapUri":"foo/web/main.ddc.js.map",'
     '"moduleUri":"foo/web/main.ddc.js",'
-    '"soundNullSafety":true,'
     '"libraries":[{"name":"main",'
     '"importUri":"file:/Users/foo/blah/sample/lib/bar.dart",'
     '"fileUri":"org-dartlang-app:///web/main.dart","partUris":[]}]}\n'
@@ -57,19 +46,6 @@ void main() {
       await provider.libraries,
       contains('org-dartlang-app:///web/main.dart'),
     );
-    expect(await provider.soundNullSafety, isNotNull);
-  });
-
-  test('can parse metadata with no null safety information', () async {
-    final provider = MetadataProvider(
-      'foo.bootstrap.js',
-      FakeAssetReader(metadata: _noNullSafetyMetadata),
-    );
-    expect(
-      await provider.libraries,
-      contains('org-dartlang-app:///web/main.dart'),
-    );
-    expect(await provider.soundNullSafety, false);
   });
 
   test('throws on metadata with absolute import uris', () async {
@@ -114,6 +90,5 @@ void main() {
       expect(parts.length, 1);
       expect(parts[0], 'org-dartlang-app:///web/main.dart');
     }
-    expect(metadata.soundNullSafety, false);
   });
 }