analyzer: Simplify AnalysisContextCollectionImpl with new updateAnalysisOptions4 parameter

The new updateAnalysisOptions4 parameter is a callback function,
similar to updateAnalysisOptions3, but with one removed parameter
itself, the `DartSdk sdk` parameter.

The one place where this parameter was used in the callback passed by
the caller, was in DAS's ContextManager, and it was just used in order
to compute a FeatureSet from (a) the computed DartSdk and (b) the
command-line `--enable-experiment` options. We can instead provide a
`enabledExperiments` parameter on AnalysisContextCollectionImpl,
leading to a simpler API and still a simple implementation in
ContextBuilder.

The reason for the incremental migration is that dartdoc, and possibly
some internal clients, use updateAnalysisOptions3.

Change-Id: Ie990ab8f7b850aed1ce3ef39ab71c01835c080da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447400
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: srawlins

pkg/analysis_server/lib/src/context_manager.dart

@@ -10,7 +10,6 @@ import 'package:analysis_server/src/lsp/handlers/handlers.dart';
 import 'package:analysis_server/src/scheduler/scheduled_message.dart';
 import 'package:analysis_server/src/services/correction/fix/data_driven/transform_set_parser.dart';
 import 'package:analyzer/dart/analysis/analysis_context.dart';
-import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/file_system/file_system.dart';
@@ -597,17 +596,7 @@ class ContextManagerImpl implements ContextManager {
               packagesFile: packagesFile,
               fileContentCache: _fileContentCache,
               unlinkedUnitStore: _unlinkedUnitStore,
-              updateAnalysisOptions3: ({
-                required analysisOptions,
-                required sdk,
-              }) {
-                if (_enabledExperiments.isNotEmpty) {
-                  analysisOptions.contextFeatures = FeatureSet.fromEnableFlags2(
-                    sdkLanguageVersion: sdk.languageVersion,
-                    flags: _enabledExperiments,
-                  );
-                }
-              },
+              enabledExperiments: _enabledExperiments,
             );
 
         for (var analysisContext in collection.contexts) {

pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart

@@ -56,18 +56,29 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
     AnalysisDriverScheduler? scheduler,
     FileContentCache? fileContentCache,
     UnlinkedUnitStore? unlinkedUnitStore,
+    List<String> enabledExperiments = const [],
+    @Deprecated('Use updateAnalysisOptions4 instead')
     void Function({
       required AnalysisOptionsImpl analysisOptions,
       required DartSdk sdk,
     })?
     updateAnalysisOptions3,
+    void Function({required AnalysisOptionsImpl analysisOptions})?
+    updateAnalysisOptions4,
     bool enableLintRuleTiming = false,
   }) : resourceProvider =
            resourceProvider ?? PhysicalResourceProvider.INSTANCE {
     sdkPath ??= getSdkPath();
 
     performanceLog ??= PerformanceLog(null);
 
+    if (updateAnalysisOptions3 != null && updateAnalysisOptions4 != null) {
+      throw ArgumentError(
+        'Only one of updateAnalysisOptions3 and updateAnalysisOptions4 may be '
+        'given',
+      );
+    }
+
     if (scheduler == null) {
       scheduler = AnalysisDriverScheduler(performanceLog);
       if (drainStreams) {
@@ -102,6 +113,16 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
       resourceProvider: this.resourceProvider,
     );
 
+    // While users can use the deprecated `updateAnalysisOptions3` and the new
+    // `updateAnalysisOptions4` parameter, prefer `updateAnalysisOptions4`, but
+    // create a new closure with the signature of the old.
+    var updateAnalysisOptions = updateAnalysisOptions4 != null
+        ? ({
+            required AnalysisOptionsImpl analysisOptions,
+            required DartSdk sdk,
+          }) => updateAnalysisOptions4(analysisOptions: analysisOptions)
+        : updateAnalysisOptions3;
+
     for (var root in roots) {
       var context = contextBuilder.createContext(
         byteStore: byteStore,
@@ -117,12 +138,13 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection {
         sdkPath: sdkPath,
         sdkSummaryPath: sdkSummaryPath,
         scheduler: scheduler,
-        updateAnalysisOptions3: updateAnalysisOptions3,
+        updateAnalysisOptions3: updateAnalysisOptions,
         fileContentCache: fileContentCache,
         unlinkedUnitStore: unlinkedUnitStore ?? UnlinkedUnitStoreImpl(),
         ownedFiles: ownedFiles,
         enableLintRuleTiming: enableLintRuleTiming,
         withFineDependencies: withFineDependencies,
+        enabledExperiments: enabledExperiments,
       );
       contexts.add(context);
     }

pkg/analyzer/lib/src/dart/analysis/context_builder.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 
 import 'package:analyzer/dart/analysis/context_root.dart';
 import 'package:analyzer/dart/analysis/declared_variables.dart';
+import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/physical_file_system.dart';
 import 'package:analyzer/src/analysis_options/analysis_options_provider.dart';
@@ -91,6 +92,7 @@ class ContextBuilderImpl {
     bool enableLintRuleTiming = false,
     LinkedBundleProvider? linkedBundleProvider,
     required bool withFineDependencies,
+    List<String> enabledExperiments = const [],
   }) {
     byteStore ??= MemoryByteStore();
     performanceLog ??= PerformanceLog(null);
@@ -141,6 +143,7 @@ class ContextBuilderImpl {
           sourceFactory,
           sdk,
           updateAnalysisOptions3,
+          enabledExperiments,
         ),
       );
     } else {
@@ -292,6 +295,7 @@ class ContextBuilderImpl {
       required DartSdk sdk,
     })?
     updateAnalysisOptions,
+    List<String> enabledExperiments,
   ) {
     AnalysisOptionsImpl? options;
 
@@ -308,6 +312,10 @@ class ContextBuilderImpl {
       }
     }
     options ??= AnalysisOptionsImpl(file: optionsFile);
+    options.contextFeatures = FeatureSet.fromEnableFlags2(
+      sdkLanguageVersion: sdk.languageVersion,
+      flags: enabledExperiments,
+    );
 
     if (updateAnalysisOptions != null) {
       updateAnalysisOptions(analysisOptions: options, sdk: sdk);

pkg/analyzer/test/src/dart/analysis/analysis_context_collection_test.dart

@@ -9,7 +9,6 @@ import 'package:analyzer/src/dart/analysis/analysis_options.dart';
 import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/dart/analysis/experiments.dart';
 import 'package:analyzer/src/dart/analysis/file_state.dart';
-import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/test_utilities/mock_sdk.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
 import 'package:analyzer/src/utilities/extensions/file_system.dart';
@@ -379,7 +378,7 @@ name: test
 
     _assertWorkspaceCollectionText(
       workspaceRootPath,
-      updateAnalysisOptions: ({required analysisOptions, required sdk}) {
+      updateAnalysisOptions: ({required analysisOptions}) {
         analysisOptions.contextFeatures = FeatureSet.fromEnableFlags2(
           sdkLanguageVersion: ExperimentStatus.currentVersion,
           flags: ['digit-separators', 'variance'],
@@ -461,7 +460,7 @@ name: test
 
     _assertWorkspaceCollectionText(
       workspaceRootPath,
-      updateAnalysisOptions: ({required analysisOptions, required sdk}) {
+      updateAnalysisOptions: ({required analysisOptions}) {
         analysisOptions.contextFeatures = FeatureSet.fromEnableFlags2(
           sdkLanguageVersion: ExperimentStatus.currentVersion,
           flags: ['variance'],
@@ -1243,10 +1242,7 @@ workspaces
     String workspaceRootPath,
     String expected, {
     File? optionsFile,
-    void Function({
-      required AnalysisOptionsImpl analysisOptions,
-      required DartSdk sdk,
-    })?
+    void Function({required AnalysisOptionsImpl analysisOptions})?
     updateAnalysisOptions,
   }) {
     if (optionsFile != null) {
@@ -1257,7 +1253,7 @@ workspaces
       sdkPath: sdkRoot.path,
       includedPaths: [getFolder(workspaceRootPath).path],
       optionsFile: optionsFile?.path,
-      updateAnalysisOptions3: updateAnalysisOptions,
+      updateAnalysisOptions4: updateAnalysisOptions,
     );
 
     _assertCollectionText(collection, expected);

pkg/analyzer/test/src/dart/resolution/context_collection_resolution.dart

@@ -14,7 +14,6 @@ import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/dart/analysis/results.dart';
 import 'package:analyzer/src/dart/analysis/unlinked_unit_store.dart';
 import 'package:analyzer/src/dart/element/element.dart';
-import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/test_utilities/mock_sdk.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
 import 'package:analyzer/src/workspace/basic.dart';
@@ -125,7 +124,6 @@ abstract class ContextResolutionTest
       sdkPath: sdkRoot.path,
       sdkSummaryPath: sdkSummaryFile?.path,
       librarySummaryPaths: librarySummaryFiles?.map((e) => e.path).toList(),
-      updateAnalysisOptions3: updateAnalysisOptions,
       drainStreams: false,
       withFineDependencies: withFineDependencies,
     );
@@ -255,13 +253,6 @@ abstract class ContextResolutionTest
     await disposeAnalysisContextCollection();
   }
 
-  /// Override this method to update [analysisOptions] for every context root,
-  /// the default or already updated with `analysis_options.yaml` file.
-  void updateAnalysisOptions({
-    required AnalysisOptionsImpl analysisOptions,
-    required DartSdk sdk,
-  }) {}
-
   /// Call this method if the test needs to use the empty byte store, without
   /// any information cached.
   void useEmptyByteStore() {

pkg/analyzer/test/util/id_testing_helper.dart

@@ -144,7 +144,7 @@ Future<TestResult<T>> runTestForConfig<T>(
     resourceProvider: resourceProvider,
     retainDataForTesting: true,
     sdkPath: sdkRoot.path,
-    updateAnalysisOptions3: ({required analysisOptions, required sdk}) {
+    updateAnalysisOptions4: ({required analysisOptions}) {
       analysisOptions.contextFeatures = config.featureSet;
     },
   );

pkg/analyzer_cli/lib/src/driver.dart

@@ -21,7 +21,6 @@ import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
 import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
 import 'package:analyzer/src/dart/analysis/file_state.dart';
 import 'package:analyzer/src/dart/analysis/results.dart';
-import 'package:analyzer/src/generated/sdk.dart';
 import 'package:analyzer/src/manifest/manifest_validator.dart';
 import 'package:analyzer/src/pubspec/pubspec_validator.dart';
 import 'package:analyzer/src/source/path_filter.dart';
@@ -617,7 +616,7 @@ class _AnalysisContextProvider {
       packagesFile: _commandLineOptions!.defaultPackagesPath,
       resourceProvider: _resourceProvider,
       sdkPath: _commandLineOptions!.dartSdkPath,
-      updateAnalysisOptions3: _updateAnalysisOptions,
+      updateAnalysisOptions4: _updateAnalysisOptions,
       fileContentCache: _fileContentCache,
     );
     _toDispose.add(_collection!);
@@ -652,10 +651,7 @@ class _AnalysisContextProvider {
     _analysisContext = _collection!.contextFor(path);
   }
 
-  void _updateAnalysisOptions({
-    required AnalysisOptionsImpl analysisOptions,
-    required DartSdk sdk,
-  }) {
+  void _updateAnalysisOptions({required AnalysisOptionsImpl analysisOptions}) {
     _commandLineOptions!.updateAnalysisOptions(analysisOptions);
   }
 }

pkg/linter/tool/test_linter.dart

@@ -52,14 +52,15 @@ class TestLinter implements DiagnosticListener {
   Future<List<Diagnostic>> _analyze(Iterable<io.File> files) async {
     AnalysisEngine.instance.instrumentationService = _StdInstrumentation();
 
-    var filePaths =
-        files.map((file) => _absoluteNormalizedPath(file.path)).toList();
+    var filePaths = files
+        .map((file) => _absoluteNormalizedPath(file.path))
+        .toList();
 
     var contextCollection = AnalysisContextCollectionImpl(
       resourceProvider: _resourceProvider,
       sdkPath: _dartSdkPath,
       includedPaths: filePaths,
-      updateAnalysisOptions3: ({required analysisOptions, required sdk}) {
+      updateAnalysisOptions4: ({required analysisOptions}) {
         analysisOptions.lint = true;
         analysisOptions.warning = false;
         analysisOptions.lintRules = _rules;