Implement some more LSP rename tests
Change-Id: Idd0f3734b20f716d5e92f13c3a85f716c226fc5b
Typo
Change-Id: Ica50195f94f8c57be0e16edde75e6e287b45c06b
Reviewed-on: https://dart-review.googlesource.com/c/89365
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/constants.dart b/pkg/analysis_server/lib/src/lsp/constants.dart
index 2110d4d..464ab95 100644
--- a/pkg/analysis_server/lib/src/lsp/constants.dart
+++ b/pkg/analysis_server/lib/src/lsp/constants.dart
@@ -40,7 +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);
+ static const RenameNotValid = 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_rename.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
index 090d560..3a9075e 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
@@ -42,8 +42,8 @@
// Check the rename is valid here.
final initStatus = await refactoring.checkInitialConditions();
if (initStatus.hasFatalError) {
- return error(ServerErrorCodes.FatalRefactoringProblem,
- initStatus.problem.message, null);
+ return error(
+ ServerErrorCodes.RenameNotValid, initStatus.problem.message, null);
}
return success(new RangeAndPlaceholder(
@@ -99,26 +99,32 @@
final refactoring = new RenameRefactoring(server.refactoringWorkspace,
unit.result.session, refactorDetails.element);
+ // TODO(dantup): Consider using window/showMessageRequest to prompt
+ // the user to see if they'd like to proceed with a rename if there
+ // are non-fatal errors or warnings. For now we will reject all errors
+ // (fatal and not) as this seems like the most logical behaviour when
+ // without a prompt.
+
// Check the rename is valid here.
final initStatus = await refactoring.checkInitialConditions();
- if (initStatus.hasFatalError) {
- return error(ServerErrorCodes.FatalRefactoringProblem,
- initStatus.problem.message, null);
+ if (initStatus.hasError) {
+ return error(
+ ServerErrorCodes.RenameNotValid, initStatus.problem.message, null);
}
// Check the name is valid.
refactoring.newName = params.newName;
final optionsStatus = refactoring.checkNewName();
- if (optionsStatus.hasFatalError) {
- return error(ServerErrorCodes.FatalRefactoringProblem,
+ if (optionsStatus.hasError) {
+ return error(ServerErrorCodes.RenameNotValid,
optionsStatus.problem.message, null);
}
// Final validation.
final finalStatus = await refactoring.checkFinalConditions();
- if (finalStatus.hasFatalError) {
- return error(ServerErrorCodes.FatalRefactoringProblem,
- finalStatus.problem.message, null);
+ if (finalStatus.hasError) {
+ return error(
+ ServerErrorCodes.RenameNotValid, finalStatus.problem.message, null);
}
// Compute the actual change.
diff --git a/pkg/analysis_server/test/lsp/rename_test.dart b/pkg/analysis_server/test/lsp/rename_test.dart
index 8e984f2..6226b60 100644
--- a/pkg/analysis_server/test/lsp/rename_test.dart
+++ b/pkg/analysis_server/test/lsp/rename_test.dart
@@ -18,11 +18,8 @@
@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): check the version returned matches?
// TODO(dantup): renames across multiple files
test_prepare_class() {
@@ -97,7 +94,7 @@
expect(response.id, equals(request.id));
expect(response.result, isNull);
expect(response.error, isNotNull);
- expect(response.error.code, ServerErrorCodes.FatalRefactoringProblem);
+ expect(response.error.code, ServerErrorCodes.RenameNotValid);
expect(response.error.message, contains('is defined in the SDK'));
}
@@ -180,6 +177,38 @@
return _test_rename_withDocumentChanges(content, 'MyNewClass', null);
}
+ test_rename_rejectedForBadName() async {
+ const content = '''
+ class MyClass {}
+ final a = n^ew MyClass();
+ ''';
+ final error = await _test_rename_failure(content, 'not a valid class name');
+ expect(error.code, equals(ServerErrorCodes.RenameNotValid));
+ expect(error.message, contains('name must not contain'));
+ }
+
+ test_rename_rejectedForDuplicateName() async {
+ const content = '''
+ class MyOtherClass {}
+ class MyClass {}
+ final a = n^ew MyClass();
+ ''';
+ final error = await _test_rename_failure(content, 'MyOtherClass');
+ expect(error.code, equals(ServerErrorCodes.RenameNotValid));
+ expect(error.message, contains('already declares class with name'));
+ }
+
+ test_rename_rejectedForStaleDocument() async {
+ const content = '''
+ class MyClass {}
+ final a = n^ew MyClass();
+ ''';
+ final error =
+ await _test_rename_failure(content, 'MyNewClass', openFileVersion: 111);
+ expect(error.code, equals(ErrorCodes.ContentModified));
+ expect(error.message, contains('Document was modified'));
+ }
+
test_rename_sdkClass() async {
const content = '''
final a = new [[Ob^ject]]();
@@ -201,7 +230,7 @@
expect(response.id, equals(request.id));
expect(response.result, isNull);
expect(response.error, isNotNull);
- expect(response.error.code, ServerErrorCodes.FatalRefactoringProblem);
+ expect(response.error.code, ServerErrorCodes.RenameNotValid);
expect(response.error.message, contains('is defined in the SDK'));
}
@@ -266,6 +295,31 @@
}
}
+ Future<ResponseError> _test_rename_failure(
+ String content,
+ String newName, {
+ int openFileVersion = 222,
+ int renameRequestFileVersion = 222,
+ }) async {
+ await initialize(
+ workspaceCapabilities:
+ withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+ );
+ await openFile(mainFileUri, withoutMarkers(content),
+ version: openFileVersion);
+
+ final result = await renameRaw(
+ mainFileUri,
+ renameRequestFileVersion,
+ positionFromMarker(content),
+ newName,
+ );
+
+ expect(result.result, isNull);
+ expect(result.error, isNotNull);
+ return result.error;
+ }
+
_test_rename_withDocumentChanges(
String content, String newName, String expectedContent) async {
await initialize(
@@ -280,7 +334,6 @@
positionFromMarker(content),
newName,
);
-
if (expectedContent == null) {
expect(result, isNull);
} else {
diff --git a/pkg/analysis_server/test/lsp/server_abstract.dart b/pkg/analysis_server/test/lsp/server_abstract.dart
index 1ccbbb1..fad7baf 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -446,6 +446,18 @@
return new NotificationMessage(method, params, jsonRpcVersion);
}
+ RequestMessage makeRenameRequest(
+ int version, Uri uri, 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 request;
+ }
+
RequestMessage makeRequest(Method method, ToJsonable params) {
final id = Either2<num, String>.t1(_id++);
return new RequestMessage(id, method, params, jsonRpcVersion);
@@ -537,16 +549,20 @@
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),
- );
+ final request = makeRenameRequest(version, uri, pos, newName);
return expectSuccessfulResponseTo<WorkspaceEdit>(request);
}
+ Future<ResponseMessage> renameRaw(
+ Uri uri,
+ int version,
+ Position pos,
+ String newName,
+ ) {
+ final request = makeRenameRequest(version, uri, pos, newName);
+ return channel.sendRequestToServer(request);
+ }
+
Future replaceFile(int newVersion, Uri uri, String content) {
return changeFile(
newVersion,
