Change in dart/sdk[main]: Basic support for folders in MOVE_FILE refactor
10 views
Skip to first unread message
Danny Tuppeny (Gerrit)
Dec 8, 2021, 1:50:09 PM12/8/21
to Brian Wilkerson, rev...@dartlang.org, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Brian Wilkerson.
Danny Tuppeny would like Brian Wilkerson to review this change.
Basic support for folders in MOVE_FILE refactor
Change-Id: If960592199526df3dcd7f35c8a2396060b27e46e
---
M pkg/analysis_server/doc/api.html
M pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
M pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
M pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
M pkg/analysis_server/test/services/refactoring/move_file_test.dart
M pkg/analysis_server/lib/src/services/refactoring/move_file.dart
M pkg/analysis_server/tool/spec/spec_input.html
M pkg/analysis_server/lib/src/edit/edit_domain.dart
M pkg/analysis_server/test/edit/refactoring_test.dart
M pkg/analysis_server/lib/protocol/protocol_constants.dart
10 files changed, 231 insertions(+), 87 deletions(-)
diff --git a/pkg/analysis_server/doc/api.html b/pkg/analysis_server/doc/api.html
index f825a1e..d3a95ac 100644
--- a/pkg/analysis_server/doc/api.html
+++ b/pkg/analysis_server/doc/api.html
@@ -109,7 +109,7 @@
<body>
<h1>Analysis Server API Specification</h1>
<h1 style="color:#999999">Version
- 1.32.8
+ 1.32.9
</h1>
<p>
This document contains a specification of the API provided by the
@@ -236,6 +236,10 @@
ignoring the item or treating it with some default/fallback handling.
</p>
<h3>Changelog</h3>
+<h4>1.32.9</h4>
+<ul>
+ <li>The <tt>MOVE_FILE</tt> refactor now supports moving/renaming folders.</li>
+</ul>
<h4>1.32.8</h4>
<ul>
<li>Added <tt>server.cancelRequest</tt> to allow clients to request cancellation
@@ -6027,10 +6031,15 @@
</p>
</dd></dl></dd><dt class="refactoring">MOVE_FILE</dt><dd>
<p>
- Move the given file and update all of the references to that file
- and from it. The move operation is supported in general case - for
- renaming a file in the same folder, moving it to a different folder
- or both.
+ Move the given file or folder and update all of the references to
+ and from thit. The move operation is supported in general case -
+ for renaming an item in the same folder, moving it to a different
+ folder or both.
+
+ Moving or renaming large folders may take time and clients should
+ consider showing an indicator to the user with the ability to
+ cancel the request (which can be done using
+ <tt>server.cancelRequest</tt>).
</p>
<p>
The refactoring must be activated before an actual file moving
diff --git a/pkg/analysis_server/lib/protocol/protocol_constants.dart b/pkg/analysis_server/lib/protocol/protocol_constants.dart
index 00b690a..ae1bdb6 100644
--- a/pkg/analysis_server/lib/protocol/protocol_constants.dart
+++ b/pkg/analysis_server/lib/protocol/protocol_constants.dart
@@ -6,7 +6,7 @@
// To regenerate the file, use the script
// "pkg/analysis_server/tool/spec/generate_files".
-const String PROTOCOL_VERSION = '1.32.8';
+const String PROTOCOL_VERSION = '1.32.9';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES = 'analysis.analyzedFiles';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES_DIRECTORIES = 'directories';
diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart
index ab7cbeb..62f10ce 100644
--- a/pkg/analysis_server/lib/src/edit/edit_domain.dart
+++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart
@@ -356,7 +356,7 @@
getFixes(request);
return Response.DELAYED_RESPONSE;
} else if (requestName == EDIT_REQUEST_GET_REFACTORING) {
- return _getRefactoring(request);
+ return _getRefactoring(request, cancellationToken);
} else if (requestName == EDIT_REQUEST_IMPORT_ELEMENTS) {
importElements(request);
return Response.DELAYED_RESPONSE;
@@ -877,7 +877,8 @@
}
}
- Response _getRefactoring(Request request) {
+ Response _getRefactoring(
+ Request request, CancellationToken cancellationToken) {
final refactoringManager = this.refactoringManager;
if (refactoringManager == null) {
return Response.unsupportedFeature(request.id, 'Search is not enabled.');
@@ -886,7 +887,7 @@
refactoringManager.cancel();
_newRefactoringManager();
}
- refactoringManager.getRefactoring(request);
+ refactoringManager.getRefactoring(request, cancellationToken);
return Response.DELAYED_RESPONSE;
}
@@ -985,7 +986,7 @@
_reset();
}
- void getRefactoring(Request _request) {
+ void getRefactoring(Request _request, CancellationToken cancellationToken) {
// prepare for processing the request
request = _request;
final result = this.result = EditGetRefactoringResult(
@@ -1002,7 +1003,8 @@
?.sendEvent('refactor', params.kind.name.toLowerCase());
runZonedGuarded(() async {
- await _init(params.kind, file, params.offset, params.length);
+ await _init(
+ params.kind, file, params.offset, params.length, cancellationToken);
if (initStatus.hasFatalError) {
feedback = null;
_sendResultResponse();
@@ -1088,8 +1090,8 @@
}
}
- Future<void> _createRefactoringFromKind(
- String file, int offset, int length) async {
+ Future<void> _createRefactoringFromKind(String file, int offset, int length,
+ CancellationToken cancellationToken) async {
if (kind == RefactoringKind.CONVERT_GETTER_TO_METHOD) {
var resolvedUnit = await server.getResolvedUnit(file);
if (resolvedUnit != null) {
@@ -1156,11 +1158,9 @@
);
}
} else if (kind == RefactoringKind.MOVE_FILE) {
- var resolvedUnit = await server.getResolvedUnit(file);
- if (resolvedUnit != null) {
- refactoring = MoveFileRefactoring(
- server.resourceProvider, refactoringWorkspace, resolvedUnit, file);
- }
+ refactoring = MoveFileRefactoring(
+ server.resourceProvider, refactoringWorkspace, file)
+ ..cancellationToken = cancellationToken;
} else if (kind == RefactoringKind.RENAME) {
var resolvedUnit = await server.getResolvedUnit(file);
if (resolvedUnit != null) {
@@ -1183,8 +1183,8 @@
/// Initializes this context to perform a refactoring with the specified
/// parameters. The existing [Refactoring] is reused or created as needed.
- Future _init(
- RefactoringKind kind, String file, int offset, int length) async {
+ Future _init(RefactoringKind kind, String file, int offset, int length,
+ CancellationToken cancellationToken) async {
// check if we can continue with the existing Refactoring instance
if (this.kind == kind &&
this.file == file &&
@@ -1203,7 +1203,7 @@
throw 'A simulated refactoring exception - init.';
}
// create a new Refactoring instance
- await _createRefactoringFromKind(file, offset, length);
+ await _createRefactoringFromKind(file, offset, length, cancellationToken);
final refactoring = this.refactoring;
if (refactoring == null) {
initStatus = RefactoringStatus.fatal('Unable to create a refactoring');
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
index d08152f..6cbcd6c 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
@@ -23,7 +23,10 @@
Future<ErrorOr<WorkspaceEdit?>> handle(
RenameFilesParams params, CancellationToken token) async {
final files = params.files;
- // For performance reasons, only single-file rename/moves are currently supported.
+ // Only single-file rename/moves are currently supported.
+ // TODO(dantup): Tweak this when VS Code can correctly pass us cancellation
+ // requests to not check for .dart to also support folders (although we
+ // may still only support a single entry initially).
if (files.length > 1 || files.any((f) => !f.oldUri.endsWith('.dart'))) {
return success(null);
}
@@ -31,20 +34,17 @@
final file = files.single;
final oldPath = pathOfUri(Uri.tryParse(file.oldUri));
final newPath = pathOfUri(Uri.tryParse(file.newUri));
+
return oldPath.mapResult((oldPath) =>
- newPath.mapResult((newPath) => _renameFile(oldPath, newPath)));
+ newPath.mapResult((newPath) => _renameFile(oldPath, newPath, token)));
}
Future<ErrorOr<WorkspaceEdit?>> _renameFile(
- String oldPath, String newPath) async {
- final resolvedUnit = await server.getResolvedUnit(oldPath);
- if (resolvedUnit == null) {
- return success(null);
- }
-
- final refactoring = MoveFileRefactoring(server.resourceProvider,
- server.refactoringWorkspace, resolvedUnit, oldPath)
- ..newFile = newPath;
+ String oldPath, String newPath, CancellationToken token) async {
+ final refactoring = MoveFileRefactoring(
+ server.resourceProvider, server.refactoringWorkspace, oldPath)
+ ..newFile = newPath
+ ..cancellationToken = token;
// If we're unable to update imports for a rename, we should silently do
// nothing rather than interrupt the users file rename with an error.
diff --git a/pkg/analysis_server/lib/src/services/refactoring/move_file.dart b/pkg/analysis_server/lib/src/services/refactoring/move_file.dart
index f3cae42..157b370 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/move_file.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/move_file.dart
@@ -6,6 +6,7 @@
import 'package:analysis_server/src/services/correction/status.dart';
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/refactoring/refactoring_internal.dart';
+import 'package:analysis_server/src/utilities/progress.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/file_system/file_system.dart';
@@ -22,18 +23,22 @@
final ResourceProvider resourceProvider;
final pathos.Context pathContext;
final RefactoringWorkspace refactoringWorkspace;
- final ResolvedUnitResult resolvedUnit;
late AnalysisDriver driver;
late String oldFile;
late String newFile;
+ CancellationToken? cancellationToken;
+
final packagePrefixedStringPattern = RegExp(r'''^r?['"]+package:''');
- MoveFileRefactoringImpl(this.resourceProvider, this.refactoringWorkspace,
- this.resolvedUnit, this.oldFile)
+ MoveFileRefactoringImpl(
+ this.resourceProvider, this.refactoringWorkspace, this.oldFile)
: pathContext = resourceProvider.pathContext;
+ bool get isCancellationRequested =>
+ cancellationToken?.isCancellationRequested ?? false;
+
@override
String get refactoringName => 'Move File';
@@ -54,7 +59,7 @@
}
driver = drivers.first;
- if (!driver.resourceProvider.getFile(oldFile).exists) {
+ if (!driver.resourceProvider.getResource(oldFile).exists) {
return RefactoringStatus.fatal('$oldFile does not exist.');
}
@@ -68,24 +73,46 @@
@override
Future<SourceChange> createChange() async {
- var changeBuilder = ChangeBuilder(session: resolvedUnit.session);
+ var changeBuilder = ChangeBuilder(session: driver.currentSession);
+
+ final resource = driver.resourceProvider.getResource(oldFile);
+ await _appendChangesForResource(changeBuilder, resource, newFile);
+
+ // If cancellation was requested the results may be incomplete so return
+ // a new empty change instead of a partial one with a descriptive name
+ // so it's clear from any logs that cancellation was processed.
+ if (isCancellationRequested) {
+ return SourceChange('Refactor cancelled');
+ }
+
+ return changeBuilder.sourceChange;
+ }
+
+ Future<void> _appendChangeForFile(
+ ChangeBuilder changeBuilder, File file, String newPath) async {
+ var oldPath = file.path;
+ var oldDir = pathContext.dirname(oldPath);
+ var newDir = pathContext.dirname(newPath);
+
+ final resolvedUnit = await driver.getResult(file.path);
+ if (resolvedUnit is! ResolvedUnitResult) {
+ return;
+ }
+
var element = resolvedUnit.unit.declaredElement;
if (element == null) {
- return changeBuilder.sourceChange;
+ return;
}
var libraryElement = element.library;
- final oldDir = pathContext.dirname(oldFile);
- final newDir = pathContext.dirname(newFile);
-
// If this element is a library, update outgoing references inside the file.
if (element == libraryElement.definingCompilationUnit) {
// Handle part-of directives in this library
var libraryResult = await driver.currentSession
.getResolvedLibraryByElement(libraryElement);
if (libraryResult is! ResolvedLibraryResult) {
- return changeBuilder.sourceChange;
+ return;
}
var definingUnitResult = libraryResult.units.first;
for (var result in libraryResult.units) {
@@ -99,9 +126,7 @@
await changeBuilder.addDartFileEdit(
result.unit.declaredElement!.source.fullName, (builder) {
partOfs.forEach((uri) {
- var newLocation =
- pathContext.join(newDir, pathos.basename(newFile));
- var newUri = _getRelativeUri(newLocation, oldDir);
+ var newUri = _getRelativeUri(newPath, oldDir);
builder.addSimpleReplacement(
SourceRange(uri.offset, uri.length), "'$newUri'");
});
@@ -145,27 +170,41 @@
var references = getSourceReferences(matches);
for (var reference in references) {
await changeBuilder.addDartFileEdit(reference.file, (builder) {
- var newUri = _computeNewUri(reference);
+ var newUri = _computeNewUri(reference, newPath);
builder.addSimpleReplacement(reference.range, "'$newUri'");
});
}
-
- return changeBuilder.sourceChange;
}
- /// Computes the URI to use to reference [newFile] from [reference].
- String _computeNewUri(SourceReference reference) {
+ Future<void> _appendChangesForResource(
+ ChangeBuilder changeBuilder, Resource resource, String newPath) async {
+ if (isCancellationRequested) {
+ return;
+ }
+
+ if (resource is File) {
+ await _appendChangeForFile(changeBuilder, resource, newPath);
+ } else if (resource is Folder) {
+ for (final child in resource.getChildren()) {
+ await _appendChangesForResource(changeBuilder, child,
+ pathContext.join(newPath, pathContext.basename(child.path)));
+ }
+ }
+ }
+
+ /// Computes the URI to use to reference [newPath] from [reference].
+ String _computeNewUri(SourceReference reference, String newPath) {
var refDir = pathContext.dirname(reference.file);
// Try to keep package: URI
if (_isPackageReference(reference)) {
- var restoredUri = driver.sourceFactory.pathToUri(newFile);
+ var restoredUri = driver.sourceFactory.pathToUri(newPath);
// If the new URI is not a package: URI, fall back to computing a relative
// URI below.
if (restoredUri?.isScheme('package') ?? false) {
return restoredUri.toString();
}
}
- return _getRelativeUri(newFile, refDir);
+ return _getRelativeUri(newPath, refDir);
}
String _getRelativeUri(String path, String from) {
diff --git a/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart b/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
index 05e0eb3..39cc4d0 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
@@ -20,6 +20,7 @@
import 'package:analysis_server/src/services/refactoring/rename_local.dart';
import 'package:analysis_server/src/services/refactoring/rename_unit_member.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
+import 'package:analysis_server/src/utilities/progress.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
@@ -248,18 +249,16 @@
bool isAvailable();
}
-/// [Refactoring] to move/rename a file.
+/// [Refactoring] to move/rename a file or folder.
abstract class MoveFileRefactoring implements Refactoring {
/// Returns a new [MoveFileRefactoring] instance.
- factory MoveFileRefactoring(
- ResourceProvider resourceProvider,
- RefactoringWorkspace workspace,
- ResolvedUnitResult resolveResult,
- String oldFilePath) {
- return MoveFileRefactoringImpl(
- resourceProvider, workspace, resolveResult, oldFilePath);
+ factory MoveFileRefactoring(ResourceProvider resourceProvider,
+ RefactoringWorkspace workspace, String oldFilePath) {
+ return MoveFileRefactoringImpl(resourceProvider, workspace, oldFilePath);
}
+ set cancellationToken(CancellationToken token);
+
/// The new file path to which the given file is being moved.
set newFile(String newName);
}
diff --git a/pkg/analysis_server/test/edit/refactoring_test.dart b/pkg/analysis_server/test/edit/refactoring_test.dart
index bffa9fc..aa8c499 100644
--- a/pkg/analysis_server/test/edit/refactoring_test.dart
+++ b/pkg/analysis_server/test/edit/refactoring_test.dart
@@ -4,6 +4,7 @@
import 'package:analysis_server/protocol/protocol.dart';
import 'package:analysis_server/protocol/protocol_generated.dart';
+import 'package:analysis_server/src/domain_server.dart';
import 'package:analysis_server/src/edit/edit_domain.dart';
import 'package:analyzer_plugin/protocol/protocol_common.dart';
import 'package:test/test.dart';
@@ -27,15 +28,6 @@
});
}
-/// Wrapper around the test package's `fail` function.
-///
-/// Unlike the test package's `fail` function, this function is not annotated
-/// with @alwaysThrows, so we can call it at the top of a test method without
-/// causing the rest of the method to be flagged as dead code.
-void _fail(String message) {
- fail(message);
-}
-
@reflectiveTest
class ConvertGetterMethodToMethodTest extends _AbstractGetRefactoring_Test {
Future<void> test_function() {
@@ -1325,9 +1317,7 @@
class MoveFileTest extends _AbstractGetRefactoring_Test {
late MoveFileOptions options;
- @failingTest
- Future<void> test_OK() {
- _fail('The move file refactoring is not supported under the new driver');
+ Future<void> test_file_OK() {
newFile('/project/bin/lib.dart');
addTestFile('''
import 'dart:math';
@@ -1335,16 +1325,58 @@
''');
_setOptions('/project/test.dart');
return assertSuccessfulRefactoring(() {
- return _sendMoveRequest();
+ return _sendMoveRequest(testFile);
}, '''
import 'dart:math';
import 'bin/lib.dart';
''');
}
- Future<Response> _sendMoveRequest() {
+ Future<void> test_folder_cancel() {
+ newFile('/project/bin/original_folder/file.dart');
+ addTestFile('''
+import 'dart:math';
+import 'original_folder/file.dart';
+''');
+ _setOptions('/project/bin/new_folder');
+ return assertEmptySuccessfulRefactoring(() async {
+ return _sendAndCancelMoveRequest('/project/bin/original_folder');
+ });
+ }
+
+ Future<void> test_folder_OK() {
+ newFile('/project/bin/original_folder/file.dart');
+ addTestFile('''
+import 'dart:math';
+import 'original_folder/file.dart';
+''');
+ _setOptions('/project/bin/new_folder');
+ return assertSuccessfulRefactoring(() async {
+ return _sendMoveRequest('/project/bin/original_folder');
+ }, '''
+import 'dart:math';
+import 'new_folder/file.dart';
+''');
+ }
+
+ Future<Response> _cancelMoveRequest() {
+ // 0 is the id from _sendMoveRequest
+ // 1 is another aribtrary id for the cancel request
+ var request = ServerCancelRequestParams('0').toRequest('1');
+ return serverChannel.sendRequest(request);
+ }
+
+ Future<Response> _sendAndCancelMoveRequest(String item) async {
+ final responses = await Future.wait([
+ _sendMoveRequest(item),
+ _cancelMoveRequest(),
+ ]);
+ return responses.first;
+ }
+
+ Future<Response> _sendMoveRequest(String item) {
var request = EditGetRefactoringParams(
- RefactoringKind.MOVE_FILE, testFile, 0, 0, false,
+ RefactoringKind.MOVE_FILE, item, 0, 0, false,
options: options)
.toRequest('0');
return serverChannel.sendRequest(request);
@@ -2201,6 +2233,26 @@
class _AbstractGetRefactoring_Test extends AbstractAnalysisTest {
bool shouldWaitForFullAnalysis = true;
+ Future assertEmptySuccessfulRefactoring(
+ Future<Response> Function() requestSender,
+ {void Function(RefactoringFeedback?)? feedbackValidator}) async {
+ var result = await getRefactoringResult(requestSender);
+ assertResultProblemsOK(result);
+ if (feedbackValidator != null) {
+ feedbackValidator(result.feedback);
+ }
+ assertNoTestRefactoringResult(result);
+ }
+
+ /// Asserts that the given [EditGetRefactoringResult] does not have a change
+ /// for [testFile].
+ void assertNoTestRefactoringResult(EditGetRefactoringResult result) {
+ var change = result.change!;
+ if (change.edits.any((edit) => edit.file == testFile)) {
+ fail('Found a SourceFileEdit for $testFile in $change');
+ }
+ }
+
/// Asserts that [problems] has a single ERROR problem.
void assertResultProblemsError(List<RefactoringProblem> problems,
[String? message]) {
@@ -2292,7 +2344,9 @@
void setUp() {
super.setUp();
createProject();
- handler = EditDomainHandler(server);
- server.handlers = [handler];
+ server.handlers = [
+ EditDomainHandler(server),
+ ServerDomainHandler(server),
+ ];
}
}
diff --git a/pkg/analysis_server/test/services/refactoring/move_file_test.dart b/pkg/analysis_server/test/services/refactoring/move_file_test.dart
index 52fe0df..40c3b35 100644
--- a/pkg/analysis_server/test/services/refactoring/move_file_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/move_file_test.dart
@@ -340,9 +340,34 @@
assertNoFileChange(testFile);
}
- @failingTest
Future<void> test_folder_inside_project() async {
- fail('Not yet implemented/tested');
+ final pathA = convertPath('/home/test/lib/old/a.dart');
+ final pathB = convertPath('/home/test/lib/old/b.dart');
+ final pathC = convertPath('/home/test/lib/old/nested/c.dart');
+ final pathD = convertPath('/home/test/lib/old/nested/d.dart');
+ testFile = convertPath('/home/test/lib/test.dart');
+ addSource(pathA, '');
+ addSource(pathB, '');
+ addSource(pathC, '');
+ addSource(pathD, '');
+ verifyNoTestUnitErrors = false;
+ await resolveTestCode('''
+import 'old/a.dart';
+import 'package:test/old/b.dart';
+import 'old/nested/c.dart';
+import 'package:test/old/nested/d.dart';
+''');
+ // Rename the whole 'old' folder to 'new''.
+ _createRefactoring('/home/test/lib/new', oldFile: '/home/test/lib/old');
+ await _assertSuccessfulRefactoring();
+ assertNoFileChange(pathA);
+ assertNoFileChange(pathB);
+ assertFileChangeResult(testFile, '''
+import 'new/a.dart';
+import 'package:test/new/b.dart';
+import 'new/nested/c.dart';
+import 'package:test/new/nested/d.dart';
+''');
}
Future<void> test_folder_outside_workspace_returns_failure() async {
@@ -517,8 +542,8 @@
// Allow passing an oldName for when we don't want to rename testSource,
// but otherwise fall back to testSource.fullname
oldFile = convertPath(oldFile ?? testFile);
- refactoring = MoveFileRefactoring(
- resourceProvider, refactoringWorkspace, testAnalysisResult, oldFile);
+ refactoring =
+ MoveFileRefactoring(resourceProvider, refactoringWorkspace, oldFile);
refactoring.newFile = convertPath(newFile);
}
}
diff --git a/pkg/analysis_server/tool/spec/spec_input.html b/pkg/analysis_server/tool/spec/spec_input.html
index 5c0b821..590274a 100644
--- a/pkg/analysis_server/tool/spec/spec_input.html
+++ b/pkg/analysis_server/tool/spec/spec_input.html
@@ -7,7 +7,7 @@
<body>
<h1>Analysis Server API Specification</h1>
<h1 style="color:#999999">Version
- <version>1.32.8</version>
+ <version>1.32.9</version>
</h1>
<p>
This document contains a specification of the API provided by the
@@ -134,6 +134,10 @@
ignoring the item or treating it with some default/fallback handling.
</p>
<h3>Changelog</h3>
+<h4>1.32.9</h4>
+<ul>
+ <li>The <tt>MOVE_FILE</tt> refactor now supports moving/renaming folders.</li>
+</ul>
<h4>1.32.8</h4>
<ul>
<li>Added <tt>server.cancelRequest</tt> to allow clients to request cancellation
@@ -5856,10 +5860,15 @@
</refactoring>
<refactoring kind="MOVE_FILE">
<p>
- Move the given file and update all of the references to that file
- and from it. The move operation is supported in general case - for
- renaming a file in the same folder, moving it to a different folder
- or both.
+ Move the given file or folder and update all of the references to
+ and from thit. The move operation is supported in general case -
+ for renaming an item in the same folder, moving it to a different
+ folder or both.
+
+ Moving or renaming large folders may take time and clients should
+ consider showing an indicator to the user with the ability to
+ cancel the request (which can be done using
+ <tt>server.cancelRequest</tt>).
</p>
<p>
The refactoring must be activated before an actual file moving
diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
index 00b690a..ae1bdb6 100644
--- a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
+++ b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
@@ -6,7 +6,7 @@
// To regenerate the file, use the script
// "pkg/analysis_server/tool/spec/generate_files".
-const String PROTOCOL_VERSION = '1.32.8';
+const String PROTOCOL_VERSION = '1.32.9';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES = 'analysis.analyzedFiles';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES_DIRECTORIES = 'directories';
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Dec 8, 2021, 1:50:09 PM12/8/21
to rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Brian Wilkerson.
1 comment:
Patchset:
This adds support for renaming folders through MOVE_FILE. Right now, it's only supported in the original protocol (not LSP - I am waiting to see progress on a VS Code LSP Client change before enabling that) but I think IntelliJ could start using it. My recommendation is that when an editor passes a folder over, it shows some UI to the user indicating something is happening and with the option to cancel (which should call the new server.cancelRequest request to abort the rename on the server).
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Dec 9, 2021, 11:51:51 AM12/9/21
to Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
7 comments:
File pkg/analysis_server/doc/api.html:
Patch Set #11, Line 6035:
thit"thit" --> "it"
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart:
Patch Set #11, Line 31:
CancellationToken? cancellationToken;Consider moving this field and `isCancellationRequested` to `RefactoringImpl` so that it's available for all refactorings (even if it isn't currently being used by any of the others).
Patch Set #11, Line 76:
driver.currentSessionGetting the session from the driver is OK as long as the `resolvedUnit` is never accessed. If the resolved unit result is ever accessed, then this opens the door for getting results from two different sessions, which could result in inconsistent changes.
Patch Set #11, Line 78:
driver.resourceProviderIt isn't clear to me why we're accessing the resource provider from the driver when we have it stored in a field. I noticed that we're doing it elsewhere, but it seems unnecessary. (There should really only ever be one instance of resource provider, but if there's ever a need for more than one I would assume that we'd want to use the locally stored instance rather than the one used by the driver.)
Patch Set #11, Line 97:
driver.getResultBy bypassing the session and going directly to the driver the code is avoiding the consistency check that I mentioned above. I think we really need to capture the current session at the time we start to compute changes and use the same session everywhere.
File pkg/analysis_server/lib/src/services/refactoring/refactoring.dart:
Patch Set #11, Line 260:
cancellationTokenSimilarly, consider moving this to `Refactoring`.
File pkg/analysis_server/test/services/refactoring/move_file_test.dart:
Patch Set #11, Line 344:
convertPathNot required, but consider using `newFile`. It will return an instance of `File` with the converted path, which you can then use below. I think it will make the code a bit cleaner.
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Dec 14, 2021, 11:43:05 AM12/14/21
to rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Brian Wilkerson.
7 comments:
File pkg/analysis_server/doc/api.html:
Patch Set #11, Line 6035:
thit"thit" --> "it"
Done!
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart:
Patch Set #11, Line 31:
CancellationToken? cancellationToken;
Consider moving this field and `isCancellationRequested` to `RefactoringImpl` so that it's available […]
Done!
Patch Set #11, Line 76:
driver.currentSession
Getting the session from the driver is OK as long as the `resolvedUnit` is never accessed. […]
I'm not sure I fully understand the lifecycles of the driver/session here to know how best to change this.
From this and the comment below, I thought you meant we should capture `driver.currentSession` once, however it looks like `driver` is being captured in `checkFinalConditions` and as far as I can tell `currentSession` is a getter over `_currentSession` which is `final`, so it seems like that wouldn't actually change anything.
It also sounds like we shouldn't get getting resolved units through a session from the driver like this(?) and it doesn't seem like that would resolve this.
Should the session be coming from somewhere else?
Patch Set #11, Line 78:
driver.resourceProvider
It isn't clear to me why we're accessing the resource provider from the driver when we have it store […]
Makes sense - I've removed the `driver.` prefix from both locations doing this in this file.
Patch Set #11, Line 97:
driver.getResult
By bypassing the session and going directly to the driver the code is avoiding the consistency check […]
I've changed this to `driver.currentSession.getResolvedUnit` for now (as I presume that would run the consistency check you mention), although I'm not sure that's exactly what's required (see comment above).
File pkg/analysis_server/lib/src/services/refactoring/refactoring.dart:
Patch Set #11, Line 260:
cancellationTokenSimilarly, consider moving this to `Refactoring`.
Done!
File pkg/analysis_server/test/services/refactoring/move_file_test.dart:
Patch Set #11, Line 344:
convertPath
Not required, but consider using `newFile`. […]
Done!
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Dec 14, 2021, 1:12:48 PM12/14/21
to Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
2 comments:
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart:
Patch Set #11, Line 76:
driver.currentSessionI'm not sure I fully understand the lifecycles of the driver/session here to know how best to change […]
It turns out that the mechanism is currently broken, but, the way Konstantin and I designed it, the AnalysisDriver is intended to be long lived, handling changes to the content of the underlying files and re-analyzing the code as needed. The analysis session is similar to a database session in that it ensures that all of the results returned by it are consistent with each other. It has a limited lifetime: from the time you ask for it until something happens that causes new results to potentially be inconsistent with older results. When it throws an exception, the code that's accessing results through it should discard all previous work, ask the driver to the new current session, and start over from scratch.
Patch Set #11, Line 97:
driver.getResult
I've changed this to `driver.currentSession. […]
That won't be sufficient when the analysis session consistency checks are fixed, because we could still be using results from two different sessions, which means results that might be inconsistent. We really ought to be using a single session for everything and aborting the refactoring if the session throws an exception.
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Dec 14, 2021, 1:21:50 PM12/14/21
to rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart:
Patch Set #11, Line 76:
driver.currentSession
It turns out that the mechanism is currently broken, but, the way Konstantin and I designed it, the […]
Ah, gotcha. So if I understand correctly, I should:
- capture the session from the driver in `checkFinalConditions` (where the driver is currently captured)
- change `driver.currentSession` in several places in here to just use that session
- handle `InconsistentAnalysisException` from calling session.getResolvedUnit` and abort the refactor (although this is the part that may not be working correctly right now, and `AnalysisSessionImpl._checkConsistency` might always be succeeding)
Thanks!
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Feb 22, 2022, 12:51:51 PM2/22/22
to rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Patchset:
I think all comments/issues here were addressed. I was holding off trying to land it because of the delay in the LSP release, however with some additional complications (sometimes failing to get `LineInfo`s during large renames) it might be better to land as-is (to avoid it rotting and so other editors can use it if they wish) and just enable for LSP once more of the `LineInfo` issues are resolved.
@Jaime if you're likely to be adding support to IntelliJ, I presume the API changes look ok to you (I'd recommend providing a way for the user to cancel in case they rename a huge folder and it's quite slow - via the new `server.cancelRequest` request added here).
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Feb 22, 2022, 1:56:51 PM2/22/22
to Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
Patch set 17:Code-Review +1Commit-Queue +1
2 comments:
Patchset:
This all lgtm, so I'll run it through the bots while we wait for Jaime's feedback.
File pkg/analysis_server/lib/src/services/refactoring/move_file.dart:
Patch Set #11, Line 97:
driver.getResultThat won't be sufficient when the analysis session consistency checks are fixed, because we could st […]
FWIW, I believe that the consistency checking has now been restored.
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Feb 22, 2022, 2:37:03 PM2/22/22
to Danny Tuppeny, rev...@dartlang.org, Commit Bot, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/bd720414f8ebbf663cbfd6f35956d8e567feb343
Brian Wilkerson (Gerrit)
Feb 22, 2022, 6:17:10 PM2/22/22
to Danny Tuppeny, rev...@dartlang.org, Dart CI, Commit Bot, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
Patch set 17:Commit-Queue +1
1 comment:
Patchset:
Jaime won't have a chance to look at this until early next week at the earliest. If you're comfortable doing so I'm happy to land this and we can iterate if he sees any problems.
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
NOTUSED (Gerrit)
Feb 23, 2022, 4:02:10 AM2/23/22
to Danny Tuppeny, rev...@dartlang.org, Dart CI, Commit Bot, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny, Brian Wilkerson.
1 comment:
Patchset:
Jaime won't have a chance to look at this until early next week at the earliest. […]
Yep, that's fine by me! Thanks!
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Feb 23, 2022, 4:08:15 AM2/23/22
to rev...@dartlang.org, Dart CI, Commit Bot, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Brian Wilkerson.
1 comment:
Patchset:
Yep, that's fine by me! Thanks!
Oops.. having multiple accounts is hard 🙃
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Danny Tuppeny (Gerrit)
Feb 23, 2022, 4:08:15 AM2/23/22
to NOTUSED, rev...@dartlang.org, Brian Wilkerson
Attention is currently required from: Brian Wilkerson.
Danny Tuppeny removed NOTUSED from this change.
Basic support for folders in MOVE_FILE refactor
Change-Id: If960592199526df3dcd7f35c8a2396060b27e46e
---
M pkg/analysis_server/doc/api.html
M pkg/analysis_server/lib/protocol/protocol_constants.dart
M pkg/analysis_server/lib/src/edit/edit_domain.dart
M pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
M pkg/analysis_server/lib/src/services/refactoring/move_file.dart
M pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
M pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart
M pkg/analysis_server/test/edit/refactoring_test.dart
M pkg/analysis_server/test/services/refactoring/move_file_test.dart
M pkg/analysis_server/tool/spec/spec_input.html
M pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
11 files changed, 250 insertions(+), 98 deletions(-)
diff --git a/pkg/analysis_server/doc/api.html b/pkg/analysis_server/doc/api.html
index 7b19889..5423376 100644
--- a/pkg/analysis_server/doc/api.html
+++ b/pkg/analysis_server/doc/api.html
@@ -109,7 +109,7 @@
<body>
<h1>Analysis Server API Specification</h1>
<h1 style="color:#999999">Version
- 1.32.9
+ 1.32.10
</h1>
<p>
This document contains a specification of the API provided by the
@@ -236,6 +236,10 @@
ignoring the item or treating it with some default/fallback handling.
</p>
<h3>Changelog</h3>
+<h4>1.32.10</h4>
+<ul>
+ <li>The <tt>MOVE_FILE</tt> refactor now supports moving/renaming folders.</li>
+</ul>
<h4>1.32.8</h4>
<ul>
<li>Added <tt>server.cancelRequest</tt> to allow clients to request cancellation
@@ -6067,10 +6071,15 @@
</p>
</dd></dl></dd><dt class="refactoring">MOVE_FILE</dt><dd>
<p>
- Move the given file and update all of the references to that file
- and from it. The move operation is supported in general case - for
- renaming a file in the same folder, moving it to a different folder
- or both.
+ Move the given file or folder and update all of the references to
+ and from it. The move operation is supported in general case -
+ for renaming an item in the same folder, moving it to a different
+ folder or both.
+
+ Moving or renaming large folders may take time and clients should
+ consider showing an indicator to the user with the ability to
+ cancel the request (which can be done using
+ <tt>server.cancelRequest</tt>).
</p>
<p>
The refactoring must be activated before an actual file moving
diff --git a/pkg/analysis_server/lib/protocol/protocol_constants.dart b/pkg/analysis_server/lib/protocol/protocol_constants.dart
index 6847dd0..e8a4e7e 100644
--- a/pkg/analysis_server/lib/protocol/protocol_constants.dart
+++ b/pkg/analysis_server/lib/protocol/protocol_constants.dart
@@ -6,7 +6,7 @@
// To regenerate the file, use the script
// "pkg/analysis_server/tool/spec/generate_files".
-const String PROTOCOL_VERSION = '1.32.9';
+const String PROTOCOL_VERSION = '1.32.10';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES = 'analysis.analyzedFiles';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES_DIRECTORIES = 'directories';
diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart
index 655d9d9..a05df43 100644
--- a/pkg/analysis_server/lib/src/edit/edit_domain.dart
+++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart
@@ -355,7 +355,7 @@
getFixes(request);
return Response.DELAYED_RESPONSE;
} else if (requestName == EDIT_REQUEST_GET_REFACTORING) {
- return _getRefactoring(request);
+ return _getRefactoring(request, cancellationToken);
} else if (requestName == EDIT_REQUEST_IMPORT_ELEMENTS) {
importElements(request);
return Response.DELAYED_RESPONSE;
@@ -865,7 +865,8 @@
}
}
- Response _getRefactoring(Request request) {
+ Response _getRefactoring(
+ Request request, CancellationToken cancellationToken) {
final refactoringManager = this.refactoringManager;
if (refactoringManager == null) {
return Response.unsupportedFeature(request.id, 'Search is not enabled.');
@@ -874,7 +875,7 @@
refactoringManager.cancel();
_newRefactoringManager();
}
- refactoringManager.getRefactoring(request);
+ refactoringManager.getRefactoring(request, cancellationToken);
return Response.DELAYED_RESPONSE;
}
@@ -973,7 +974,7 @@
_reset();
}
- void getRefactoring(Request _request) {
+ void getRefactoring(Request _request, CancellationToken cancellationToken) {
// prepare for processing the request
request = _request;
final result = this.result = EditGetRefactoringResult(
@@ -990,7 +991,8 @@
?.sendEvent('refactor', params.kind.name.toLowerCase());
runZonedGuarded(() async {
- await _init(params.kind, file, params.offset, params.length);
+ await _init(
+ params.kind, file, params.offset, params.length, cancellationToken);
if (initStatus.hasFatalError) {
feedback = null;
_sendResultResponse();
@@ -1076,8 +1078,8 @@
}
}
- Future<void> _createRefactoringFromKind(
- String file, int offset, int length) async {
+ Future<void> _createRefactoringFromKind(String file, int offset, int length,
+ CancellationToken cancellationToken) async {
if (kind == RefactoringKind.CONVERT_GETTER_TO_METHOD) {
var resolvedUnit = await server.getResolvedUnit(file);
if (resolvedUnit != null) {
@@ -1144,11 +1146,9 @@
);
}
} else if (kind == RefactoringKind.MOVE_FILE) {
- var resolvedUnit = await server.getResolvedUnit(file);
- if (resolvedUnit != null) {
- refactoring = MoveFileRefactoring(
- server.resourceProvider, refactoringWorkspace, resolvedUnit, file);
- }
+ refactoring = MoveFileRefactoring(
+ server.resourceProvider, refactoringWorkspace, file)
+ ..cancellationToken = cancellationToken;
} else if (kind == RefactoringKind.RENAME) {
var resolvedUnit = await server.getResolvedUnit(file);
if (resolvedUnit != null) {
@@ -1171,8 +1171,8 @@
/// Initializes this context to perform a refactoring with the specified
/// parameters. The existing [Refactoring] is reused or created as needed.
- Future _init(
- RefactoringKind kind, String file, int offset, int length) async {
+ Future _init(RefactoringKind kind, String file, int offset, int length,
+ CancellationToken cancellationToken) async {
// check if we can continue with the existing Refactoring instance
if (this.kind == kind &&
this.file == file &&
@@ -1191,7 +1191,7 @@index f3cae42..caa3706 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/move_file.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/move_file.dart
@@ -7,6 +7,7 @@
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/refactoring/refactoring_internal.dart';
import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/file_system/file_system.dart';
import 'package:analyzer/src/dart/analysis/driver.dart';
@@ -22,16 +23,16 @@
final ResourceProvider resourceProvider;
final pathos.Context pathContext;
final RefactoringWorkspace refactoringWorkspace;
- final ResolvedUnitResult resolvedUnit;
late AnalysisDriver driver;
+ late AnalysisSession _session;
late String oldFile;
late String newFile;
final packagePrefixedStringPattern = RegExp(r'''^r?['"]+package:''');
- MoveFileRefactoringImpl(this.resourceProvider, this.refactoringWorkspace,
- this.resolvedUnit, this.oldFile)
+ MoveFileRefactoringImpl(
+ this.resourceProvider, this.refactoringWorkspace, this.oldFile)
: pathContext = resourceProvider.pathContext;
@override
@@ -54,7 +55,8 @@
}
driver = drivers.first;
- if (!driver.resourceProvider.getFile(oldFile).exists) {
+ _session = driver.currentSession;
+ if (!resourceProvider.getResource(oldFile).exists) {
return RefactoringStatus.fatal('$oldFile does not exist.');
}
@@ -68,24 +70,53 @@
@override
Future<SourceChange> createChange() async {
- var changeBuilder = ChangeBuilder(session: resolvedUnit.session);
+ var changeBuilder = ChangeBuilder(session: _session);
+
+ final resource = resourceProvider.getResource(oldFile);
+
+ try {
+ await _appendChangesForResource(changeBuilder, resource, newFile);
+ } on InconsistentAnalysisException {
+ // If an InconsistentAnalysisException occurs, it's likely the user
+ // modified the source and is no longer interested in the results.
+ return SourceChange('Refactor cancelled by file modifications');
+ }
+
+ // If cancellation was requested the results may be incomplete so return
+ // a new empty change instead of a partial one with a descriptive name
+ // so it's clear from any logs that cancellation was processed.
+ if (isCancellationRequested) {
+ return SourceChange('Refactor cancelled');
+ }
+
+ return changeBuilder.sourceChange;
+ }
+
+ Future<void> _appendChangeForFile(
+ ChangeBuilder changeBuilder, File file, String newPath) async {
+ var oldPath = file.path;
+ var oldDir = pathContext.dirname(oldPath);
+ var newDir = pathContext.dirname(newPath);
+
+ final resolvedUnit = await _session.getResolvedUnit(file.path);
+ if (resolvedUnit is! ResolvedUnitResult) {
+ return;
+ }
+
var element = resolvedUnit.unit.declaredElement;
if (element == null) {
- return changeBuilder.sourceChange;
+ return;
}
var libraryElement = element.library;
- final oldDir = pathContext.dirname(oldFile);
- final newDir = pathContext.dirname(newFile);
-
// If this element is a library, update outgoing references inside the file.
if (element == libraryElement.definingCompilationUnit) {
// Handle part-of directives in this library
var libraryResult = await driver.currentSession
.getResolvedLibraryByElement(libraryElement);
if (libraryResult is! ResolvedLibraryResult) {
- return changeBuilder.sourceChange;
+ return;
}
var definingUnitResult = libraryResult.units.first;
for (var result in libraryResult.units) {
@@ -99,9 +130,7 @@
await changeBuilder.addDartFileEdit(
result.unit.declaredElement!.source.fullName, (builder) {
partOfs.forEach((uri) {
- var newLocation =
- pathContext.join(newDir, pathos.basename(newFile));
- var newUri = _getRelativeUri(newLocation, oldDir);
+ var newUri = _getRelativeUri(newPath, oldDir);
builder.addSimpleReplacement(
SourceRange(uri.offset, uri.length), "'$newUri'");
});
@@ -145,27 +174,41 @@index cdb3485..efe3880 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
@@ -20,6 +20,7 @@
import 'package:analysis_server/src/services/refactoring/rename_local.dart';
import 'package:analysis_server/src/services/refactoring/rename_unit_member.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
+import 'package:analysis_server/src/utilities/progress.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/session.dart';
import 'package:analyzer/dart/ast/ast.dart';
@@ -248,16 +249,12 @@
bool isAvailable();
}
-/// [Refactoring] to move/rename a file.
+/// [Refactoring] to move/rename a file or folder.
abstract class MoveFileRefactoring implements Refactoring {
/// Returns a new [MoveFileRefactoring] instance.
- factory MoveFileRefactoring(
- ResourceProvider resourceProvider,
- RefactoringWorkspace workspace,
- ResolvedUnitResult resolveResult,
- String oldFilePath) {
- return MoveFileRefactoringImpl(
- resourceProvider, workspace, resolveResult, oldFilePath);
+ factory MoveFileRefactoring(ResourceProvider resourceProvider,
+ RefactoringWorkspace workspace, String oldFilePath) {
+ return MoveFileRefactoringImpl(resourceProvider, workspace, oldFilePath);
}
/// The new file path to which the given file is being moved.
@@ -266,6 +263,8 @@
/// Abstract interface for all refactorings.
abstract class Refactoring {
+ set cancellationToken(CancellationToken token);
+
/// The ids of source edits that are not known to be valid.
///
/// An edit is not known to be valid if there was insufficient type
diff --git a/pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart b/pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart
index 7eef982..d969475 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart
@@ -8,6 +8,7 @@
import 'package:analysis_server/src/services/correction/status.dart';
import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
+import 'package:analysis_server/src/utilities/progress.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/source.dart';
@@ -37,6 +38,11 @@
@override
final List<String> potentialEditIds = <String>[];
+ CancellationToken? cancellationToken;
+
+ bool get isCancellationRequested =>
+ cancellationToken?.isCancellationRequested ?? false;
+
@override
Future<RefactoringStatus> checkAllConditions() async {
var result = RefactoringStatus();
diff --git a/pkg/analysis_server/test/edit/refactoring_test.dart b/pkg/analysis_server/test/edit/refactoring_test.dart
index 8d4e69a..7f1a3be 100644
@@ -2253,6 +2285,26 @@@@ -2344,7 +2396,9 @@
Future<void> setUp() async {
super.setUp();
await createProject();
- handler = EditDomainHandler(server);
- server.handlers = [handler];
+ server.handlers = [
+ EditDomainHandler(server),
+ ServerDomainHandler(server),
+ ];
}
}
diff --git a/pkg/analysis_server/test/services/refactoring/move_file_test.dart b/pkg/analysis_server/test/services/refactoring/move_file_test.dart
index 52fe0df..983df07 100644
--- a/pkg/analysis_server/test/services/refactoring/move_file_test.dart
+++ b/pkg/analysis_server/test/services/refactoring/move_file_test.dart
@@ -22,14 +22,12 @@
late MoveFileRefactoring refactoring;
Future<void> test_file_containing_imports_exports_parts() async {
- var pathA = convertPath('/home/test/000/1111/a.dart');
- var pathB = convertPath('/home/test/000/1111/b.dart');
- var pathC = convertPath('/home/test/000/1111/22/c.dart');
- testFile = convertPath('/home/test/000/1111/test.dart');
- addSource('/absolute/uri.dart', '');
- addSource(pathA, 'part of lib;');
- addSource(pathB, "import 'test.dart';");
- addSource(pathC, '');
+ final root = '/home/test/000/1111';
+ testFile = convertPath('$root/test.dart');
+ newFile('/absolute/uri.dart', content: '');
+ final fileA = newFile('$root/a.dart', content: 'part of lib;');
+ final fileB = newFile('$root/b.dart', content: "import 'test.dart';");
+ final fileC = newFile('$root/22/c.dart', content: '');
verifyNoTestUnitErrors = false;
await resolveTestCode('''
library lib;
@@ -43,9 +41,9 @@
// perform refactoring
_createRefactoring('/home/test/000/1111/22/new_name.dart');
await _assertSuccessfulRefactoring();
- assertNoFileChange(pathA);
- assertFileChangeResult(pathB, "import '22/new_name.dart';");
- assertNoFileChange(pathC);
+ assertNoFileChange(fileA.path);
+ assertFileChangeResult(fileB.path, "import '22/new_name.dart';");
+ assertNoFileChange(fileC.path);
assertFileChangeResult(testFile, '''
library lib;
import 'dart:math';
@@ -340,9 +338,34 @@
@@ -517,8 +540,8 @@
// Allow passing an oldName for when we don't want to rename testSource,
// but otherwise fall back to testSource.fullname
oldFile = convertPath(oldFile ?? testFile);
- refactoring = MoveFileRefactoring(
- resourceProvider, refactoringWorkspace, testAnalysisResult, oldFile);
+ refactoring =
+ MoveFileRefactoring(resourceProvider, refactoringWorkspace, oldFile);
refactoring.newFile = convertPath(newFile);
}
}
diff --git a/pkg/analysis_server/tool/spec/spec_input.html b/pkg/analysis_server/tool/spec/spec_input.html
index 7210871..1d7b96a 100644
--- a/pkg/analysis_server/tool/spec/spec_input.html
+++ b/pkg/analysis_server/tool/spec/spec_input.html
@@ -7,7 +7,7 @@
<body>
<h1>Analysis Server API Specification</h1>
<h1 style="color:#999999">Version
- <version>1.32.9</version>
+ <version>1.32.10</version>
</h1>
<p>
This document contains a specification of the API provided by the
@@ -134,6 +134,10 @@
ignoring the item or treating it with some default/fallback handling.
</p>
<h3>Changelog</h3>
+<h4>1.32.10</h4>
+<ul>
+ <li>The <tt>MOVE_FILE</tt> refactor now supports moving/renaming folders.</li>
+</ul>
<h4>1.32.8</h4>
<ul>
<li>Added <tt>server.cancelRequest</tt> to allow clients to request cancellation
@@ -5907,10 +5911,15 @@
</refactoring>
<refactoring kind="MOVE_FILE">
<p>
- Move the given file and update all of the references to that file
- and from it. The move operation is supported in general case - for
- renaming a file in the same folder, moving it to a different folder
- or both.
+ Move the given file or folder and update all of the references to
+ and from it. The move operation is supported in general case -
+ for renaming an item in the same folder, moving it to a different
+ folder or both.
+
+ Moving or renaming large folders may take time and clients should
+ consider showing an indicator to the user with the ability to
+ cancel the request (which can be done using
+ <tt>server.cancelRequest</tt>).
</p>
<p>
The refactoring must be activated before an actual file moving
diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
index 6847dd0..e8a4e7e 100644
--- a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
+++ b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
@@ -6,7 +6,7 @@
// To regenerate the file, use the script
// "pkg/analysis_server/tool/spec/generate_files".
-const String PROTOCOL_VERSION = '1.32.9';
+const String PROTOCOL_VERSION = '1.32.10';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES = 'analysis.analyzedFiles';
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES_DIRECTORIES = 'directories';
To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Brian Wilkerson (Gerrit)
Feb 23, 2022, 11:37:03 AM2/23/22
to Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Dart CI, Commit Bot, Konstantin Shcheglov, Jaime Wren
Attention is currently required from: Danny Tuppeny.
Patch set 18:Code-Review +1Commit-Queue +2
Commit Bot (Gerrit)
Feb 23, 2022, 12:22:48 PM2/23/22
to Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Dart CI, Konstantin Shcheglov, Jaime Wren
Commit Bot submitted this change.
Approvals:
Brian Wilkerson: Looks good to me, approved; Commit
Basic support for folders in MOVE_FILE refactor
Change-Id: If960592199526df3dcd7f35c8a2396060b27e46e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158005
Reviewed-by: Brian Wilkerson <brianwi...@google.com>
Commit-Queue: Brian Wilkerson <brianwi...@google.com>
---
M pkg/analysis_server/doc/api.html
M pkg/analysis_server/lib/protocol/protocol_constants.dart
M pkg/analysis_server/lib/src/edit/edit_domain.dart
M pkg/analysis_server/lib/src/lsp/handlers/handler_will_rename_files.dart
M pkg/analysis_server/lib/src/services/refactoring/move_file.dart
M pkg/analysis_server/lib/src/services/refactoring/refactoring.dart
M pkg/analysis_server/lib/src/services/refactoring/refactoring_internal.dart
M pkg/analysis_server/test/edit/refactoring_test.dart
M pkg/analysis_server/test/services/refactoring/move_file_test.dart
M pkg/analysis_server/tool/spec/spec_input.html
M pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart
11 files changed, 254 insertions(+), 98 deletions(-)
index 8d4e69a..74507b0 100644@@ -1335,16 +1325,59 @@
''');
_setOptions('/project/test.dart');
return assertSuccessfulRefactoring(() {
- return _sendMoveRequest();
+ return _sendMoveRequest(testFile);
}, '''
import 'dart:math';
import 'bin/lib.dart';
''');
}
- Future<Response> _sendMoveRequest() {
+ Future<void> test_folder_cancel() {
+ newFile('/project/bin/original_folder/file.dart');
+ addTestFile('''
+import 'dart:math';
+import 'original_folder/file.dart';
+''');
+ _setOptions('/project/bin/new_folder');
+ return assertEmptySuccessfulRefactoring(() async {
+ return _sendAndCancelMoveRequest(
+ convertPath('/project/bin/original_folder'));
+ });
+ }
+
+ Future<void> test_folder_OK() {
+ newFile('/project/bin/original_folder/file.dart');
+ addTestFile('''
+import 'dart:math';
+import 'original_folder/file.dart';
+''');
+ _setOptions('/project/bin/new_folder');
+ return assertSuccessfulRefactoring(() async {
+ return _sendMoveRequest(convertPath('/project/bin/original_folder'));@@ -2253,6 +2286,26 @@@@ -2344,7 +2397,9 @@To view, visit change 158005. To unsubscribe, or for help writing mail filters, visit settings.
Dart CI (Gerrit)
Feb 23, 2022, 12:56:50 PM2/23/22
to Commit Bot, Danny Tuppeny, rev...@dartlang.org, Brian Wilkerson, Konstantin Shcheglov, Jaime Wren
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/729b2e5d34bbca2a899ba12c7738cfa0bc3fa3b6
Reply all
Reply to author
Forward
0 new messages