Add implementation/tests for LSP rename
Change-Id: I28e45cb7c979edb3d64b17e86ad0e03b7cd746cd
Reviewed-on: https://dart-review.googlesource.com/c/89361
Commit-Queue: Danny Tuppeny <dantup@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/constants.dart b/pkg/analysis_server/lib/src/lsp/constants.dart
index 7674d6f..2110d4d 100644
--- a/pkg/analysis_server/lib/src/lsp/constants.dart
+++ b/pkg/analysis_server/lib/src/lsp/constants.dart
@@ -40,6 +40,7 @@
static const FileNotAnalyzed = const ErrorCodes(-32007);
static const FileHasErrors = const ErrorCodes(-32008);
static const ClientFailedToApplyEdit = const ErrorCodes(-32009);
+ static const FatalRefactoringProblem = const ErrorCodes(-32010);
/// An error raised when the server detects that the server and client are out
/// of sync and cannot recover. For example if a textDocument/didChange notification
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart
index 858e383..e1a72f6 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_initialize.dart
@@ -44,6 +44,9 @@
final codeActionLiteralSupport =
params.capabilities.textDocument?.codeAction?.codeActionLiteralSupport;
+ final renameOptionsSupport =
+ params.capabilities.textDocument?.rename?.prepareSupport ?? false;
+
server.capabilities = new ServerCapabilities(
Either2<TextDocumentSyncOptions, num>.t1(new TextDocumentSyncOptions(
true,
@@ -86,7 +89,9 @@
true, // documentFormattingProvider
false, // documentRangeFormattingProvider
new DocumentOnTypeFormattingOptions('}', [';']),
- null,
+ renameOptionsSupport
+ ? Either2<bool, RenameOptions>.t2(new RenameOptions(true))
+ : Either2<bool, RenameOptions>.t1(true),
null,
null,
null,
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
new file mode 100644
index 0000000..090d560
--- /dev/null
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
@@ -0,0 +1,139 @@
+// Copyright (c) 2019, 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/lsp_protocol/protocol_generated.dart';
+import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:analysis_server/src/lsp/handlers/handlers.dart';
+import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
+import 'package:analysis_server/src/lsp/mapping.dart';
+import 'package:analysis_server/src/services/refactoring/refactoring.dart';
+
+class PrepareRenameHandler
+ extends MessageHandler<TextDocumentPositionParams, RangeAndPlaceholder> {
+ PrepareRenameHandler(LspAnalysisServer server) : super(server);
+ Method get handlesMessage => Method.textDocument_prepareRename;
+
+ @override
+ TextDocumentPositionParams convertParams(Map<String, dynamic> json) =>
+ TextDocumentPositionParams.fromJson(json);
+
+ @override
+ Future<ErrorOr<RangeAndPlaceholder>> handle(
+ TextDocumentPositionParams params) async {
+ final pos = params.position;
+ final path = pathOfDoc(params.textDocument);
+ final unit = await path.mapResult(requireUnit);
+ final offset = await unit.mapResult((unit) => toOffset(unit.lineInfo, pos));
+
+ return offset.mapResult((offset) async {
+ final node = await server.getNodeAtOffset(path.result, offset);
+ final element = server.getElementOfNode(node);
+ if (node == null || element == null) {
+ return success(null);
+ }
+
+ final refactorDetails =
+ RenameRefactoring.getElementToRename(node, element);
+ final refactoring = new RenameRefactoring(server.refactoringWorkspace,
+ unit.result.session, refactorDetails.element);
+
+ // Check the rename is valid here.
+ final initStatus = await refactoring.checkInitialConditions();
+ if (initStatus.hasFatalError) {
+ return error(ServerErrorCodes.FatalRefactoringProblem,
+ initStatus.problem.message, null);
+ }
+
+ return success(new RangeAndPlaceholder(
+ toRange(
+ unit.result.lineInfo,
+ // If the offset is set to -1 it means there is no location for the
+ // old name. However since we must provide a range for LSP, we'll use
+ // a 0-character span at the originally requested location to ensure
+ // it's valid.
+ refactorDetails.offset == -1 ? offset : refactorDetails.offset,
+ refactorDetails.length,
+ ),
+ refactoring.oldName,
+ ));
+ });
+ }
+}
+
+class RenameHandler extends MessageHandler<RenameParams, WorkspaceEdit> {
+ RenameHandler(LspAnalysisServer server) : super(server);
+
+ Method get handlesMessage => Method.textDocument_rename;
+
+ @override
+ RenameParams convertParams(Map<String, dynamic> json) =>
+ RenameParams.fromJson(json);
+
+ @override
+ Future<ErrorOr<WorkspaceEdit>> handle(RenameParams params) async {
+ final pos = params.position;
+ final path = pathOfDoc(params.textDocument);
+ // If the client provided us a version doc identifier, we'll use it to ensure
+ // we're not computing a rename for an old document. If not, we'll just assume
+ // the version the server had at the time of recieving the request is valid
+ // and then use it to verify the document hadn't changed again before we
+ // send the edits.
+ final docIdentifier = await path.mapResult((path) => success(
+ params.textDocument is VersionedTextDocumentIdentifier
+ ? params.textDocument
+ : server.getVersionedDocumentIdentifier(path)));
+ final unit = await path.mapResult(requireUnit);
+ final offset = await unit.mapResult((unit) => toOffset(unit.lineInfo, pos));
+
+ return offset.mapResult((offset) async {
+ final node = await server.getNodeAtOffset(path.result, offset);
+ final element = server.getElementOfNode(node);
+ if (node == null || element == null) {
+ return success(null);
+ }
+
+ final refactorDetails =
+ RenameRefactoring.getElementToRename(node, element);
+ final refactoring = new RenameRefactoring(server.refactoringWorkspace,
+ unit.result.session, refactorDetails.element);
+
+ // Check the rename is valid here.
+ final initStatus = await refactoring.checkInitialConditions();
+ if (initStatus.hasFatalError) {
+ return error(ServerErrorCodes.FatalRefactoringProblem,
+ initStatus.problem.message, null);
+ }
+
+ // Check the name is valid.
+ refactoring.newName = params.newName;
+ final optionsStatus = refactoring.checkNewName();
+ if (optionsStatus.hasFatalError) {
+ return error(ServerErrorCodes.FatalRefactoringProblem,
+ optionsStatus.problem.message, null);
+ }
+
+ // Final validation.
+ final finalStatus = await refactoring.checkFinalConditions();
+ if (finalStatus.hasFatalError) {
+ return error(ServerErrorCodes.FatalRefactoringProblem,
+ finalStatus.problem.message, null);
+ }
+
+ // Compute the actual change.
+ final change = await refactoring.createChange();
+
+ // Before we send anything back, ensure the original file didn't change
+ // while we were computing changes.
+ if (server.getVersionedDocumentIdentifier(path.result) !=
+ docIdentifier.result) {
+ return error(ErrorCodes.ContentModified,
+ 'Document was modified while rename was being computed', null);
+ }
+
+ final workspaceEdit = createWorkspaceEdit(server, change);
+ return success(workspaceEdit);
+ });
+ }
+}
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart
index 7e86071..9375b8b 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart
@@ -19,6 +19,7 @@
import 'package:analysis_server/src/lsp/handlers/handler_initialize.dart';
import 'package:analysis_server/src/lsp/handlers/handler_initialized.dart';
import 'package:analysis_server/src/lsp/handlers/handler_references.dart';
+import 'package:analysis_server/src/lsp/handlers/handler_rename.dart';
import 'package:analysis_server/src/lsp/handlers/handler_signature_help.dart';
import 'package:analysis_server/src/lsp/handlers/handler_text_document_changes.dart';
import 'package:analysis_server/src/lsp/handlers/handler_change_workspace_folders.dart';
@@ -61,6 +62,8 @@
registerHandler(new CodeActionHandler(server));
registerHandler(new ExecuteCommandHandler(server));
registerHandler(new WorkspaceFoldersHandler(server));
+ registerHandler(new PrepareRenameHandler(server));
+ registerHandler(new RenameHandler(server));
}
}
diff --git a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
index 20950cc..925e5a7 100644
--- a/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
+++ b/pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
@@ -24,6 +24,7 @@
import 'package:analysis_server/src/protocol_server.dart' as protocol;
import 'package:analysis_server/src/services/completion/completion_performance.dart'
show CompletionPerformance;
+import 'package:analysis_server/src/services/refactoring/refactoring.dart';
import 'package:analysis_server/src/services/search/search_engine.dart';
import 'package:analysis_server/src/services/search/search_engine_internal.dart';
import 'package:analysis_server/src/utilities/null_string_sink.dart';
@@ -79,6 +80,15 @@
final AnalysisOptionsImpl defaultContextOptions = new AnalysisOptionsImpl();
/**
+ * The workspace for rename refactorings. Should be accessed through the
+ * refactoringWorkspace getter to be automatically created (lazily).
+ */
+ RefactoringWorkspace _refactoringWorkspace;
+
+ RefactoringWorkspace get refactoringWorkspace => _refactoringWorkspace ??=
+ new RefactoringWorkspace(driverMap.values, searchEngine);
+
+ /**
* The versions of each document known to the server (keyed by path), used to
* send back to the client for server-initiated edits so that the client can
* ensure they have a matching version of the document before applying them.
diff --git a/pkg/analysis_server/test/lsp/rename_test.dart b/pkg/analysis_server/test/lsp/rename_test.dart
new file mode 100644
index 0000000..8e984f2
--- /dev/null
+++ b/pkg/analysis_server/test/lsp/rename_test.dart
@@ -0,0 +1,295 @@
+// Copyright (c) 2019, 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/lsp_protocol/protocol_generated.dart';
+import 'package:analysis_server/src/lsp/constants.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'server_abstract.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(RenameTest);
+ });
+}
+
+@reflectiveTest
+class RenameTest extends AbstractLspAnalysisServerTest {
+ // TODO(dantup): send a rename without a version
+ // TODO(dantup): file changes during computation?
+ // TODO(dantup): send an old version of the doc?
+ // TODO(dantup): check the version return matches?
+ // TODO(dantup): a rename that fails on options step
+ // TODO(dantup): a rename that fails on final step
+ // TODO(dantup): renames across multiple files
+
+ test_prepare_class() {
+ const content = '''
+ class MyClass {}
+ final a = new [[My^Class]]();
+ ''';
+
+ return _test_prepare(content, 'MyClass');
+ }
+
+ test_prepare_classNewKeyword() async {
+ const content = '''
+ class MyClass {}
+ final a = n^ew [[MyClass]]();
+ ''';
+
+ return _test_prepare(content, 'MyClass');
+ }
+
+ test_prepare_importPrefix() async {
+ const content = '''
+ import 'dart:async' as [[myPr^efix]];
+ ''';
+
+ return _test_prepare(content, 'myPrefix');
+ }
+
+ test_prepare_importWithoutPrefix() async {
+ const content = '''
+ imp[[^]]ort 'dart:async';
+ ''';
+
+ return _test_prepare(content, '');
+ }
+
+ test_prepare_importWithPrefix() async {
+ const content = '''
+ imp^ort 'dart:async' as [[myPrefix]];
+ ''';
+
+ return _test_prepare(content, 'myPrefix');
+ }
+
+ test_prepare_invalidRenameLocation() async {
+ const content = '''
+ main() {
+ // comm^ent
+ }
+ ''';
+
+ return _test_prepare(content, null);
+ }
+
+ test_prepare_sdkClass() async {
+ const content = '''
+ final a = new [[Ob^ject]]();
+ ''';
+
+ await initialize();
+ await openFile(mainFileUri, withoutMarkers(content));
+
+ final request = makeRequest(
+ Method.textDocument_prepareRename,
+ new TextDocumentPositionParams(
+ new TextDocumentIdentifier(mainFileUri.toString()),
+ positionFromMarker(content),
+ ),
+ );
+ final response = await channel.sendRequestToServer(request);
+
+ expect(response.id, equals(request.id));
+ expect(response.result, isNull);
+ expect(response.error, isNotNull);
+ expect(response.error.code, ServerErrorCodes.FatalRefactoringProblem);
+ expect(response.error.message, contains('is defined in the SDK'));
+ }
+
+ test_prepare_variable() async {
+ const content = '''
+ main() {
+ var variable = 0;
+ print([[vari^able]]);
+ }
+ ''';
+
+ return _test_prepare(content, 'variable');
+ }
+
+ test_rename_class() {
+ const content = '''
+ class MyClass {}
+ final a = new [[My^Class]]();
+ ''';
+ const expectedContent = '''
+ class MyNewClass {}
+ final a = new MyNewClass();
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'MyNewClass', expectedContent);
+ }
+
+ test_rename_classNewKeyword() async {
+ const content = '''
+ class MyClass {}
+ final a = n^ew MyClass();
+ ''';
+ const expectedContent = '''
+ class MyNewClass {}
+ final a = new MyNewClass();
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'MyNewClass', expectedContent);
+ }
+
+ test_rename_importPrefix() {
+ const content = '''
+ import 'dart:async' as myPr^efix;
+ ''';
+ const expectedContent = '''
+ import 'dart:async' as myNewPrefix;
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'myNewPrefix', expectedContent);
+ }
+
+ test_rename_importWithoutPrefix() {
+ const content = '''
+ imp^ort 'dart:async';
+ ''';
+ const expectedContent = '''
+ import 'dart:async' as myAddedPrefix;
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'myAddedPrefix', expectedContent);
+ }
+
+ test_rename_importWithPrefix() {
+ const content = '''
+ imp^ort 'dart:async' as myPrefix;
+ ''';
+ const expectedContent = '''
+ import 'dart:async' as myNewPrefix;
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'myNewPrefix', expectedContent);
+ }
+
+ test_rename_invalidRenameLocation() {
+ const content = '''
+ main() {
+ // comm^ent
+ }
+ ''';
+ return _test_rename_withDocumentChanges(content, 'MyNewClass', null);
+ }
+
+ test_rename_sdkClass() async {
+ const content = '''
+ final a = new [[Ob^ject]]();
+ ''';
+
+ await newFile(mainFilePath, content: withoutMarkers(content));
+ await initialize();
+
+ final request = makeRequest(
+ Method.textDocument_rename,
+ new RenameParams(
+ new TextDocumentIdentifier(mainFileUri.toString()),
+ positionFromMarker(content),
+ 'Object2',
+ ),
+ );
+ final response = await channel.sendRequestToServer(request);
+
+ expect(response.id, equals(request.id));
+ expect(response.result, isNull);
+ expect(response.error, isNotNull);
+ expect(response.error.code, ServerErrorCodes.FatalRefactoringProblem);
+ expect(response.error.message, contains('is defined in the SDK'));
+ }
+
+ test_rename_usingLegacyChangeInterface() async {
+ // This test initializes without support for DocumentChanges (versioning)
+ // whereas the other tests all use DocumentChanges support (preferred).
+ const content = '''
+ class MyClass {}
+ final a = new My^Class();
+ ''';
+ const expectedContent = '''
+ class MyNewClass {}
+ final a = new MyNewClass();
+ ''';
+
+ await initialize();
+ await openFile(mainFileUri, withoutMarkers(content), version: 222);
+
+ final result = await rename(
+ mainFileUri,
+ 222,
+ positionFromMarker(content),
+ 'MyNewClass',
+ );
+
+ // Ensure applying the changes will give us the expected content.
+ final contents = {
+ mainFilePath: withoutMarkers(content),
+ };
+ applyChanges(contents, result.changes);
+ expect(contents[mainFilePath], equals(expectedContent));
+ }
+
+ test_rename_variable() {
+ const content = '''
+ main() {
+ var variable = 0;
+ print([[vari^able]]);
+ }
+ ''';
+ const expectedContent = '''
+ main() {
+ var foo = 0;
+ print(foo);
+ }
+ ''';
+ return _test_rename_withDocumentChanges(content, 'foo', expectedContent);
+ }
+
+ _test_prepare(String content, String expectedPlaceholder) async {
+ await initialize();
+ await openFile(mainFileUri, withoutMarkers(content));
+
+ final result =
+ await prepareRename(mainFileUri, positionFromMarker(content));
+
+ if (expectedPlaceholder == null) {
+ expect(result, isNull);
+ } else {
+ expect(result.range, equals(rangeFromMarkers(content)));
+ expect(result.placeholder, equals(expectedPlaceholder));
+ }
+ }
+
+ _test_rename_withDocumentChanges(
+ String content, String newName, String expectedContent) async {
+ await initialize(
+ workspaceCapabilities:
+ withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+ );
+ await openFile(mainFileUri, withoutMarkers(content), version: 222);
+
+ final result = await rename(
+ mainFileUri,
+ 222,
+ positionFromMarker(content),
+ newName,
+ );
+
+ if (expectedContent == null) {
+ expect(result, isNull);
+ } else {
+ // Ensure applying the changes will give us the expected content.
+ final contents = {
+ mainFilePath: withoutMarkers(content),
+ };
+ applyDocumentChanges(contents, result.documentChanges);
+ expect(contents[mainFilePath], equals(expectedContent));
+ }
+ }
+}
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
index b8f21a0..1ccbbb1 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -469,6 +469,17 @@
return toPosition(lineInfo.getLocation(offset));
}
+ Future<RangeAndPlaceholder> prepareRename(Uri uri, Position pos) {
+ final request = makeRequest(
+ Method.textDocument_prepareRename,
+ new TextDocumentPositionParams(
+ new TextDocumentIdentifier(uri.toString()),
+ pos,
+ ),
+ );
+ return expectSuccessfulResponseTo<RangeAndPlaceholder>(request);
+ }
+
/// Returns the range surrounded by `[[markers]]` in the provided string,
/// excluding the markers themselves (as well as position markers `^` from
/// the offsets).
@@ -520,6 +531,22 @@
return rangesFromMarkersImpl(content).toList();
}
+ Future<WorkspaceEdit> rename(
+ Uri uri,
+ int version,
+ Position pos,
+ String newName,
+ ) {
+ final docIdentifier = version != null
+ ? new VersionedTextDocumentIdentifier(version, uri.toString())
+ : new TextDocumentIdentifier(uri.toString());
+ final request = makeRequest(
+ Method.textDocument_rename,
+ new RenameParams(docIdentifier, pos, newName),
+ );
+ return expectSuccessfulResponseTo<WorkspaceEdit>(request);
+ }
+
Future replaceFile(int newVersion, Uri uri, String content) {
return changeFile(
newVersion,
diff --git a/pkg/analysis_server/test/lsp/test_all.dart b/pkg/analysis_server/test/lsp/test_all.dart
index 003a401..d1e29ad 100644
--- a/pkg/analysis_server/test/lsp/test_all.dart
+++ b/pkg/analysis_server/test/lsp/test_all.dart
@@ -12,14 +12,15 @@
import 'completion_test.dart' as completion_test;
import 'definition_test.dart' as definition_test;
import 'diagnostic_test.dart' as diagnostic_test;
-import 'document_symbols_test.dart' as document_symbols_test;
import 'document_highlights_test.dart' as document_highlights_test;
+import 'document_symbols_test.dart' as document_symbols_test;
import 'file_modification_test.dart' as file_modification_test;
import 'format_test.dart' as format_test;
import 'hover_test.dart' as hover_test;
import 'initialization_test.dart' as initialization_test;
import 'priority_files_test.dart' as priority_files_test;
import 'references_test.dart' as references_test;
+import 'rename_test.dart' as rename_test;
import 'server_test.dart' as server_test;
import 'signature_help_test.dart' as signature_help_test;
@@ -42,5 +43,6 @@
fixes_code_action_tests.main();
assists_code_action_tests.main();
packet_transformer_tests.main();
+ rename_test.main();
}, name: 'lsp');
}
diff --git a/pkg/analysis_server/tool/lsp_spec/README.md b/pkg/analysis_server/tool/lsp_spec/README.md
index 5258007..fe5bf80 100644
--- a/pkg/analysis_server/tool/lsp_spec/README.md
+++ b/pkg/analysis_server/tool/lsp_spec/README.md
@@ -70,7 +70,7 @@
| textDocument/formatting | ✅ | ✅ | ✅ | ✅ |
| textDocument/rangeFormatting | | | | | requires support from dart_style?
| textDocument/onTypeFormatting | ✅ | ✅ | ✅ | ✅ |
-| textDocument/rename | | | | |
+| textDocument/rename | ✅ | ✅ | Incomplete! | |
| textDocument/prepareRename | | | | |
| textDocument/foldingRange | | | | |
