[DAS] Fixes update combinators to follow lint and to work with multiple

Fixes: #60131
Change-Id: Idfb03889875e3e902cb530bf10a9f4fd51b6a560
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409765
Auto-Submit: Felipe Morschel <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: FMorschel

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

@@ -26,6 +26,7 @@ import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_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';
 
 typedef _ProducersGenerators =
     Future<List<ResolvedCorrectionProducer>> Function(
@@ -110,12 +111,9 @@ class ImportLibrary extends MultiCorrectionProducer {
     String? prefix,
   }) async {
     var combinators = import.combinators;
-    if (combinators.length != 1) {
+    if (combinators.isEmpty) {
       return (null, null);
     }
-    _ImportLibraryCombinator? importCombinator;
-    _ImportLibraryCombinatorMultiple? importCombinatorMultiple;
-    var combinator = combinators.first;
     var otherNames = await _otherUnresolvedNames(prefix, name);
     var namesInThisLibrary = <String>[
       name,
@@ -125,41 +123,23 @@ class ImportLibrary extends MultiCorrectionProducer {
           exportedName,
     ];
     var importPrefix = import.prefix2?.element;
-    if (combinator is HideElementCombinator) {
-      importCombinator = _ImportLibraryCombinator(
-        uri,
-        combinator,
-        name,
-        removePrefix: importPrefix == null,
-        context: context,
-      );
-      if (namesInThisLibrary.length > 1) {
-        importCombinatorMultiple = _ImportLibraryCombinatorMultiple(
-          uri,
-          combinator,
-          namesInThisLibrary,
-          removePrefix: importPrefix == null,
-          context: context,
-        );
-      }
-    } else if (combinator is ShowElementCombinator) {
-      importCombinator = _ImportLibraryCombinator(
-        uri,
-        combinator,
-        name,
-        removePrefix: importPrefix == null,
-        context: context,
-      );
-      if (namesInThisLibrary.length > 1) {
-        importCombinatorMultiple = _ImportLibraryCombinatorMultiple(
-          uri,
-          combinator,
-          namesInThisLibrary,
-          removePrefix: importPrefix == null,
-          context: context,
-        );
-      }
+    var importCombinator = _ImportLibraryCombinator(
+      uri,
+      combinators,
+      name,
+      removePrefix: importPrefix == null,
+      context: context,
+    );
+    if (namesInThisLibrary.length == 1) {
+      return (importCombinator, null);
     }
+    var importCombinatorMultiple = _ImportLibraryCombinatorMultiple(
+      uri,
+      combinators,
+      namesInThisLibrary,
+      removePrefix: importPrefix == null,
+      context: context,
+    );
     return (importCombinator, importCombinatorMultiple);
   }
 
@@ -435,8 +415,6 @@ class ImportLibrary extends MultiCorrectionProducer {
       for (var instantiatedExtension in instantiatedExtensions) {
         // If the import has a combinator that needs to be updated, then offer
         // to update it.
-        // TODO(FMorschel): We should fix all combinators for the import, if
-        // we don't, we may not import at all.
         var libraryElement = import.importedLibrary2;
         if (libraryElement == null) {
           continue;
@@ -857,11 +835,11 @@ enum _ImportKind {
 class _ImportLibraryCombinator extends _ImportLibraryCombinatorMultiple {
   _ImportLibraryCombinator(
     String libraryName,
-    NamespaceCombinator combinator,
+    List<NamespaceCombinator> combinators,
     String updatedName, {
     super.removePrefix,
     required super.context,
-  }) : super(libraryName, combinator, [updatedName]);
+  }) : super(libraryName, combinators, [updatedName]);
 
   @override
   List<String> get fixArguments => [_updatedNames.first, _libraryName];
@@ -875,15 +853,15 @@ class _ImportLibraryCombinator extends _ImportLibraryCombinatorMultiple {
 class _ImportLibraryCombinatorMultiple extends ResolvedCorrectionProducer {
   final String _libraryName;
 
-  final NamespaceCombinator _combinator;
+  final List<NamespaceCombinator> _combinators;
 
   final List<String> _updatedNames;
 
   final bool _removePrefix;
 
   _ImportLibraryCombinatorMultiple(
     this._libraryName,
-    this._combinator,
+    this._combinators,
     this._updatedNames, {
     bool removePrefix = false,
     required super.context,
@@ -910,50 +888,57 @@ class _ImportLibraryCombinatorMultiple extends ResolvedCorrectionProducer {
 
   @override
   Future<void> compute(ChangeBuilder builder) async {
-    Set<String> finalNames = SplayTreeSet<String>();
-    int offset;
-    int length;
-    Keyword keyword;
-    if (_combinator case ShowElementCombinator(shownNames: var names)) {
-      finalNames.addAll(names);
-      offset = _combinator.offset;
-      length = _combinator.end - offset;
-      finalNames.addAll(_updatedNames);
-      keyword = Keyword.SHOW;
-    } else if (_combinator case HideElementCombinator(hiddenNames: var names)) {
-      finalNames.addAll(names);
-      offset = _combinator.offset;
-      length = _combinator.end - offset;
-      finalNames.removeAll(_updatedNames);
-      keyword = Keyword.HIDE;
-    } else {
-      return;
-    }
-    var newCombinatorCode = '';
-    if (finalNames.isNotEmpty) {
-      newCombinatorCode = ' ${keyword.lexeme} ${finalNames.join(', ')}';
-    }
-    var libraryPath = unitResult.libraryElement2.firstFragment.source.fullName;
-    await builder.addDartFileEdit(libraryPath, (builder) {
-      builder.addSimpleReplacement(
-        SourceRange(offset - 1, length + 1),
-        newCombinatorCode,
-      );
-      if (_removePrefix) {
-        AstNode? prefix;
-        if (node case NamedType(:var importPrefix?)) {
-          prefix = importPrefix;
-        } else if (node case PrefixedIdentifier(:var prefix)) {
-          prefix = prefix;
-        } else {
-          return;
-        }
-        if (prefix == null) {
-          return;
-        }
-        builder.addDeletion(range.node(prefix));
+    var codeStyleOptions = getCodeStyleOptions(unitResult.file);
+
+    for (var combinator in _combinators) {
+      var combinatorNames = <String>{};
+      var offset = combinator.offset;
+      var length = combinator.end - offset;
+
+      Keyword keyword;
+      switch (combinator) {
+        case ShowElementCombinator(shownNames: var names):
+          combinatorNames.addAll(names);
+          combinatorNames.addAll(_updatedNames);
+          keyword = Keyword.SHOW;
+        case HideElementCombinator(hiddenNames: var names):
+          combinatorNames.addAll(names);
+          combinatorNames.removeAll(_updatedNames);
+          keyword = Keyword.HIDE;
       }
-    });
+
+      var names =
+          codeStyleOptions.sortCombinators
+              ? combinatorNames.sorted()
+              : combinatorNames;
+
+      var newCombinatorCode = '';
+      if (names.isNotEmpty) {
+        newCombinatorCode = ' ${keyword.lexeme} ${names.join(', ')}';
+      }
+      var libraryPath =
+          unitResult.libraryElement2.firstFragment.source.fullName;
+      await builder.addDartFileEdit(libraryPath, (builder) {
+        builder.addSimpleReplacement(
+          SourceRange(offset - 1, length + 1),
+          newCombinatorCode,
+        );
+        if (_removePrefix) {
+          AstNode? prefix;
+          if (node case NamedType(:var importPrefix?)) {
+            prefix = importPrefix;
+          } else if (node case PrefixedIdentifier(:var prefix)) {
+            prefix = prefix;
+          } else {
+            return;
+          }
+          if (prefix == null) {
+            return;
+          }
+          builder.addDeletion(range.node(prefix));
+        }
+      });
+    }
   }
 }
 

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

@@ -5,6 +5,7 @@
 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:linter/src/lint_names.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import 'fix_processor.dart';
@@ -457,6 +458,55 @@ void f() {
 ''');
   }
 
+  Future<void> test_lint_active() async {
+    createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]);
+    newFile('$testPackageLibPath/lib.dart', '''
+class A {}
+class B {}
+class C {}
+class D {}
+''');
+    await resolveTestCode(r'''
+// ignore: combinators_ordering
+import 'lib.dart' hide C, D, B;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+    await assertHasFix(r'''
+// ignore: combinators_ordering
+import 'lib.dart' hide B, D;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+  }
+
+  // Two hides, one that should be removed entirely and a show that should be
+  // updated. Even though the show is not part of these tests, it should be
+  // fixed too for making the import correct.
+  Future<void> test_multiple_combinators() async {
+    newFile('$testPackageLibPath/lib.dart', '''
+class A {}
+class B {}
+class C {}
+''');
+    await resolveTestCode(r'''
+// ignore: multiple_combinators
+import 'lib.dart' hide B, C show A hide C;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+    await assertHasFix(r'''
+// ignore: multiple_combinators
+import 'lib.dart' hide B show A, C;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+  }
+
   Future<void> test_multipleHide_extension_getter() async {
     newFile('$testPackageLibPath/lib.dart', '''
 class C {}

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

@@ -5,6 +5,7 @@
 import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:linter/src/lint_names.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import 'fix_processor.dart';
@@ -47,7 +48,7 @@ void f(A a, B b, C c) {
     );
     await assertHasFix(
       r'''
-import 'lib.dart' show A, B, C;
+import 'lib.dart' show A, C, B;
 void f(A a, B b, C c) {
   print('$a');
 }
@@ -252,7 +253,7 @@ void f(A a1) {
     );
     await assertHasFix(
       r'''
-import 'lib.dart' show A, E, a;
+import 'lib.dart' show A, a, E;
 void f(A a1) {
   print('$a1 ${E.m()} $a');
 }
@@ -402,14 +403,59 @@ void f(String s, lib2.C c) {
 ''');
     await assertHasFix('''
 import 'package:test/lib.dart' as lib show C;
-import 'package:test/lib.dart' show C, E;
+import 'package:test/lib.dart' show E, C;
 
 void f(String s, C c) {
   s.m;
 }
 ''');
   }
 
+  Future<void> test_lint_active() async {
+    createAnalysisOptionsFile(lints: [LintNames.combinators_ordering]);
+    newFile('$testPackageLibPath/lib.dart', '''
+class A {}
+class B {}
+class C {}
+''');
+    await resolveTestCode(r'''
+import 'lib.dart' show B, C;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+    await assertHasFix(r'''
+import 'lib.dart' show A, B, C;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+  }
+
+  // Two shows, and a hide that should be updated. Even though the hide is not
+  // part of these tests, it should be fixed too for making the import correct.
+  Future<void> test_multiple_combinators() async {
+    newFile('$testPackageLibPath/lib.dart', '''
+class A {}
+class B {}
+class C {}
+''');
+    await resolveTestCode(r'''
+// ignore: multiple_combinators
+import 'lib.dart' show A show A, B hide B, C;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+    await assertHasFix(r'''
+// ignore: multiple_combinators
+import 'lib.dart' show A, C show A, B, C hide B;
+void f(A a, C c) {
+  print('$a $c');
+}
+''');
+  }
+
   Future<void> test_override_samePackage() async {
     newFile('$testPackageLibPath/lib.dart', '''
 class A {}