Skip to content

Commit 1861eac

Browse files
DanTupCommit Bot
authored andcommitted
[analysis_server] Fix an edit conflict renaming folders containing files that reference siblings
Renaming a folder with files that referenced each other would fail with edit conflicts. The pass over the first file would update the reference to the other (going from "a.dart" to "../old/a.dart" as if the other file wasn't moving too) and then then when processing the second file, it would try to update the import back. It doesn't seem to ever make sense to update relative paths when the "other" file is also in the folder being renamed, so this skips them. Change-Id: I14c09977aa66017eecac5922d0e3caacf59350fb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247541 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent 0d53291 commit 1861eac

File tree

2 files changed

+46
-0
lines changed

2 files changed

+46
-0
lines changed

pkg/analysis_server/lib/src/services/refactoring/move_file.dart

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,14 @@ class MoveFileRefactoringImpl extends RefactoringImpl
2626
late AnalysisDriver driver;
2727
late AnalysisSession _session;
2828

29+
/// The path provided by the client to be renamed from.
30+
///
31+
/// May be a file or folder path.
2932
late String oldFile;
33+
34+
/// The path provided by the client to be renamed to.
35+
///
36+
/// May be a file or folder path.
3037
late String newFile;
3138

3239
final packagePrefixedStringPattern = RegExp(r'''^r?['"]+package:''');
@@ -144,6 +151,19 @@ class MoveFileRefactoringImpl extends RefactoringImpl
144151
await changeBuilder.addDartFileEdit(definingUnitResult.path, (builder) {
145152
for (var directive in definingUnitResult.unit.directives) {
146153
if (directive is UriBasedDirective) {
154+
// If the import is relative and the referenced file is also in
155+
// the moved folder, no update is necessary.
156+
var uriContent = directive.uriContent;
157+
var uriFullPath = directive.uriSource?.fullName;
158+
if (uriContent != null &&
159+
uriFullPath != null &&
160+
pathContext.isRelative(uriContent) &&
161+
// `oldFile` is used here and not `oldDir` because we care
162+
// about whether this is within the folder being renamed, not
163+
// the folder for this specific resource.
164+
pathContext.isWithin(oldFile, uriFullPath)) {
165+
continue;
166+
}
147167
_updateUriReference(builder, directive, oldDir, newDir);
148168
}
149169
}
@@ -174,6 +194,16 @@ class MoveFileRefactoringImpl extends RefactoringImpl
174194
await refactoringWorkspace.searchEngine.searchReferences(element);
175195
var references = getSourceReferences(matches);
176196
for (var reference in references) {
197+
// If the import is relative and the referencing file is also in
198+
// the moved folder, no update is necessary.
199+
var uriFullPath = reference.file;
200+
if (!_isPackageReference(reference) &&
201+
// `oldFile` is used here and not `oldDir` because we care
202+
// about whether this is within the folder being renamed, not
203+
// the folder for this specific resource.
204+
pathContext.isWithin(oldFile, uriFullPath)) {
205+
continue;
206+
}
177207
await changeBuilder.addDartFileEdit(reference.file, (builder) {
178208
var newUri = _computeNewUri(reference, newPath);
179209
builder.addSimpleReplacement(reference.range, "'$newUri'");

pkg/analysis_server/test/services/refactoring/move_file_test.dart

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,22 @@ import 'package:test/new/nested/d.dart';
375375
'${convertPath('/tmp')} does not belong to an analysis root.');
376376
}
377377

378+
Future<void> test_folder_siblingFiles() async {
379+
testFile = convertPath('/home/test/lib/old/a.dart');
380+
final pathB = convertPath('/home/test/lib/old/b.dart');
381+
addSource(pathB, '');
382+
await resolveTestCode('''
383+
import 'a.dart';
384+
''');
385+
// Rename the whole 'old' folder to 'new''.
386+
_createRefactoring('/home/test/lib/new', oldFile: '/home/test/lib/old');
387+
await _assertSuccessfulRefactoring();
388+
// No changes, because import was a relative path and both files are inside
389+
// the renamed folder.
390+
assertNoFileChange(testFile);
391+
assertNoFileChange(pathB);
392+
}
393+
378394
Future<void> test_nonexistent_file_returns_failure() async {
379395
await resolveTestFile();
380396
_createRefactoring(convertPath('/home/test/test_missing_new.dart'),

0 commit comments

Comments
 (0)