Skip to content

Commit e9abc78

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Enable textDocument/codeActions and related refactor commands for LSP-over-Legacy
This moves the codeAction handler and the refactor commands to be shared (and therefor work for either base server kind). It also adds LSP-over-Legacy test classes that use the shared mixins so that all relevant tests run in both modes. It does _not_ yet enable this functionality for DTD (though this is a requirement to do so in a way that will work for either IDE/server). Change-Id: I790b98dafc59e83e72b399c9b88b94003a102f66 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428322 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent 496c5ca commit e9abc78

13 files changed

+213
-15
lines changed

pkg/analysis_server/lib/src/lsp/handlers/commands/abstract_refactor.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
import 'dart:async';
66

77
import 'package:analysis_server/lsp_protocol/protocol.dart';
8+
import 'package:analysis_server/src/analysis_server.dart';
89
import 'package:analysis_server/src/lsp/client_capabilities.dart';
910
import 'package:analysis_server/src/lsp/constants.dart';
1011
import 'package:analysis_server/src/lsp/error_or.dart';
1112
import 'package:analysis_server/src/lsp/handlers/commands/simple_edit_handler.dart';
1213
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
13-
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
1414
import 'package:analysis_server/src/lsp/progress.dart';
1515
import 'package:analysis_server/src/protocol_server.dart';
1616
import 'package:analysis_server/src/services/refactoring/legacy/refactoring.dart';
@@ -23,7 +23,7 @@ final _manager = LspRefactorManager._();
2323
/// A base class for refactoring commands that need to create Refactorings from
2424
/// client-supplied arguments.
2525
abstract class AbstractRefactorCommandHandler
26-
extends SimpleEditCommandHandler<LspAnalysisServer>
26+
extends SimpleEditCommandHandler<AnalysisServer>
2727
with PositionalArgCommandHandler {
2828
AbstractRefactorCommandHandler(super.server);
2929

pkg/analysis_server/lib/src/lsp/handlers/commands/perform_refactor.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:analysis_server/src/lsp/client_capabilities.dart';
88
import 'package:analysis_server/src/lsp/error_or.dart';
99
import 'package:analysis_server/src/lsp/handlers/commands/abstract_refactor.dart';
1010
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
11+
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
1112
import 'package:analysis_server/src/lsp/mapping.dart';
1213
import 'package:analysis_server/src/lsp/progress.dart';
1314
import 'package:analysis_server/src/protocol_server.dart';
@@ -74,7 +75,10 @@ class PerformRefactorCommandHandler extends AbstractRefactorCommandHandler {
7475
// Show the error to the user but don't fail the request, as the
7576
// LSP Client may show a failed request in a way that looks like a
7677
// server error.
77-
server.showErrorMessageToUser(status.message!);
78+
if (server case LspAnalysisServer server) {
79+
// Error notifications are not supported for LSP-over-Legacy.
80+
server.showErrorMessageToUser(status.message!);
81+
}
7882
return success(null);
7983
}
8084

pkg/analysis_server/lib/src/lsp/handlers/commands/refactor_command_handler.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:analysis_server/lsp_protocol/protocol.dart';
6+
import 'package:analysis_server/src/analysis_server.dart';
67
import 'package:analysis_server/src/lsp/client_capabilities.dart';
78
import 'package:analysis_server/src/lsp/constants.dart';
89
import 'package:analysis_server/src/lsp/error_or.dart';
910
import 'package:analysis_server/src/lsp/handlers/commands/simple_edit_handler.dart';
1011
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
11-
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
1212
import 'package:analysis_server/src/lsp/mapping.dart';
1313
import 'package:analysis_server/src/lsp/progress.dart';
1414
import 'package:analysis_server/src/lsp/source_edits.dart';
@@ -19,8 +19,7 @@ import 'package:analyzer/dart/analysis/results.dart';
1919
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
2020

2121
/// A command handler for any of the commands used to implement refactorings.
22-
class RefactorCommandHandler
23-
extends SimpleEditCommandHandler<LspAnalysisServer> {
22+
class RefactorCommandHandler extends SimpleEditCommandHandler<AnalysisServer> {
2423
@override
2524
final String commandName;
2625

pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import 'package:analysis_server/src/lsp/registration/feature_registration.dart';
1414
typedef StaticOptions = Either2<bool, CodeActionOptions>;
1515

1616
class CodeActionHandler
17-
extends LspMessageHandler<CodeActionParams, TextDocumentCodeActionResult> {
17+
extends
18+
SharedMessageHandler<CodeActionParams, TextDocumentCodeActionResult> {
1819
CodeActionHandler(super.server);
1920

2021
@override
@@ -24,6 +25,9 @@ class CodeActionHandler
2425
LspJsonHandler<CodeActionParams> get jsonHandler =>
2526
CodeActionParams.jsonHandler;
2627

28+
@override
29+
bool get requiresTrustedCaller => false;
30+
2731
@override
2832
Future<ErrorOr<TextDocumentCodeActionResult>> handle(
2933
CodeActionParams params,

pkg/analysis_server/lib/src/lsp/handlers/handler_execute_command.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,18 @@ class ExecuteCommandHandler
4040
Commands.sendWorkspaceEdit: SendWorkspaceEditCommandHandler(server),
4141
Commands.logAction: LogActionCommandHandler(server),
4242
Commands.applyCodeAction: ApplyCodeActionCommandHandler(server),
43+
Commands.performRefactor: PerformRefactorCommandHandler(server),
44+
Commands.validateRefactor: ValidateRefactorCommandHandler(server),
45+
// Add commands for each of the refactorings.
46+
for (var entry in RefactoringProcessor.generators.entries)
47+
entry.key: RefactorCommandHandler(server, entry.key, entry.value),
4348

4449
// Commands that currently require an underlying LSP server.
4550
if (server is LspAnalysisServer) ...{
4651
Commands.fixAll: FixAllCommandHandler(server),
4752
Commands.fixAllInWorkspace: FixAllInWorkspaceCommandHandler(server),
4853
Commands.previewFixAllInWorkspace:
4954
PreviewFixAllInWorkspaceCommandHandler(server),
50-
Commands.performRefactor: PerformRefactorCommandHandler(server),
51-
Commands.validateRefactor: ValidateRefactorCommandHandler(server),
52-
// Add commands for each of the refactorings.
53-
for (var entry in RefactoringProcessor.generators.entries)
54-
entry.key: RefactorCommandHandler(server, entry.key, entry.value),
5555
},
5656
} {
5757
server.executeCommandHandler = this;

pkg/analysis_server/lib/src/lsp/handlers/handler_states.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ class InitializedLspStateMessageHandler extends InitializedStateMessageHandler {
9191
DefinitionHandler.new,
9292
DocumentLinkHandler.new,
9393
ReferencesHandler.new,
94-
CodeActionHandler.new,
9594
ChangeWorkspaceFoldersHandler.new,
9695
PrepareRenameHandler.new,
9796
RenameHandler.new,
@@ -123,6 +122,7 @@ class InitializedStateMessageHandler extends ServerStateMessageHandler {
123122
<_RequestHandlerGenerator<AnalysisServer>>[
124123
AugmentationHandler.new,
125124
AugmentedHandler.new,
125+
CodeActionHandler.new,
126126
CodeLensHandler.new,
127127
ConnectToDtdHandler.new,
128128
DartTextDocumentContentProviderHandler.new,
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import '../lsp/code_actions_mixin.dart';
6+
import 'abstract_lsp_over_legacy.dart';
7+
8+
class AbstractCodeActionsTest extends SharedLspOverLegacyTest
9+
with CodeActionsTestMixin {
10+
@override
11+
Future<void> initializeServer() async {
12+
await super.initializeServer();
13+
14+
// Most CodeActions tests set LSP capabilities so automatically send these
15+
// to the legacy server as part of initialization.
16+
await sendClientCapabilities();
17+
}
18+
}

pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,12 +422,22 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
422422
// TODO(dantup): Support this for LSP-over-Legacy shared tests.
423423
var failTestOnErrorDiagnostic = false;
424424

425+
/// The current set of priority files. This is the same as the set of
426+
/// currently-open files (see [openFile]/[closeFile]).
427+
final Set<String> _priorityFiles = {};
428+
425429
@override
426430
Future<void> get currentAnalysis => waitForTasksFinished();
427431

428432
@override
429433
Future<void> closeFile(Uri uri) async {
430-
await removeOverlay(fromUri(uri));
434+
// closeFile should both remove the overlay and remove from priority files,
435+
// since that's equivalent of what the LSP document handlers do and shared
436+
// tests need to have the same behaviour.
437+
var filePath = fromUri(uri);
438+
await removeOverlay(filePath);
439+
_priorityFiles.remove(filePath);
440+
_updatePriorityFiles();
431441
}
432442

433443
@override
@@ -442,7 +452,13 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
442452

443453
@override
444454
Future<void> openFile(Uri uri, String content, {int version = 1}) async {
445-
await addOverlay(fromUri(uri), content, version);
455+
// closeFile should both add an overlay and add to priority files,
456+
// since that's equivalent of what the LSP document handlers do and shared
457+
// tests need to have the same behaviour.
458+
var filePath = fromUri(uri);
459+
await addOverlay(filePath, content, version);
460+
_priorityFiles.add(filePath);
461+
_updatePriorityFiles();
446462
}
447463

448464
/// Opens a file content without providing a version number.
@@ -469,4 +485,8 @@ abstract class SharedLspOverLegacyTest extends LspOverLegacyTest
469485
// For legacy, we can use addOverlay to replace the whole file.
470486
await addOverlay(fromUri(uri), content);
471487
}
488+
489+
void _updatePriorityFiles() {
490+
setPriorityFiles(_priorityFiles.map(getFile).toList());
491+
}
472492
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import '../shared/shared_code_actions_assists_tests.dart';
8+
import 'abstract_code_actions.dart';
9+
10+
void main() {
11+
defineReflectiveSuite(() {
12+
defineReflectiveTests(CodeActionAssistsTest);
13+
});
14+
}
15+
16+
@reflectiveTest
17+
class CodeActionAssistsTest extends AbstractCodeActionsTest
18+
with
19+
// Tests are defined in a shared mixin.
20+
SharedAssistsCodeActionsTests {}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:test_reflective_loader/test_reflective_loader.dart';
6+
7+
import '../shared/shared_code_actions_fixes_tests.dart';
8+
import 'abstract_code_actions.dart';
9+
10+
void main() {
11+
defineReflectiveSuite(() {
12+
defineReflectiveTests(CodeActionFixesTest);
13+
});
14+
}
15+
16+
@reflectiveTest
17+
class CodeActionFixesTest extends AbstractCodeActionsTest
18+
with
19+
// Tests are defined in a shared mixin.
20+
SharedFixesCodeActionsTests {}

0 commit comments

Comments
 (0)