Skip to content

Commit fe43c00

Browse files
DanTupCommit Queue
DanTup
authored and
Commit Queue
committed
[analysis_server] Add document version support for the legacy server
This adds an optional version number to AddContentOverlay and UpdateContentOverlay in the legacy protocol. It also moves some code (such as the `Map` that stores the current document versions) from the LSP server into the base class, and updates the overlay handlers to update it. And finally, it removes the FailingTest() annotations on the versioned EditArguments test since they now work. Change-Id: Icf2a2825eb6227f5faa3e21ae6c3a7d15997ef5a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404821 Commit-Queue: Samuel Rawlins <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent f9c5944 commit fe43c00

File tree

18 files changed

+250
-105
lines changed

18 files changed

+250
-105
lines changed

‎pkg/analysis_server/doc/api.html‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@
110110
<body>
111111
<h1>Analysis Server API Specification</h1>
112112
<h1 style="color:#999999">Version
113-
1.39.0
113+
1.40.0
114114
</h1>
115115
<p>
116116
This document contains a specification of the API provided by the
@@ -245,6 +245,10 @@ <h3>Enumerations</h3>
245245
ignoring the item or treating it with some default/fallback handling.
246246
</p>
247247
<h3>Changelog</h3>
248+
<h4>1.40.0</h4>
249+
<ul>
250+
<li>Added a an optional <tt>version</tt> field to <tt>AddContentOverlay</tt> and <tt>ChangeContentOverlay</tt> to allow clients/servers to have a way to identify specific versions of documents.</li>
251+
</ul>
248252
<h4>1.39.0</h4>
249253
<ul>
250254
<li>Added a new <tt>lspCapabilities</tt> field to the <tt>setClientCapabilities</tt> parameters to allow clients to provide their LSP capabilities. These capabilities can indicate that a client can support <tt>lsp.handle</tt> requests in the server-to-client direction.</li>
@@ -3283,6 +3287,10 @@ <h2 class="domain"><a name="types">Types</a></h2>
32833287
<p>
32843288
The new content of the file.
32853289
</p>
3290+
</dd><dt class="field"><b>version: int<span style="color:#999999"> (optional)</span></b></dt><dd>
3291+
3292+
<p>An optional version number for the document. Version numbers allow the server to tag edits with the version of the document they apply to which can avoid applying edits to documents that have already been updated since the edits were computed.</p>
3293+
<p>If version numbers are supplied with AddContentOverlay and ChangeContentOverlay, they must be increasing (not not necessarily consecutive) numbers.</p>
32863294
</dd></dl></dd><dt class="typeDefinition"><a name="type_AnalysisError">AnalysisError: object</a></dt><dd>
32873295
<p>
32883296
An indication of an error, warning, or hint that was produced by the
@@ -3512,6 +3520,10 @@ <h2 class="domain"><a name="types">Types</a></h2>
35123520
<p>
35133521
The edits to be applied to the file.
35143522
</p>
3523+
</dd><dt class="field"><b>version: int<span style="color:#999999"> (optional)</span></b></dt><dd>
3524+
3525+
<p>An optional version number for the document. Version numbers allow the server to tag edits with the version of the document they apply to which can avoid applying edits to documents that have already been updated since the edits were computed.</p>
3526+
<p>If version numbers are supplied with AddContentOverlay and ChangeContentOverlay, they must be increasing (not not necessarily consecutive) numbers.</p>
35153527
</dd></dl></dd><dt class="typeDefinition"><a name="type_ClosingLabel">ClosingLabel: object</a></dt><dd>
35163528
<p>
35173529
A label that is associated with a range of code that may be useful to

‎pkg/analysis_server/lib/protocol/protocol_constants.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// To regenerate the file, use the script
77
// "pkg/analysis_server/tool/spec/generate_files".
88

9-
const String PROTOCOL_VERSION = '1.39.0';
9+
const String PROTOCOL_VERSION = '1.40.0';
1010

1111
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES = 'analysis.analyzedFiles';
1212
const String ANALYSIS_NOTIFICATION_ANALYZED_FILES_DIRECTORIES = 'directories';

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,15 @@ abstract class AnalysisServer {
110110
/// The object through which analytics are to be sent.
111111
final AnalyticsManager analyticsManager;
112112

113+
/// The versions of each document known to the server (keyed by path), used to
114+
/// send back to the client for server-initiated edits so that the client can
115+
/// ensure they have a matching version of the document before applying them.
116+
///
117+
/// Handlers should prefer to use the `getVersionedDocumentIdentifier` method
118+
/// which will return a null-versioned identifier if the document version is
119+
/// not known.
120+
final Map<String, int> documentVersions = {};
121+
113122
/// A connection to DTD (the Dart Tooling Daemon) that allows other clients to
114123
/// call server functionality.
115124
///
@@ -635,8 +644,8 @@ abstract class AnalysisServer {
635644
return analysisContext.driver.dartdocDirectiveInfo;
636645
}
637646

638-
/// Gets the current version number of a document (if known).
639-
int? getDocumentVersion(String path);
647+
/// Gets the current version number of a document.
648+
int? getDocumentVersion(String path) => documentVersions[path];
640649

641650
/// Return a [Future] that completes with the [Element] at the given
642651
/// [offset] of the given [file], or with `null` if there is no node at the
@@ -765,7 +774,7 @@ abstract class AnalysisServer {
765774
String path,
766775
) {
767776
return lsp.OptionalVersionedTextDocumentIdentifier(
768-
uri: resourceProvider.pathContext.toUri(path),
777+
uri: uriConverter.toClientUri(path),
769778
version: getDocumentVersion(path),
770779
);
771780
}

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -590,13 +590,6 @@ class LegacyAnalysisServer extends AnalysisServer {
590590
return driver?.getCachedResolvedUnit(path);
591591
}
592592

593-
/// Gets the current version number of a document.
594-
///
595-
/// For the legacy server we do not track version numbers, these are
596-
/// LSP-specific.
597-
@override
598-
int? getDocumentVersion(String path) => null;
599-
600593
@override
601594
FutureOr<void> handleAnalysisStatusChange(analysis.AnalysisStatus status) {
602595
super.handleAnalysisStatusChange(status);
@@ -1015,8 +1008,10 @@ class LegacyAnalysisServer extends AnalysisServer {
10151008

10161009
// Prepare the new contents.
10171010
String? newContents;
1011+
int? newVersion;
10181012
if (change is AddContentOverlay) {
10191013
newContents = change.content;
1014+
newVersion = change.version;
10201015
} else if (change is ChangeContentOverlay) {
10211016
if (oldContents == null) {
10221017
// The client may only send a ChangeContentOverlay if there is
@@ -1033,6 +1028,7 @@ class LegacyAnalysisServer extends AnalysisServer {
10331028
}
10341029
try {
10351030
newContents = SourceEdit.applySequence(oldContents, change.edits);
1031+
newVersion = change.version;
10361032
} on RangeError {
10371033
throw RequestFailure(
10381034
Response(
@@ -1046,6 +1042,7 @@ class LegacyAnalysisServer extends AnalysisServer {
10461042
}
10471043
} else if (change is RemoveContentOverlay) {
10481044
newContents = null;
1045+
newVersion = null;
10491046
} else {
10501047
// Protocol parsing should have ensured that we never get here.
10511048
throw AnalysisException('Illegal change type');
@@ -1060,6 +1057,11 @@ class LegacyAnalysisServer extends AnalysisServer {
10601057
} else {
10611058
resourceProvider.removeOverlay(file);
10621059
}
1060+
if (newVersion != null) {
1061+
documentVersions[file] = newVersion;
1062+
} else {
1063+
documentVersions.remove(file);
1064+
}
10631065

10641066
for (var driver in driverMap.values) {
10651067
driver.changeFile(file);

‎pkg/analysis_server/lib/src/lsp/handlers/handler_text_document_changes.dart‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ class TextDocumentChangeHandler
5959
failureIsCritical: true,
6060
);
6161
return newContents.mapResultSync((result) {
62-
server.documentVersions[path] = params.textDocument;
62+
server.documentVersions[path] = params.textDocument.version;
6363
server.onOverlayUpdated(path, result.edits, newContent: result.content);
6464
return success(null);
6565
});
@@ -124,12 +124,7 @@ class TextDocumentOpenHandler
124124
var path = pathOfDocItem(doc);
125125
return path.mapResult((path) async {
126126
if (isEditableDocument(doc.uri)) {
127-
// We don't get a OptionalVersionedTextDocumentIdentifier with a didOpen but we
128-
// do get the necessary info to create one.
129-
server.documentVersions[path] = VersionedTextDocumentIdentifier(
130-
version: params.textDocument.version,
131-
uri: params.textDocument.uri,
132-
);
127+
server.documentVersions[path] = params.textDocument.version;
133128
// It's critical overlays are processed synchronously because other
134129
// requests that sneak in when we `await` rely on them being
135130
// correct.

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

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,6 @@ class LspAnalysisServer extends AnalysisServer {
7777
/// be sent.
7878
final LspServerCommunicationChannel channel;
7979

80-
/// The versions of each document known to the server (keyed by path), used to
81-
/// send back to the client for server-initiated edits so that the client can
82-
/// ensure they have a matching version of the document before applying them.
83-
///
84-
/// Handlers should prefer to use the `getVersionedDocumentIdentifier` method
85-
/// which will return a null-versioned identifier if the document version is
86-
/// not known.
87-
final Map<String, VersionedTextDocumentIdentifier> documentVersions = {};
88-
8980
/// The message handler for the server based on the current state
9081
/// (uninitialized, initializing, initialized, etc.).
9182
late ServerStateMessageHandler messageHandler;
@@ -390,28 +381,6 @@ class LspAnalysisServer extends AnalysisServer {
390381
unawaited(capabilitiesComputer.performDynamicRegistration());
391382
}
392383

393-
/// Gets the current version number of a document.
394-
@override
395-
int? getDocumentVersion(String path) => documentVersions[path]?.version;
396-
397-
/// Gets the current identifier/version of a document known to the server,
398-
/// returning an [OptionalVersionedTextDocumentIdentifier] with a version of
399-
/// `null` if the document version is not known.
400-
///
401-
/// Prefer using [HandlerHelperMixin.extractDocumentVersion] when you
402-
/// already have a [TextDocumentIdentifier] from the client because it is
403-
/// guaranteed to be what the client expected and not just the current version
404-
/// the server has.
405-
@override
406-
OptionalVersionedTextDocumentIdentifier getVersionedDocumentIdentifier(
407-
String path,
408-
) {
409-
return OptionalVersionedTextDocumentIdentifier(
410-
uri: uriConverter.toClientUri(path),
411-
version: getDocumentVersion(path),
412-
);
413-
}
414-
415384
@override
416385
FutureOr<void> handleAnalysisStatusChange(
417386
analysis.AnalysisStatus status,

‎pkg/analysis_server/test/integration/support/protocol_matchers.dart‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,14 @@ import 'integration_tests.dart';
2020
/// {
2121
/// "type": "add"
2222
/// "content": String
23+
/// "version": optional int
2324
/// }
2425
final Matcher isAddContentOverlay = LazyMatcher(
25-
() => MatchesJsonObject('AddContentOverlay', {
26-
'type': equals('add'),
27-
'content': isString,
28-
}),
26+
() => MatchesJsonObject(
27+
'AddContentOverlay',
28+
{'type': equals('add'), 'content': isString},
29+
optionalFields: {'version': isInt},
30+
),
2931
);
3032

3133
/// AnalysisError
@@ -206,12 +208,14 @@ final Matcher isBulkFixDetail = LazyMatcher(
206208
/// {
207209
/// "type": "change"
208210
/// "edits": List<SourceEdit>
211+
/// "version": optional int
209212
/// }
210213
final Matcher isChangeContentOverlay = LazyMatcher(
211-
() => MatchesJsonObject('ChangeContentOverlay', {
212-
'type': equals('change'),
213-
'edits': isListOf(isSourceEdit),
214-
}),
214+
() => MatchesJsonObject(
215+
'ChangeContentOverlay',
216+
{'type': equals('change'), 'edits': isListOf(isSourceEdit)},
217+
optionalFields: {'version': isInt},
218+
),
215219
);
216220

217221
/// ClosingLabel

‎pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,10 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest
199199
@override
200200
ClientUriConverter get uriConverter => server.uriConverter;
201201

202-
Future<void> addOverlay(String filePath, String content) {
202+
Future<void> addOverlay(String filePath, String content, [int? version]) {
203203
return handleSuccessfulRequest(
204204
AnalysisUpdateContentParams({
205-
convertPath(filePath): AddContentOverlay(content),
205+
convertPath(filePath): AddContentOverlay(content, version: version),
206206
}).toRequest(
207207
'${nextRequestId++}',
208208
clientUriConverter: server.uriConverter,
@@ -417,8 +417,7 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
417417

418418
@override
419419
Future<void> openFile(Uri uri, String content, {int version = 1}) async {
420-
// TODO(dantup): Add version here when legacy protocol supports.
421-
await addOverlay(fromUri(uri), content);
420+
await addOverlay(fromUri(uri), content, version);
422421
}
423422

424423
/// Opens a file content without providing a version number.
@@ -432,9 +431,8 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
432431

433432
@override
434433
Future<void> replaceFile(int newVersion, Uri uri, String content) async {
435-
// TODO(dantup): Add version here when legacy protocol supports.
436434
// For legacy, we can use addOverlay to replace the whole file.
437-
await addOverlay(fromUri(uri), content);
435+
await addOverlay(fromUri(uri), content, newVersion);
438436
}
439437

440438
/// Replaces a file content without providing a version number.

‎pkg/analysis_server/test/lsp_over_legacy/editable_arguments_test.dart‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -94,20 +94,4 @@ class MyWidget extends StatelessWidget {
9494
.having((td) => td.version, 'version', isNull),
9595
);
9696
}
97-
98-
@override
99-
@FailingTest(reason: 'Document versions not currently supported for legacy')
100-
test_textDocument_versioned() {
101-
// TODO(dantup): Implement support for version numbers in the legacy
102-
// protocol.
103-
return super.test_textDocument_versioned();
104-
}
105-
106-
@override
107-
@FailingTest(reason: 'Document versions not currently supported for legacy')
108-
test_textDocument_versioned_closedFile() {
109-
// TODO(dantup): Implement support for version numbers in the legacy
110-
// protocol.
111-
return super.test_textDocument_versioned_closedFile();
112-
}
11397
}

‎pkg/analysis_server/tool/spec/generated/java/types/AddContentOverlay.java‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,28 +41,41 @@ public class AddContentOverlay {
4141
*/
4242
private final String content;
4343

44+
/**
45+
* An optional version number for the document. Version numbers allow the server to tag edits with
46+
* the version of the document they apply to which can avoid applying edits to documents that have
47+
* already been updated since the edits were computed.
48+
*
49+
* If version numbers are supplied with AddContentOverlay and ChangeContentOverlay, they must be
50+
* increasing (not not necessarily consecutive) numbers.
51+
*/
52+
private final Integer version;
53+
4454
/**
4555
* Constructor for {@link AddContentOverlay}.
4656
*/
47-
public AddContentOverlay(String content) {
57+
public AddContentOverlay(String content, Integer version) {
4858
this.type = "add";
4959
this.content = content;
60+
this.version = version;
5061
}
5162

5263
@Override
5364
public boolean equals(Object obj) {
5465
if (obj instanceof AddContentOverlay other) {
5566
return
5667
Objects.equals(other.type, type) &&
57-
Objects.equals(other.content, content);
68+
Objects.equals(other.content, content) &&
69+
Objects.equals(other.version, version);
5870
}
5971
return false;
6072
}
6173

6274
public static AddContentOverlay fromJson(JsonObject jsonObject) {
6375
String type = jsonObject.get("type").getAsString();
6476
String content = jsonObject.get("content").getAsString();
65-
return new AddContentOverlay(content);
77+
Integer version = jsonObject.get("version") == null ? null : jsonObject.get("version").getAsInt();
78+
return new AddContentOverlay(content, version);
6679
}
6780

6881
public static List<AddContentOverlay> fromJsonArray(JsonArray jsonArray) {
@@ -87,18 +100,34 @@ public String getType() {
87100
return type;
88101
}
89102

103+
/**
104+
* An optional version number for the document. Version numbers allow the server to tag edits with
105+
* the version of the document they apply to which can avoid applying edits to documents that have
106+
* already been updated since the edits were computed.
107+
*
108+
* If version numbers are supplied with AddContentOverlay and ChangeContentOverlay, they must be
109+
* increasing (not not necessarily consecutive) numbers.
110+
*/
111+
public Integer getVersion() {
112+
return version;
113+
}
114+
90115
@Override
91116
public int hashCode() {
92117
return Objects.hash(
93118
type,
94-
content
119+
content,
120+
version
95121
);
96122
}
97123

98124
public JsonObject toJson() {
99125
JsonObject jsonObject = new JsonObject();
100126
jsonObject.addProperty("type", type);
101127
jsonObject.addProperty("content", content);
128+
if (version != null) {
129+
jsonObject.addProperty("version", version);
130+
}
102131
return jsonObject;
103132
}
104133

@@ -111,6 +140,9 @@ public String toString() {
111140
builder.append(", ");
112141
builder.append("content=");
113142
builder.append(content);
143+
builder.append(", ");
144+
builder.append("version=");
145+
builder.append(version);
114146
builder.append("]");
115147
return builder.toString();
116148
}

0 commit comments

Comments
 (0)