Skip to content

Commit 800421b

Browse files
DanTupCommit Queue
authored and
Commit Queue
committedOct 10, 2023
[analysis_server] Refactor servers to allow producing edits for LSP over Legacy
This is a no-op refactor extracted from an upcoming change to enable the onWillRenameFiles request to work for LSP over the Legacy protocol. It's mostly lifting some API from the LSP server to the base server and some into a new mixin (`LspVerifyEditHelpersMixin`) to provide the required methods for verifying edits without the usual LSP base test classes. Change-Id: Id6084a80e3afb9c6af1a84d9e8a18d460b5f4cce Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329980 Commit-Queue: Keerti Parthasarathy <keertip@google.com> Reviewed-by: Keerti Parthasarathy <keertip@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
1 parent 07bb4d1 commit 800421b

File tree

11 files changed

+128
-66
lines changed

11 files changed

+128
-66
lines changed
 

‎pkg/analysis_server/lib/src/analysis_server.dart

+39-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import 'dart:io' as io;
77
import 'dart:io';
88

99
import 'package:analysis_server/lsp_protocol/protocol.dart' as lsp
10-
show MessageType;
10+
show MessageType, OptionalVersionedTextDocumentIdentifier;
1111
import 'package:analysis_server/src/analytics/analytics_manager.dart';
1212
import 'package:analysis_server/src/collections.dart';
1313
import 'package:analysis_server/src/context_manager.dart';
@@ -29,6 +29,7 @@ import 'package:analysis_server/src/services/correction/namespace.dart';
2929
import 'package:analysis_server/src/services/pub/pub_api.dart';
3030
import 'package:analysis_server/src/services/pub/pub_command.dart';
3131
import 'package:analysis_server/src/services/pub/pub_package_service.dart';
32+
import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
3233
import 'package:analysis_server/src/services/search/element_visitors.dart';
3334
import 'package:analysis_server/src/services/search/search_engine.dart';
3435
import 'package:analysis_server/src/services/search/search_engine_internal.dart';
@@ -49,6 +50,7 @@ import 'package:analyzer/file_system/file_system.dart';
4950
import 'package:analyzer/file_system/overlay_file_system.dart';
5051
import 'package:analyzer/file_system/physical_file_system.dart';
5152
import 'package:analyzer/instrumentation/instrumentation.dart';
53+
import 'package:analyzer/source/line_info.dart';
5254
import 'package:analyzer/src/dart/analysis/byte_store.dart';
5355
import 'package:analyzer/src/dart/analysis/driver.dart' as analysis;
5456
import 'package:analyzer/src/dart/analysis/driver.dart';
@@ -202,6 +204,10 @@ abstract class AnalysisServer {
202204
/// Starts completed and will be replaced each time a context rebuild starts.
203205
Completer<void> analysisContextRebuildCompleter = Completer()..complete();
204206

207+
/// The workspace for rename refactorings.
208+
late final refactoringWorkspace =
209+
RefactoringWorkspace(driverMap.values, searchEngine);
210+
205211
AnalysisServer(
206212
this.options,
207213
this.sdkManager,
@@ -477,6 +483,9 @@ abstract class AnalysisServer {
477483
DartdocDirectiveInfo();
478484
}
479485

486+
/// Gets the current version number of a document (if known).
487+
int? getDocumentVersion(String path);
488+
480489
/// Return a [Future] that completes with the [Element] at the given
481490
/// [offset] of the given [file], or with `null` if there is no node at the
482491
/// [offset] or the node does not have an element.
@@ -524,6 +533,25 @@ abstract class AnalysisServer {
524533
return element;
525534
}
526535

536+
/// Return a [LineInfo] for the file with the given [path].
537+
///
538+
/// If the file does not exist or cannot be read, returns `null`.
539+
///
540+
/// This method supports non-Dart files but uses the current content of the
541+
/// file which may not be the latest analyzed version of the file if it was
542+
/// recently modified, so using the lineInfo from an analyzed result may be
543+
/// preferable.
544+
LineInfo? getLineInfo(String path) {
545+
try {
546+
final content = resourceProvider.getFile(path).readAsStringSync();
547+
return LineInfo.fromContent(content);
548+
} on FileSystemException {
549+
// If the file does not exist or cannot be read, return null to allow
550+
// the caller to decide how to handle this.
551+
return null;
552+
}
553+
}
554+
527555
/// Return a [Future] that completes with the resolved [AstNode] at the
528556
/// given [offset] of the given [file], or with `null` if there is no node as
529557
/// the [offset].
@@ -606,6 +634,16 @@ abstract class AnalysisServer {
606634
});
607635
}
608636

637+
/// Gets the version of a document known to the server, returning a
638+
/// [lsp.OptionalVersionedTextDocumentIdentifier] with a version of `null` if the
639+
/// document version is not known.
640+
lsp.OptionalVersionedTextDocumentIdentifier getVersionedDocumentIdentifier(
641+
String path) {
642+
return lsp.OptionalVersionedTextDocumentIdentifier(
643+
uri: resourceProvider.pathContext.toUri(path),
644+
version: getDocumentVersion(path));
645+
}
646+
609647
@mustCallSuper
610648
FutureOr<void> handleAnalysisStatusChange(analysis.AnalysisStatus status) {
611649
if (isFirstAnalysisSinceContextsBuilt && !status.isAnalyzing) {

‎pkg/analysis_server/lib/src/handler/legacy/lsp_over_legacy_handler.dart

+7
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import 'package:analysis_server/src/handler/legacy/legacy_handler.dart';
1010
import 'package:analysis_server/src/lsp/constants.dart';
1111
import 'package:analysis_server/src/lsp/handlers/handler_states.dart';
1212
import 'package:analysis_server/src/lsp/handlers/handlers.dart' as lsp;
13+
import 'package:analyzer/dart/analysis/session.dart';
1314
import 'package:language_server_protocol/json_parsing.dart';
1415
import 'package:language_server_protocol/protocol_custom_generated.dart';
16+
import 'package:language_server_protocol/protocol_generated.dart';
1517
import 'package:language_server_protocol/protocol_special.dart';
1618

1719
/// The handler for the `lsp.handle` request.
@@ -71,6 +73,11 @@ class LspOverLegacyHandler extends LegacyHandler {
7173
ErrorOr<Object?> result;
7274
try {
7375
result = await handler.handleMessage(message, messageInfo);
76+
} on InconsistentAnalysisException {
77+
result = error(
78+
ErrorCodes.ContentModified,
79+
'Document was modified before operation completed',
80+
);
7481
} catch (e) {
7582
final errorMessage =
7683
'An error occurred while handling ${message.method} request: $e';

‎pkg/analysis_server/lib/src/legacy_analysis_server.dart

+7-5
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ import 'package:analysis_server/src/server/sdk_configuration.dart';
9393
import 'package:analysis_server/src/services/completion/completion_state.dart';
9494
import 'package:analysis_server/src/services/execution/execution_context.dart';
9595
import 'package:analysis_server/src/services/flutter/widget_descriptions.dart';
96-
import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
9796
import 'package:analysis_server/src/services/refactoring/legacy/refactoring_manager.dart';
9897
import 'package:analysis_server/src/services/user_prompts/dart_fix_prompt_manager.dart';
9998
import 'package:analysis_server/src/utilities/process.dart';
@@ -320,9 +319,6 @@ class LegacyAnalysisServer extends AnalysisServer {
320319
/// The state used by the completion domain handlers.
321320
final CompletionState completionState = CompletionState();
322321

323-
/// The workspace for rename refactorings.
324-
late RefactoringWorkspace refactoringWorkspace;
325-
326322
/// The object used to manage uncompleted refactorings.
327323
late RefactoringManager? _refactoringManager;
328324

@@ -428,7 +424,6 @@ class LegacyAnalysisServer extends AnalysisServer {
428424
);
429425
debounceRequests(channel, discardedRequests)
430426
.listen(handleRequestOrResponse, onDone: done, onError: error);
431-
refactoringWorkspace = RefactoringWorkspace(driverMap.values, searchEngine);
432427
_newRefactoringManager();
433428
}
434429

@@ -513,6 +508,13 @@ class LegacyAnalysisServer extends AnalysisServer {
513508
return driver?.getCachedResult(path);
514509
}
515510

511+
/// Gets the current version number of a document.
512+
///
513+
/// For the legacy server we do not track version numbers, these are
514+
/// LSP-specific.
515+
@override
516+
int? getDocumentVersion(String path) => null;
517+
516518
@override
517519
FutureOr<void> handleAnalysisStatusChange(analysis.AnalysisStatus status) {
518520
super.handleAnalysisStatusChange(status);

‎pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart

+4-30
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import 'package:analysis_server/src/server/detachable_filesystem_manager.dart';
2828
import 'package:analysis_server/src/server/diagnostic_server.dart';
2929
import 'package:analysis_server/src/server/error_notifier.dart';
3030
import 'package:analysis_server/src/server/performance.dart';
31-
import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
3231
import 'package:analysis_server/src/services/user_prompts/dart_fix_prompt_manager.dart';
3332
import 'package:analysis_server/src/utilities/flutter.dart';
3433
import 'package:analysis_server/src/utilities/process.dart';
@@ -39,7 +38,6 @@ import 'package:analyzer/error/error.dart';
3938
import 'package:analyzer/exception/exception.dart';
4039
import 'package:analyzer/file_system/file_system.dart';
4140
import 'package:analyzer/instrumentation/instrumentation.dart';
42-
import 'package:analyzer/source/line_info.dart';
4341
import 'package:analyzer/src/dart/analysis/driver.dart' as analysis;
4442
import 'package:analyzer/src/dart/analysis/status.dart' as analysis;
4543
import 'package:analyzer/src/generated/sdk.dart';
@@ -78,10 +76,6 @@ class LspAnalysisServer extends AnalysisServer {
7876
/// be sent.
7977
final LspServerCommunicationChannel channel;
8078

81-
/// The workspace for rename refactorings. Should be accessed through the
82-
/// refactoringWorkspace getter to be automatically created (lazily).
83-
RefactoringWorkspace? _refactoringWorkspace;
84-
8579
/// The versions of each document known to the server (keyed by path), used to
8680
/// send back to the client for server-initiated edits so that the client can
8781
/// ensure they have a matching version of the document before applying them.
@@ -260,9 +254,6 @@ class LspAnalysisServer extends AnalysisServer {
260254
}
261255
}
262256

263-
RefactoringWorkspace get refactoringWorkspace => _refactoringWorkspace ??=
264-
RefactoringWorkspace(driverMap.values, searchEngine);
265-
266257
/// Whether or not the client has advertised support for
267258
/// 'window/showMessageRequest'.
268259
///
@@ -355,30 +346,13 @@ class LspAnalysisServer extends AnalysisServer {
355346
}
356347

357348
/// Gets the current version number of a document.
349+
@override
358350
int? getDocumentVersion(String path) => documentVersions[path]?.version;
359351

360-
/// Return a [LineInfo] for the file with the given [path].
361-
///
362-
/// If the file does not exist or cannot be read, returns `null`.
363-
///
364-
/// This method supports non-Dart files but uses the current content of the
365-
/// file which may not be the latest analyzed version of the file if it was
366-
/// recently modified, so using the lineInfo from an analyzed result may be
367-
/// preferable.
368-
LineInfo? getLineInfo(String path) {
369-
try {
370-
final content = resourceProvider.getFile(path).readAsStringSync();
371-
return LineInfo.fromContent(content);
372-
} on FileSystemException {
373-
// If the file does not exist or cannot be read, return null to allow
374-
// the caller to decide how to handle this.
375-
return null;
376-
}
377-
}
378-
379352
/// Gets the version of a document known to the server, returning a
380-
/// [OptionalVersionedTextDocumentIdentifier] with a version of `null` if the document
381-
/// version is not known.
353+
/// [OptionalVersionedTextDocumentIdentifier] with a version of `null` if the
354+
/// document version is not known.
355+
@override
382356
OptionalVersionedTextDocumentIdentifier getVersionedDocumentIdentifier(
383357
String path) {
384358
return OptionalVersionedTextDocumentIdentifier(

‎pkg/analysis_server/lib/src/lsp/mapping.dart

+3-2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:analysis_server/lsp_protocol/protocol.dart' as lsp;
66
import 'package:analysis_server/lsp_protocol/protocol.dart' hide Declaration;
7+
import 'package:analysis_server/src/analysis_server.dart';
78
import 'package:analysis_server/src/collections.dart';
89
import 'package:analysis_server/src/computer/computer_hover.dart';
910
import 'package:analysis_server/src/lsp/client_capabilities.dart';
@@ -81,7 +82,7 @@ lsp.Either2<lsp.MarkupContent, String> asMarkupContentOrString(
8182
/// it's important to call this immediately after computing edits to ensure
8283
/// the document is not modified before the version number is read.
8384
lsp.WorkspaceEdit createPlainWorkspaceEdit(
84-
lsp.LspAnalysisServer server, List<server.SourceFileEdit> edits) {
85+
AnalysisServer server, List<server.SourceFileEdit> edits) {
8586
return toWorkspaceEdit(
8687
// Client capabilities are always available after initialization.
8788
server.lspClientCapabilities!,
@@ -133,7 +134,7 @@ WorkspaceEdit createRenameEdit(
133134
/// it's important to call this immediately after computing edits to ensure
134135
/// the document is not modified before the version number is read.
135136
lsp.WorkspaceEdit createWorkspaceEdit(
136-
lsp.LspAnalysisServer server,
137+
AnalysisServer server,
137138
server.SourceChange change, {
138139
// The caller must specify whether snippets are valid here for where they're
139140
// sending this edit. Right now, support is limited to CodeActions.

‎pkg/analysis_server/test/lsp/change_verifier.dart

+14-11
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
import 'package:analysis_server/lsp_protocol/protocol.dart';
66
import 'package:collection/collection.dart';
7-
import 'package:test/test.dart' hide expect;
7+
import 'package:test/test.dart';
88

9-
import 'server_abstract.dart';
9+
import 'request_helpers_mixin.dart';
1010

1111
/// Applies LSP [WorkspaceEdit]s to produce a flattened string describing the
1212
/// new file contents and any create/rename/deletes to use in test expectations.
@@ -21,18 +21,18 @@ class LspChangeVerifier {
2121
/// Changes collected while applying the edit.
2222
final _changes = <Uri, _Change>{};
2323

24-
/// A base test class used to obtain the current content of a file.
25-
final LspAnalysisServerTestMixin _server;
24+
/// A mixin with helpers for applying LSP edits.
25+
final LspVerifyEditHelpersMixin editHelpers;
2626

2727
/// The [WorkspaceEdit] being applied/verified.
2828
final WorkspaceEdit edit;
2929

30-
LspChangeVerifier(this._server, this.edit) {
30+
LspChangeVerifier(this.editHelpers, this.edit) {
3131
_applyEdit();
3232
}
3333

3434
void verifyFiles(String expected, {Map<Uri, int>? expectedVersions}) {
35-
_server.expect(_toChangeString(), equals(expected));
35+
expect(_toChangeString(), equals(expected));
3636
if (expectedVersions != null) {
3737
_verifyDocumentVersions(expectedVersions);
3838
}
@@ -134,11 +134,13 @@ class LspChangeVerifier {
134134
final indexedEdits =
135135
edit.edits.mapIndexed(TextEditWithIndex.fromUnion).toList();
136136
indexedEdits.sort(TextEditWithIndex.compare);
137-
return indexedEdits.map((e) => e.edit).fold(content, _server.applyTextEdit);
137+
return indexedEdits
138+
.map((e) => e.edit)
139+
.fold(content, editHelpers.applyTextEdit);
138140
}
139141

140142
String _applyTextEdits(String content, List<TextEdit> changes) =>
141-
_server.applyTextEdits(content, changes);
143+
editHelpers.applyTextEdits(content, changes);
142144

143145
_Change _change(Uri fileUri) => _changes.putIfAbsent(
144146
fileUri, () => _Change(_getCurrentFileContent(fileUri)));
@@ -150,12 +152,13 @@ class LspChangeVerifier {
150152
final uri = edit.textDocument.uri;
151153
final expectedVersion = expectedVersions[uri];
152154

153-
_server.expect(edit.textDocument.version, equals(expectedVersion));
155+
expect(edit.textDocument.version, equals(expectedVersion));
154156
}
155157

156-
String? _getCurrentFileContent(Uri uri) => _server.getCurrentFileContent(uri);
158+
String? _getCurrentFileContent(Uri uri) =>
159+
editHelpers.getCurrentFileContent(uri);
157160

158-
String _relativeUri(Uri uri) => _server.relativeUri(uri);
161+
String _relativeUri(Uri uri) => editHelpers.relativeUri(uri);
159162

160163
String _toChangeString() {
161164
final buffer = StringBuffer();

‎pkg/analysis_server/test/lsp/request_helpers_mixin.dart

+27
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ import 'package:analysis_server/src/lsp/mapping.dart';
1010
import 'package:analyzer/source/line_info.dart';
1111
import 'package:collection/collection.dart';
1212
import 'package:language_server_protocol/json_parsing.dart';
13+
import 'package:path/path.dart' as path;
1314
import 'package:test/test.dart' as test show expect;
1415
import 'package:test/test.dart' hide expect;
1516

1617
import 'change_verifier.dart';
1718

19+
/// A mixin with helpers for applying LSP edits to strings.
1820
mixin LspEditHelpersMixin {
1921
String applyTextEdit(String content, TextEdit edit) {
2022
final startPos = edit.range.start;
@@ -660,3 +662,28 @@ mixin LspRequestHelpersMixin {
660662
};
661663
}
662664
}
665+
666+
/// A mixin with helpers for verifying LSP edits in a given project.
667+
///
668+
/// Extends [LspEditHelpersMixin] with methods for accessing file state and
669+
/// information about the project to build paths.
670+
mixin LspVerifyEditHelpersMixin on LspEditHelpersMixin {
671+
path.Context get pathContext;
672+
673+
String get projectFolderPath;
674+
675+
/// A function to get the current contents of a file to apply edits.
676+
String? getCurrentFileContent(Uri uri);
677+
678+
/// Formats a path relative to the project root always using forward slashes.
679+
///
680+
/// This is used in the text format for comparing edits.
681+
String relativePath(String filePath) => pathContext
682+
.relative(filePath, from: projectFolderPath)
683+
.replaceAll(r'\', '/');
684+
685+
/// Formats a path relative to the project root always using forward slashes.
686+
///
687+
/// This is used in the text format for comparing edits.
688+
String relativeUri(Uri uri) => relativePath(pathContext.fromUri(uri));
689+
}

‎pkg/analysis_server/test/lsp/server_abstract.dart

+1-12
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ abstract class AbstractLspAnalysisServerTest
4646
ClientCapabilitiesHelperMixin,
4747
LspRequestHelpersMixin,
4848
LspEditHelpersMixin,
49+
LspVerifyEditHelpersMixin,
4950
LspAnalysisServerTestMixin,
5051
ConfigurationFilesMixin {
5152
late MockLspServerChannel channel;
@@ -1372,18 +1373,6 @@ mixin LspAnalysisServerTestMixin
13721373
);
13731374
}
13741375

1375-
/// Formats a path relative to the project root always using forward slashes.
1376-
///
1377-
/// This is used in the text format for comparing edits.
1378-
String relativePath(String filePath) => pathContext
1379-
.relative(filePath, from: projectFolderPath)
1380-
.replaceAll(r'\', '/');
1381-
1382-
/// Formats a path relative to the project root always using forward slashes.
1383-
///
1384-
/// This is used in the text format for comparing edits.
1385-
String relativeUri(Uri uri) => relativePath(pathContext.fromUri(uri));
1386-
13871376
Future<WorkspaceEdit?> rename(
13881377
Uri uri,
13891378
int? version,

0 commit comments

Comments
 (0)