Skip to content

Commit b5cc437

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Fix missing code actions for pubspec+analysis_options
Although the server supported fixes in `pubspec.yaml` and `analysis_options.yaml`, they were not listed in the server capabilities, so VS Code would never actually request them. Fixes Dart-Code/Dart-Code#5781 Change-Id: Ie9520a3363902751e773f471ce2d0fc023c27a0f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459800 Reviewed-by: Samuel Rawlins <srawlins@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Samuel Rawlins <srawlins@google.com>
1 parent 3151714 commit b5cc437

File tree

2 files changed

+64
-14
lines changed

2 files changed

+64
-14
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class CodeActionRegistrations extends FeatureRegistration
7474

7575
@override
7676
ToJsonable? get options => CodeActionRegistrationOptions(
77-
documentSelector: fullySupportedTypes,
77+
documentSelector: supportedTypes,
7878
codeActionKinds: DartCodeActionKind.serverSupportedKinds,
7979
);
8080

@@ -94,6 +94,22 @@ class CodeActionRegistrations extends FeatureRegistration
9494
)
9595
: Either2.t1(true);
9696

97+
/// Types of documents for which code actions are provided.
98+
///
99+
/// Includes Dart files, plugin types, pubspec.yaml, and analysis_options.yaml.
100+
List<TextDocumentFilterScheme> get supportedTypes {
101+
return
102+
// Join in a Set because fullSupportedTypes includes plugin registrations
103+
// and might overlap.
104+
{
105+
...fullySupportedTypes,
106+
// We support code actions in pubspec+analysis_options even though they're
107+
// not "fully supported" (most handlers do not support them).
108+
pubspecFile,
109+
analysisOptionsFile,
110+
}.toList();
111+
}
112+
97113
@override
98114
bool get supportsDynamic => clientDynamic.codeActions;
99115
}

pkg/analysis_server/test/lsp/initialization_test.dart

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,25 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
3232
final allowOverlappingHandlersDefault =
3333
MessageScheduler.allowOverlappingHandlers;
3434

35+
var isAnalysisOptionsSelector = TypeMatcher<TextDocumentFilterScheme>()
36+
.having(
37+
(selector) => selector.pattern,
38+
'pattern',
39+
'**/analysis_options.yaml',
40+
);
41+
42+
var isDartSelector = TypeMatcher<TextDocumentFilterScheme>().having(
43+
(selector) => selector.language,
44+
'language',
45+
'dart',
46+
);
47+
48+
var isPubspecSelector = TypeMatcher<TextDocumentFilterScheme>().having(
49+
(selector) => selector.pattern,
50+
'pattern',
51+
'**/pubspec.yaml',
52+
);
53+
3554
/// Waits for any in-progress analysis context rebuild.
3655
///
3756
/// Pumps the event queue before and after, to ensure any server code that
@@ -416,13 +435,14 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
416435
Future<void> test_dynamicRegistration_containsAppropriateSettings() async {
417436
// Support file operations.
418437
setFileOperationDynamicRegistration();
419-
// Support dynamic registration for both text sync + hovers.
438+
// Support dynamic registration for text sync, hovers, and code actions.
420439
setTextSyncDynamicRegistration();
421440
setHoverDynamicRegistration();
441+
setTextDocumentDynamicRegistration('codeAction');
422442

423443
// Basic check that the server responds with the capabilities we'd expect,
424-
// for ex including analysis_options.yaml in text synchronization but not
425-
// for hovers.
444+
// for ex including analysis_options.yaml in text synchronization (and code
445+
// actions) but not for hovers.
426446
var registrations = <Registration>[];
427447
var initResponse = await monitorDynamicRegistrations(
428448
registrations,
@@ -440,8 +460,9 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
440460
expect(initResult.capabilities, isNotNull);
441461
expect(initResult.capabilities.textDocumentSync, isNull);
442462

443-
// Should contain Hover, DidOpen, DidClose, DidChange, WillRenameFiles.
444-
expect(registrations, hasLength(5));
463+
// Should contain Hover, DidOpen, DidClose, DidChange, CodeAction,
464+
// WillRenameFiles.
465+
expect(registrations, hasLength(6));
445466
var hover = registrationOptionsFor(
446467
registrations,
447468
Method.textDocument_hover,
@@ -450,6 +471,10 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
450471
registrations,
451472
Method.textDocument_didChange,
452473
);
474+
var codeAction = registrationOptionsFor(
475+
registrations,
476+
Method.textDocument_codeAction,
477+
);
453478
var rename = FileOperationRegistrationOptions.fromJson(
454479
registrationFor(
455480
registrations,
@@ -469,21 +494,30 @@ class InitializationTest extends AbstractLspAnalysisServerTest {
469494
// The hover capability should only specific Dart.
470495
expect(hover, isNotNull);
471496
expect(hover.documentSelector, hasLength(1));
472-
expect(hover.documentSelector!.single.language, equals('dart'));
497+
expect(hover.documentSelector!.single, isDartSelector);
473498

474499
// didChange should also include pubspec + analysis_options.
475500
expect(change, isNotNull);
476501
expect(change.documentSelector, hasLength(greaterThanOrEqualTo(3)));
477-
expect(change.documentSelector!.any((ds) => ds.language == 'dart'), isTrue);
478502
expect(
479-
change.documentSelector!.any((ds) => ds.pattern == '**/pubspec.yaml'),
480-
isTrue,
503+
change.documentSelector,
504+
containsAll([
505+
isDartSelector,
506+
isPubspecSelector,
507+
isAnalysisOptionsSelector,
508+
]),
481509
);
510+
511+
// codeAction should also include pubspec + analysis_options.
512+
expect(codeAction, isNotNull);
513+
expect(codeAction.documentSelector, hasLength(greaterThanOrEqualTo(3)));
482514
expect(
483-
change.documentSelector!.any(
484-
(ds) => ds.pattern == '**/analysis_options.yaml',
485-
),
486-
isTrue,
515+
change.documentSelector,
516+
containsAll([
517+
isDartSelector,
518+
isPubspecSelector,
519+
isAnalysisOptionsSelector,
520+
]),
487521
);
488522

489523
expect(rename, equals(fileOperationRegistrationOptions));

0 commit comments

Comments
 (0)