Skip to content

Commit 199d50a

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Check for applyEdit support before handling EditArguments
If we get an editArgument request (usually via DDS), we should verify the connected editor supports edits and return an explicit message if not. Also adds a `setUp()` declaration to `SharedTestInterface` so shared tests can set client capabilities instead of them being duplicated in each test class (and moves the same for ApplyEdit tests to this). Change-Id: Ie6a85620b3b53663ce92e55d93e3aefcaf4dc980 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405342 Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Elliott Brooks <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]>
1 parent 3cc308d commit 199d50a

File tree

8 files changed

+63
-33
lines changed

8 files changed

+63
-33
lines changed

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
4747
return serverNotInitializedError;
4848
}
4949

50+
if (!editorClientCapabilities.applyEdit) {
51+
return error(
52+
ErrorCodes.RequestFailed,
53+
'The connected editor does not support applying edits',
54+
);
55+
}
56+
5057
var textDocument = params.textDocument;
5158
var position = params.position;
5259

pkg/analysis_server/test/lsp/edit_argument_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,8 @@ class EditArgumentTest extends SharedAbstractLspAnalysisServerTest
221221
with
222222
SharedEditArgumentTests {
223223
@override
224-
void setUp() {
225-
super.setUp();
224+
Future<void> setUp() async {
225+
await super.setUp();
226226

227227
writeTestPackageConfig(flutter: true);
228228
}

pkg/analysis_server/test/lsp/workspace_apply_edit_test.dart

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,4 @@ class WorkspaceApplyEditTest extends SharedAbstractLspAnalysisServerTest
1818
with
1919
// Tests are defined in SharedWorkspaceApplyEditTests because they
2020
// are shared and run for both LSP and Legacy servers.
21-
SharedWorkspaceApplyEditTests {
22-
@override
23-
Future<void> setUp() async {
24-
super.setUp();
25-
setApplyEditSupport();
26-
setFileCreateSupport();
27-
setDocumentChangesSupport();
28-
}
29-
}
21+
SharedWorkspaceApplyEditTests {}

pkg/analysis_server/test/lsp_over_legacy/edit_argument_test.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:async';
6+
57
import 'package:test_reflective_loader/test_reflective_loader.dart';
68

79
import '../shared/shared_edit_argument_tests.dart';
@@ -19,6 +21,12 @@ class EditArgumentTest extends SharedLspOverLegacyTest
1921
// Tests are defined in SharedEditArgumentTests because they
2022
// are shared and run for both LSP and Legacy servers.
2123
SharedEditArgumentTests {
24+
@override
25+
Future<void> initializeServer() async {
26+
await super.initializeServer();
27+
await sendClientCapabilities();
28+
}
29+
2230
@override
2331
Future<void> setUp() async {
2432
await super.setUp();

pkg/analysis_server/test/lsp_over_legacy/workspace_apply_edit_test.dart

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,4 @@ class WorkspaceApplyEditTest extends SharedLspOverLegacyTest
2424
await super.initializeServer();
2525
await sendClientCapabilities();
2626
}
27-
28-
@override
29-
Future<void> setUp() async {
30-
await super.setUp();
31-
setApplyEditSupport();
32-
setFileCreateSupport();
33-
setDocumentChangesSupport();
34-
}
3527
}

pkg/analysis_server/test/shared/shared_edit_argument_tests.dart

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,34 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:async';
6+
57
import 'package:analysis_server/lsp_protocol/protocol.dart';
68
import 'package:analyzer/src/test_utilities/test_code_format.dart';
79
import 'package:test/test.dart';
810

911
import '../lsp/request_helpers_mixin.dart';
12+
import '../lsp/server_abstract.dart';
1013
import '../tool/lsp_spec/matchers.dart';
1114
import '../utils/test_code_extensions.dart';
1215
import 'shared_test_interface.dart';
1316

1417
/// Shared edit argument tests that are used by both LSP + Legacy server
1518
/// tests.
1619
mixin SharedEditArgumentTests
17-
on SharedTestInterface, LspRequestHelpersMixin, LspVerifyEditHelpersMixin {
20+
on
21+
SharedTestInterface,
22+
LspRequestHelpersMixin,
23+
LspVerifyEditHelpersMixin,
24+
ClientCapabilitiesHelperMixin {
1825
late TestCode code;
1926

27+
@override
28+
Future<void> setUp() async {
29+
await super.setUp();
30+
setApplyEditSupport();
31+
}
32+
2033
test_comma_addArg_addsIfExists() async {
2134
await _expectSimpleArgumentEdit(
2235
params: '({ int? x, int? y })',
@@ -627,6 +640,17 @@ const myConst = E.one;
627640
);
628641
}
629642

643+
test_unsupported_noApplyEditSupport() async {
644+
setApplyEditSupport(false);
645+
646+
await _expectFailedEdit(
647+
params: '({ required String x })',
648+
originalArgs: "(x: 'a')",
649+
edit: ArgumentEdit(name: 'x'),
650+
message: 'The connected editor does not support applying edits',
651+
);
652+
}
653+
630654
/// Initializes the server with [content] and tries to apply the argument
631655
/// [edit] at the marked location. Verifies the changes made match
632656
/// [expectedContent].

pkg/analysis_server/test/shared/shared_test_interface.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:async';
6+
57
/// A common interface that can be implemented by a set of base classes to
68
/// allow tests to be written to run in different configurations
79
/// (LSP and LSP-over-Legacy, and in-process and out-of-process).
@@ -49,4 +51,6 @@ abstract interface class SharedTestInterface {
4951
/// Tells the server that the file with [uri] has had it's content replaced
5052
/// with [content].
5153
Future<void> replaceFile(int newVersion, Uri uri, String content);
54+
55+
FutureOr<void> setUp();
5256
}

pkg/analysis_server/test/shared/shared_workspace_apply_edit_tests.dart

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,30 @@ import 'package:test/test.dart';
1212

1313
import '../lsp/change_verifier.dart';
1414
import '../lsp/request_helpers_mixin.dart';
15+
import '../lsp/server_abstract.dart';
16+
import 'shared_test_interface.dart';
1517

1618
/// Shared `workspace/applyEdit` tests that are used by both LSP and legacy
1719
/// server tests.
1820
mixin SharedWorkspaceApplyEditTests
19-
on LspRequestHelpersMixin, LspVerifyEditHelpersMixin {
20-
/// Overridden by test subclasses to provide the path of a file for testing.
21-
String get testFilePath;
22-
23-
/// The URI for [testFilePath].
24-
Uri get testFileUri => Uri.file(testFilePath);
25-
26-
/// Overridden by test subclasses to create a new file.
27-
void createFile(String path, String content);
28-
29-
/// Overridden by test subclasses to initialize the server.
30-
Future<void> initializeServer();
31-
21+
on
22+
SharedTestInterface,
23+
LspRequestHelpersMixin,
24+
LspVerifyEditHelpersMixin,
25+
ClientCapabilitiesHelperMixin {
3226
/// Overridden by test subclasses to send LSP requests from the server to
3327
/// the client.
3428
Future<ResponseMessage> sendLspRequest(Method method, Object params);
3529

30+
/// Overridden by test subclasses to initialize the server.
31+
@override
32+
Future<void> setUp() async {
33+
super.setUp();
34+
setApplyEditSupport();
35+
setFileCreateSupport();
36+
setDocumentChangesSupport();
37+
}
38+
3639
test_applyEdit_existingFile() async {
3740
var code = TestCode.parse('''
3841
void f() {

0 commit comments

Comments
 (0)