[DAS] Fixes multiple fixes for insertions in multi-file library

Merges the test in https://dart-review.googlesource.com/c/sdk/+/401840 that is related to https://dart-review.googlesource.com/c/sdk/+/401180, but in the LSP handler where it can be more easily debugged. [email protected] Fixes #59572 Change-Id: I03874be3671dfbfa65a2f0b9f3916ecafcad9400 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401865 Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Phil Quitslund <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
dart-lang · Jan 6, 2025 · 1c4d7e1 · 1c4d7e1
1 parent 2d2901b
commit 1c4d7e1
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 32 deletions.
diff --git a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart
@@ -534,8 +534,9 @@ class BulkFixProcessor {
           }
         }
         if (resolvedLibrary is ResolvedLibraryResult) {
-          await _fixErrorsInLibrary(
+          await _fixErrorsInLibraryAt(
             resolvedLibrary,
+            path: path,
             stopAfterFirst: stopAfterFirst,
           );
           if (isCancelled || (stopAfterFirst && changeMap.hasFixes)) {
@@ -591,12 +592,14 @@ class BulkFixProcessor {
 
   /// Uses the change [builder] to create fixes for the diagnostics in the
   /// library associated with the analysis [libraryResult].
-  Future<void> _fixErrorsInLibrary(
+  Future<void> _fixErrorsInLibraryAt(
     ResolvedLibraryResult libraryResult, {
+    required String path,
     bool stopAfterFirst = false,
     bool autoTriggered = false,
   }) async {
-    for (var unitResult in libraryResult.units) {
+    var unitResult = libraryResult.unitWithPath(path);
+    if (unitResult != null) {
       await _fixErrorsInLibraryUnit(
         libraryResult,
         unitResult,

diff --git a/pkg/analysis_server/test/integration/edit/bulk_fixes_test.dart b/pkg/analysis_server/test/integration/edit/bulk_fixes_test.dart
@@ -37,7 +37,6 @@ class B extends A {
     expect(result.edits, hasLength(1));
   }
 
-  @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/59572')
   Future<void> test_bulk_fix_with_parts() async {
     writeFile(sourcePath(file_paths.analysisOptionsYaml), '''
 linter:

diff --git a/pkg/analysis_server/test/lsp/commands/fix_all_in_workspace_test.dart b/pkg/analysis_server/test/lsp/commands/fix_all_in_workspace_test.dart
@@ -133,6 +133,62 @@ void f() {
     }
   }
 
+  Future<void> test_partFile_issue59572() async {
+    newFile(analysisOptionsPath, '''
+linter:
+  rules:
+    - empty_statements
+    - prefer_const_constructors
+    ''');
+
+    newFile(join(projectFolderPath, 'lib', 'part.dart'), '''
+part of 'main.dart';
+
+class C {
+  const C();
+}
+
+C b() {
+  // dart fix should only add a single const
+  return C();
+}
+''');
+
+    newFile(join(projectFolderPath, 'lib', 'main.dart'), '''
+part 'part.dart';
+
+void a() {
+  // need to trigger a lint in main.dart for the bug to happen
+  ;
+  b();
+}
+''');
+
+    await initialize();
+    await verifyCommandEdits(Command(command: commandId, title: 'UNUSED'), '''
+>>>>>>>>>> lib/main.dart
+>>>>>>>>>>   Remove empty statement: lines 5-6
+part 'part.dart';
+
+void a() {
+  // need to trigger a lint in main.dart for the bug to happen
+  b();
+}
+>>>>>>>>>> lib/part.dart
+>>>>>>>>>>   Add 'const' modifier: line 9
+part of 'main.dart';
+
+class C {
+  const C();
+}
+
+C b() {
+  // dart fix should only add a single const
+  return const C();
+}
+''');
+  }
+
   Future<void> test_serverAdvertisesCommand() async {
     await initialize();
     expect(

diff --git a/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart b/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart
@@ -119,31 +119,32 @@ part 'a.dart';
     );
 
     newFile('$testPackageLibPath/a.dart', '''
-part of 'test.dart';
+part 'test.dart';
+part 'b.dart';
 
-class A { }
+class C{}
 
-var a = new A();
+var c = C();
 ''');
 
     newFile('$testPackageLibPath/b.dart', '''
-part of 'test.dart';
+part of 'a.dart';
 
 class B { }
 
 var b = new B();
 ''');
 
     await resolveTestCode('''
-part 'a.dart';
-part 'b.dart';
+part of 'a.dart';
 
-class C{}
-var c = new C();
+class A { }
+
+var a = new A();
 ''');
 
     expect(await computeHasFixes(), isTrue);
-    expect(processor.changeMap.libraryMap.length, 3);
+    expect(processor.changeMap.libraryMap.length, 1);
   }
 
   /// https://github.com/dart-lang/sdk/issues/59572
@@ -177,8 +178,10 @@ void a() {
 ''');
 
     expect(await computeHasFixes(), isTrue);
-    expect(processor.changeMap.libraryMap.length, 2);
-    expect(processor.fixDetails.length, 2);
+    expect(processor.changeMap.libraryMap.length, 1);
+    expect(processor.fixDetails.length, 1);
+    var details = processor.fixDetails;
+    expect(details.first.fixes, hasLength(1));
   }
 
   Future<void> test_hasFixes_stoppedAfterFirst() async {
@@ -284,10 +287,12 @@ import 'package:b/b.dart';
 import 'package:c/c.dart';
 import 'package:d/d.dart';
 import 'package:test/lib.dart';
+
 void f() {
   print(C());
 }
 ''');
+
     await getResolvedUnit(testFile);
     await assertFixPubspec(content, expected);
   }
@@ -385,6 +390,7 @@ import 'package:b/b.dart';
 import 'package:c/c.dart';
 import 'package:d/d.dart';
 import 'package:test/lib.dart';
+
 void f() {
   print(C());
 }

diff --git a/pkg/dartdev/test/commands/fix_test.dart b/pkg/dartdev/test/commands/fix_test.dart
@@ -16,19 +16,6 @@ void main() {
 final bullet = '•';
 final nonAnsiBullet = '-';
 
-/// Allow for different bullets; depending on how the test harness is run,
-/// subprocesses may decide to give us ansi bullets or normal bullets.
-/// TODO(jcollins): find a way to detect which one we should be expecting.
-Matcher stringContainsInOrderWithVariableBullets(List<String> substrings) {
-  var substitutedSubstrings = substrings;
-  if (substrings.any((s) => s.contains(bullet))) {
-    substitutedSubstrings =
-        substrings.map((s) => s.replaceAll(bullet, nonAnsiBullet)).toList();
-  }
-  return anyOf(stringContainsInOrder(substrings),
-      stringContainsInOrder(substitutedSubstrings));
-}
-
 void defineFix() {
   TestProject? p;
   late ProcessResult result;
@@ -404,12 +391,54 @@ linter:
     });
 
     test(
-      '--apply --code=(multiple) [part file]',
+      '--apply part.dart',
       () async {
         p = project(
           mainSrc: '''
 part 'part.dart';
+void a() {
+  b();
+}
+''',
+          analysisOptions: '''
+linter:
+  rules:
+    - prefer_const_constructors
+''',
+        );
+        p!.file('lib/part.dart', '''
+part of 'main.dart';
+Stream<String> b() {
+  return Stream.empty();
+}
+''');
+        var result = await p!.runFix([
+          '--apply',
+          '--code',
+          'empty_statements',
+          '--code',
+          'prefer_const_constructors',
+          './lib/part.dart'
+        ], workingDir: p!.dirPath);
+        expect(result.exitCode, 0);
+        expect(result.stderr, isEmpty);
+        expect(
+            result.stdout,
+            stringContainsInOrderWithVariableBullets([
+              'Applying fixes...',
+              'part.dart',
+              '  prefer_const_constructors $bullet 1 fix',
+              '1 fix made in 1 file.',
+            ]));
+      },
+    );
 
+    test(
+      '--apply --code=(multiple) [part file]',
+      () async {
+        p = project(
+          mainSrc: '''
+part 'part.dart';
 void a() {
   // need to trigger a lint in main.dart for the bug to happen
   ;
@@ -425,7 +454,6 @@ linter:
         );
         p!.file('lib/part.dart', '''
 part of 'main.dart';
-
 Stream<String> b() {
   // dart fix should only add a single const
   return Stream.empty();
@@ -439,7 +467,6 @@ Stream<String> b() {
           'prefer_const_constructors',
           '.'
         ], workingDir: p!.dirPath);
-
         expect(result.exitCode, 0);
         expect(result.stderr, isEmpty);
         expect(
@@ -453,7 +480,6 @@ Stream<String> b() {
               '2 fixes made in 2 files.',
             ]));
       },
-      skip: 'Failing: https://github.com/dart-lang/sdk/issues/59572',
     );
 
     test('--apply --code=(multiple: comma-delimited)', () async {
@@ -766,3 +792,17 @@ class A {
     });
   });
 }
+
+/// Allow for different bullets; depending on how the test harness is run,
+/// subprocesses may decide to give us ansi bullets or normal bullets.
+/// TODO(jcollins): find a way to detect which one we should be expecting.
+Matcher stringContainsInOrderWithVariableBullets(List<String> substrings) {
+  var substringMatcher = stringContainsInOrder(substrings);
+  if (substrings.any((s) => s.contains(bullet))) {
+    var alternatives = [
+      for (var s in substrings) s.replaceAll(bullet, nonAnsiBullet)
+    ];
+    return anyOf(substringMatcher, stringContainsInOrder(alternatives));
+  }
+  return substringMatcher;
+}