[DAS] Adds fix for multiple_combinators warning

Bug: #56879
Fixes: #60309

Change-Id: Ifac77bbcb0d22c96cc01ea12fba1af86721809e2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/414800
Reviewed-by: Phil Quitslund <[email protected]>
Auto-Submit: Felipe Morschel <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: FMorschel

pkg/analysis_server/lib/src/services/correction/dart/merge_combinators.dart

@@ -0,0 +1,191 @@
+// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/correction/util.dart';
+import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/element/element2.dart';
+import 'package:analyzer/src/dart/ast/ast.dart';
+import 'package:analyzer/src/dart/element/element.dart';
+import 'package:analyzer/src/dart/resolver/scope.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:analyzer_plugin/utilities/range_factory.dart';
+import 'package:collection/collection.dart';
+
+class MergeCombinators extends MultiCorrectionProducer {
+  MergeCombinators({required super.context});
+
+  @override
+  Future<List<ResolvedCorrectionProducer>> get producers async {
+    var node = this.node;
+    NamespaceDirective? directive;
+    if (node case Combinator(:NamespaceDirective parent)) {
+      directive = parent;
+    } else if (node is NamespaceDirective) {
+      directive = node;
+    } else {
+      return const [];
+    }
+
+    var combinators = directive.combinators;
+    if (combinators.length < 2) {
+      return const [];
+    }
+
+    if (combinators.whereType<ShowCombinator>().isEmpty) {
+      return [
+        _MergeCombinators(
+          DartFixKind.MERGE_COMBINATORS_HIDE_HIDE,
+          directive,
+          mergeWithShow: false,
+          context: context,
+        ),
+        _MergeCombinators(
+          DartFixKind.MERGE_COMBINATORS_SHOW_HIDE,
+          directive,
+          mergeWithShow: true,
+          context: context,
+        ),
+      ];
+    }
+
+    return [
+      _MergeCombinators(
+        DartFixKind.MERGE_COMBINATORS_SHOW_SHOW,
+        directive,
+        mergeWithShow: true,
+        context: context,
+      ),
+      _MergeCombinators(
+        DartFixKind.MERGE_COMBINATORS_HIDE_SHOW,
+        directive,
+        mergeWithShow: false,
+        context: context,
+      ),
+    ];
+  }
+}
+
+class _MergeCombinators extends ResolvedCorrectionProducer {
+  static final namespaceBuilder = NamespaceBuilder();
+
+  @override
+  final FixKind fixKind;
+
+  final bool mergeWithShow;
+  final NamespaceDirective directive;
+
+  _MergeCombinators(
+    this.fixKind,
+    this.directive, {
+    required this.mergeWithShow,
+    required super.context,
+  });
+
+  @override
+  CorrectionApplicability get applicability =>
+      CorrectionApplicability.singleLocation;
+
+  @override
+  Future<void> compute(ChangeBuilder builder) async {
+    LibraryElement2? element;
+    Map<String, Element2> namespace;
+    Namespace? originalNamespace;
+    switch (directive) {
+      case ExportDirective(:LibraryExportElementImpl libraryExport):
+        element = libraryExport.exportedLibrary2;
+        if (element is! LibraryElementImpl) {
+          return;
+        }
+        originalNamespace = _originalNamespace(element);
+        namespace = _currentNamespace(libraryExport).definedNames2;
+      case ImportDirective(:var libraryImport?):
+        namespace = getImportNamespace(libraryImport);
+        element = libraryImport.importedLibrary2;
+      default:
+        return;
+    }
+    if (element is! LibraryElementImpl) {
+      return;
+    }
+
+    if (mergeWithShow) {
+      var referencedNames = namespace.keys.toList();
+      await _buildNewCombinator(builder, Keyword.SHOW, referencedNames);
+      return;
+    }
+
+    originalNamespace ??= _originalNamespace(element);
+
+    var explicitlyHiddenNames = directive.hideCombinators.hiddenNames;
+
+    var hiddenNames =
+        {
+          ...explicitlyHiddenNames,
+          ...originalNamespace.hiddenNames({
+            ...explicitlyHiddenNames,
+            ...namespace.keys,
+          }),
+        }.toList();
+    await _buildNewCombinator(builder, Keyword.HIDE, hiddenNames);
+  }
+
+  Future<void> _buildNewCombinator(
+    ChangeBuilder builder,
+    Keyword keyword,
+    List<String> names,
+  ) async {
+    if (getCodeStyleOptions(unitResult.file).sortCombinators) {
+      names.sort();
+    }
+    await builder.addDartFileEdit(file, (builder) {
+      var combinator = '';
+      if (names.isNotEmpty) {
+        combinator = ' ${keyword.lexeme} ${names.join(', ')}';
+      }
+      var combinators = directive.combinators;
+      builder.addSimpleReplacement(
+        range.startOffsetEndOffset(combinators.offset - 1, combinators.end),
+        combinator,
+      );
+    });
+  }
+
+  Namespace _currentNamespace(LibraryExportElementImpl libraryExport) {
+    return namespaceBuilder.createExportNamespaceForDirective2(libraryExport);
+  }
+
+  Namespace _originalNamespace(LibraryElementImpl element) {
+    return namespaceBuilder.createExportNamespaceForLibrary(element);
+  }
+}
+
+extension on Namespace {
+  Iterable<String> hiddenNames(Set<String> currentNames) {
+    return definedNames2.keys.whereNot(currentNames.contains);
+  }
+}
+
+extension on NodeList<Combinator> {
+  int get end => endToken!.end;
+  int get offset => beginToken!.offset;
+}
+
+extension on NamespaceDirective {
+  Iterable<HideCombinator> get hideCombinators {
+    return combinators.whereType<HideCombinator>();
+  }
+}
+
+extension on Iterable<HideCombinator> {
+  Set<String> get hiddenNames {
+    var set = <String>{};
+    for (var combinator in this) {
+      set.addAll(combinator.hiddenNames.map((identifier) => identifier.name));
+    }
+    return set;
+  }
+}

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -3657,9 +3657,7 @@ WarningCode.MIXIN_ON_SEALED_CLASS:
     `sealed` annotation is likely to be replaced by the `sealed` keyword
     in the next stable release.
 WarningCode.MULTIPLE_COMBINATORS:
-  status: needsFix
-  notes: |-
-    Aggregate the combinators into a single one.
+  status: hasFix
 WarningCode.MUST_BE_IMMUTABLE:
   status: noFix
 WarningCode.MUST_CALL_SUPER:

pkg/analysis_server/lib/src/services/correction/fix.dart

@@ -1020,7 +1020,6 @@ abstract final class DartFixKind {
     DartFixKindPriority.standard,
     'Make final',
   );
-
   // TODO(pq): consider parameterizing: 'Make {fields} final...'
   static const MAKE_FINAL_MULTI = FixKind(
     'dart.fix.makeFinal.multi',
@@ -1072,6 +1071,38 @@ abstract final class DartFixKind {
     DartFixKindPriority.standard,
     'Match an empty map',
   );
+
+  /// Used when the user has at least one `show` combinator to suggest merging
+  /// using `show`.
+  static const MERGE_COMBINATORS_SHOW_SHOW = FixKind(
+    'dart.fix.mergeCombinatorsShow.show',
+    DartFixKindPriority.standard + 1,
+    "Merge combinators into a single 'show'",
+  );
+
+  /// Used when the user has only `hide` combinators to suggest merging using
+  /// `show`.
+  static const MERGE_COMBINATORS_SHOW_HIDE = FixKind(
+    'dart.fix.mergeCombinatorsShow.hide',
+    DartFixKindPriority.standard,
+    "Merge combinators into a single 'show'",
+  );
+
+  /// Used when the user has only `hide` combinators to suggest merging using
+  /// `hide`.
+  static const MERGE_COMBINATORS_HIDE_HIDE = FixKind(
+    'dart.fix.mergeCombinatorsHide.hide',
+    DartFixKindPriority.standard + 1,
+    "Merge combinators into a single 'hide'",
+  );
+
+  /// Used when the user has at least one `show` combinator to suggest merging
+  /// using `hide`.
+  static const MERGE_COMBINATORS_HIDE_SHOW = FixKind(
+    'dart.fix.mergeCombinatorsHide.show',
+    DartFixKindPriority.standard,
+    "Merge combinators into a single 'hide'",
+  );
   static const MOVE_ANNOTATION_TO_LIBRARY_DIRECTIVE = FixKind(
     'dart.fix.moveAnnotationToLibraryDirective',
     DartFixKindPriority.standard,

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -122,6 +122,7 @@ import 'package:analysis_server/src/services/correction/dart/make_return_type_nu
 import 'package:analysis_server/src/services/correction/dart/make_super_invocation_last.dart';
 import 'package:analysis_server/src/services/correction/dart/make_variable_not_final.dart';
 import 'package:analysis_server/src/services/correction/dart/make_variable_nullable.dart';
+import 'package:analysis_server/src/services/correction/dart/merge_combinators.dart';
 import 'package:analysis_server/src/services/correction/dart/move_annotation_to_library_directive.dart';
 import 'package:analysis_server/src/services/correction/dart/move_doc_comment_to_library_directive.dart';
 import 'package:analysis_server/src/services/correction/dart/move_type_arguments_to_class.dart';
@@ -1432,6 +1433,7 @@ final _builtInNonLintMultiGenerators = {
   HintCode.DEPRECATED_MEMBER_USE: [DataDriven.new],
   HintCode.DEPRECATED_MEMBER_USE_WITH_MESSAGE: [DataDriven.new],
   WarningCode.DEPRECATED_EXPORT_USE: [DataDriven.new],
+  WarningCode.MULTIPLE_COMBINATORS: [MergeCombinators.new],
   WarningCode.OVERRIDE_ON_NON_OVERRIDING_METHOD: [DataDriven.new],
 };
 

pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart

@@ -25,6 +25,8 @@ import '../../../../utils/test_instrumentation_service.dart';
 
 export 'package:analyzer/src/test_utilities/package_config_file_builder.dart';
 
+typedef ErrorFilter = bool Function(AnalysisError error);
+
 abstract class BaseFixProcessorTest extends AbstractSingleUnitTest {
   /// The source change associated with the fix that was found.
   late SourceChange change;
@@ -41,9 +43,7 @@ abstract class BaseFixProcessorTest extends AbstractSingleUnitTest {
   /// using the [errorFilter] to filter out errors that should be ignored, and
   /// expecting that there is a single remaining error. The error filter should
   /// return `true` if the error should not be ignored.
-  Future<AnalysisError> _findErrorToFix({
-    bool Function(AnalysisError)? errorFilter,
-  }) async {
+  Future<AnalysisError> _findErrorToFix({ErrorFilter? errorFilter}) async {
     var errors = testAnalysisResult.errors;
     if (errorFilter != null) {
       if (errors.length == 1) {
@@ -217,9 +217,7 @@ abstract class FixInFileProcessorTest extends BaseFixProcessorTest {
     expect(resultCode, expected);
   }
 
-  Future<List<Fix>> getFixesForFirst(
-    bool Function(AnalysisError error) test,
-  ) async {
+  Future<List<Fix>> getFixesForFirst(ErrorFilter test) async {
     var errors = testAnalysisResult.errors.where(test);
     expect(errors, isNotEmpty);
     String? errorCode;
@@ -275,6 +273,17 @@ abstract class FixInFileProcessorTest extends BaseFixProcessorTest {
   }
 }
 
+/// A base class defining support for writing fix processor tests that are
+/// specific to fixes associated with the [errorCode] that use the FixKind.
+abstract class FixProcessorErrorCodeTest extends FixProcessorTest {
+  /// Return the error code being tested.
+  ErrorCode get errorCode;
+
+  ErrorFilter get errorCodeFilter => (e) {
+    return e.errorCode == errorCode;
+  };
+}
+
 /// A base class defining support for writing fix processor tests that are
 /// specific to fixes associated with lints that use the FixKind.
 abstract class FixProcessorLintTest extends FixProcessorTest {
@@ -296,7 +305,7 @@ abstract class FixProcessorLintTest extends FixProcessorTest {
     return lintCodeSet.single;
   }
 
-  bool Function(AnalysisError) lintNameFilter(String name) {
+  ErrorFilter lintNameFilter(String name) {
     return (e) {
       return e.errorCode is LintCode && e.errorCode.name == name;
     };
@@ -318,7 +327,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest {
   /// [expected] output.
   Future<void> assertHasFix(
     String expected, {
-    bool Function(AnalysisError error)? errorFilter,
+    ErrorFilter? errorFilter,
     String? target,
     int? expectedNumberOfFixesForKind,
     String? matchFixMessage,
@@ -376,7 +385,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest {
   /// [expectedNumberOfFixesForKind] fixes of the appropriate kind are found,
   /// and that they have messages equal to [matchFixMessages].
   Future<void> assertHasFixesWithoutApplying({
-    bool Function(AnalysisError)? errorFilter,
+    ErrorFilter? errorFilter,
     required int expectedNumberOfFixesForKind,
     required List<String> matchFixMessages,
   }) async {
@@ -388,9 +397,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest {
     );
   }
 
-  Future<void> assertHasFixWithoutApplying({
-    bool Function(AnalysisError)? errorFilter,
-  }) async {
+  Future<void> assertHasFixWithoutApplying({ErrorFilter? errorFilter}) async {
     var error = await _findErrorToFix(errorFilter: errorFilter);
     var fix = await _assertHasFix(error);
     change = fix.change;
@@ -419,7 +426,7 @@ abstract class FixProcessorTest extends BaseFixProcessorTest {
 
   /// Compute fixes and ensure that there is no fix of the [kind] being tested by
   /// this class.
-  Future<void> assertNoFix({bool Function(AnalysisError)? errorFilter}) async {
+  Future<void> assertNoFix({ErrorFilter? errorFilter}) async {
     var error = await _findErrorToFix(errorFilter: errorFilter);
     await _assertNoFix(error);
   }