Improve testing of document versions in LSP renames
Change-Id: Ibbf562d6a99d73c00fc17521eb8576da4c708c06
Reviewed-on: https://dart-review.googlesource.com/c/89522
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/test/lsp/rename_test.dart b/pkg/analysis_server/test/lsp/rename_test.dart
index 6226b60..df2fc18 100644
--- a/pkg/analysis_server/test/lsp/rename_test.dart
+++ b/pkg/analysis_server/test/lsp/rename_test.dart
@@ -17,9 +17,6 @@
@reflectiveTest
class RenameTest extends AbstractLspAnalysisServerTest {
- // TODO(dantup): send a rename without a version
- // TODO(dantup): send an old version of the doc?
- // TODO(dantup): check the version returned matches?
// TODO(dantup): renames across multiple files
test_prepare_class() {
@@ -122,6 +119,23 @@
content, 'MyNewClass', expectedContent);
}
+ test_rename_withoutVersionedIdentifier() {
+ // Without sending a document version, the rename should still work because
+ // the server should use the version it had at the start of the rename
+ // operation.
+ const content = '''
+ class MyClass {}
+ final a = new [[My^Class]]();
+ ''';
+ const expectedContent = '''
+ class MyNewClass {}
+ final a = new MyNewClass();
+ ''';
+ return _test_rename_withDocumentChanges(
+ content, 'MyNewClass', expectedContent,
+ sendRenameVersion: false);
+ }
+
test_rename_classNewKeyword() async {
const content = '''
class MyClass {}
@@ -321,16 +335,25 @@
}
_test_rename_withDocumentChanges(
- String content, String newName, String expectedContent) async {
+ String content,
+ String newName,
+ String expectedContent, {
+ sendDocumentVersion = true,
+ sendRenameVersion = true,
+ }) async {
+ // The specific number doesn't matter here, it's just a placeholder to confirm
+ // the values match.
+ final documentVersion = 222;
await initialize(
workspaceCapabilities:
withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
);
- await openFile(mainFileUri, withoutMarkers(content), version: 222);
+ await openFile(mainFileUri, withoutMarkers(content),
+ version: sendDocumentVersion ? documentVersion : null);
final result = await rename(
mainFileUri,
- 222,
+ sendRenameVersion ? documentVersion : null,
positionFromMarker(content),
newName,
);
@@ -341,7 +364,14 @@
final contents = {
mainFilePath: withoutMarkers(content),
};
- applyDocumentChanges(contents, result.documentChanges);
+ final documentVersions = {
+ mainFilePath: documentVersion,
+ };
+ applyDocumentChanges(
+ contents,
+ result.documentChanges,
+ expectedVersions: documentVersions,
+ );
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 fad7baf..ecae12a 100644
--- a/pkg/analysis_server/test/lsp/server_abstract.dart
+++ b/pkg/analysis_server/test/lsp/server_abstract.dart
@@ -55,19 +55,62 @@
}
void applyDocumentChanges(
- Map<String, String> fileContents,
- Either2<
- List<TextDocumentEdit>,
- List<
- Either4<TextDocumentEdit, CreateFile, RenameFile,
- DeleteFile>>>
- documentChanges) {
+ Map<String, String> fileContents,
+ Either2<List<TextDocumentEdit>,
+ List<Either4<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>>>
+ documentChanges, {
+ Map<String, int> expectedVersions,
+ }) {
+ // If we were supplied with expected versions, ensure that all returned
+ // edits match the versions.
+ if (expectedVersions != null) {
+ expectDocumentVersions(documentChanges, expectedVersions);
+ }
documentChanges.map(
(edits) => applyTextDocumentEdits(fileContents, edits),
(changes) => applyResourceChanges(fileContents, changes),
);
}
+ /// Validates the document versions for a set of edits match the versions in
+ /// the supplied map.
+ void expectDocumentVersions(
+ Either2<List<TextDocumentEdit>,
+ List<Either4<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>>>
+ documentChanges,
+ Map<String, int> expectedVersions,
+ ) {
+ documentChanges.map(
+ // Validate versions on simple doc edits
+ (edits) => edits
+ .forEach((edit) => expectDocumentVersion(edit, expectedVersions)),
+ // For resource changes, we only need to validate changes since
+ // creates/renames/deletes do not supply versions.
+ (changes) => changes.forEach((change) {
+ change.map(
+ (edit) => expectDocumentVersion(edit, expectedVersions),
+ (create) => {},
+ (rename) {},
+ (delete) {},
+ );
+ }),
+ );
+ }
+
+ void expectDocumentVersion(
+ TextDocumentEdit edit,
+ Map<String, int> expectedVersions,
+ ) {
+ final path = Uri.parse(edit.textDocument.uri).toFilePath();
+ final expectedVersion = expectedVersions[path];
+
+ if (edit.textDocument is VersionedTextDocumentIdentifier) {
+ expect(edit.textDocument.version, equals(expectedVersion));
+ } else {
+ throw 'Document identifier for $path was not versioned (expected version $expectedVersion)';
+ }
+ }
+
void applyResourceChanges(
Map<String, String> oldFileContent,
List<Either4<TextDocumentEdit, CreateFile, RenameFile, DeleteFile>> changes,
