[analysis_server] Suppress snippet support for Command CodeActions
We support a non-standard mechanism (https://github.com/rust-lang/rust-analyzer/blob/b35559a2460e7f0b2b79a7029db0c5d4e0acdb44/docs/dev/lsp-extensions.md#snippet-textedit) for having snippets on edits for Code Actions. This is only specified for edits returned from `codeAction` requests (and only implemented in Dart-Code for the same), so we should not return snippets when applying edits via `workspace/applyEdit` (eg. when code actions are returned as commands).
This prevents snippet markup being inserted literally into the editor if codeActions are executed over DTD (eg. from property editor).
Change-Id: I7882bffb8038671459e7487dc2e944020395f128
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429340
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart
index 50377bf..497671c 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart
@@ -39,6 +39,14 @@
final int length;
final bool Function(CodeActionKind?) shouldIncludeKind;
+ /// Whether non-standard LSP snippets are allowed in edits produced.
+ ///
+ /// This is usually true for the `textDocument/codeAction` request (because we
+ /// support it for [CodeActionLiteral]s) but `false` for the
+ /// [Commands.applyCodeAction] handler because it's not supported for
+ /// `workspace/applyEdit` reverse requests.
+ final bool allowSnippets;
+
/// The capabilities of the caller making the request for [CodeAction]s.
final LspClientCapabilities callerCapabilities;
@@ -78,6 +86,7 @@
required this.editorCapabilities,
required this.allowCommands,
required this.allowCodeActionLiterals,
+ required this.allowSnippets,
required this.analysisOptions,
});
@@ -139,7 +148,7 @@
server,
editorCapabilities,
change,
- allowSnippets: true,
+ allowSnippets: allowSnippets,
filePath: path,
lineInfo: lineInfo,
),
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart
index 17e2dc7..597c040 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart
@@ -31,6 +31,7 @@
required super.allowCodeActionLiterals,
required super.allowCommands,
required super.analysisOptions,
+ required super.allowSnippets,
});
@override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart
index 8368f1d..01126a3 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart
@@ -46,6 +46,14 @@
/// How the request was triggered.
final CodeActionTriggerKind? triggerKind;
+ /// Whether non-standard LSP snippets are allowed in edits produced.
+ ///
+ /// This is usually true for the `textDocument/codeAction` request (because we
+ /// support it for [CodeActionLiteral]s) but `false` for the
+ /// [Commands.applyCodeAction] handler because it's not supported for
+ /// `workspace/applyEdit` reverse requests.
+ final bool allowSnippets;
+
/// Whether [CodeAction]s can be [Command]s or not.
///
/// This is usually true (because there is no capability for this), however
@@ -92,6 +100,7 @@
required this.triggerKind,
required this.allowCommands,
required this.allowCodeActionLiterals,
+ required this.allowSnippets,
required this.performance,
});
@@ -237,8 +246,9 @@
callerCapabilities: callerCapabilities,
allowCodeActionLiterals: allowCodeActionLiterals,
allowCommands: allowCommands,
- triggerKind: triggerKind,
+ allowSnippets: allowSnippets,
analysisOptions: analysisOptions,
+ triggerKind: triggerKind,
willBeDeduplicated: true,
),
if (isPubspec)
@@ -254,6 +264,7 @@
callerCapabilities: callerCapabilities,
allowCodeActionLiterals: allowCodeActionLiterals,
allowCommands: allowCommands,
+ allowSnippets: allowSnippets,
analysisOptions: analysisOptions,
),
if (isAnalysisOptions)
@@ -269,6 +280,7 @@
callerCapabilities: callerCapabilities,
allowCodeActionLiterals: allowCodeActionLiterals,
allowCommands: allowCommands,
+ allowSnippets: allowSnippets,
analysisOptions: analysisOptions,
),
PluginCodeActionsProducer(
@@ -283,6 +295,7 @@
callerCapabilities: callerCapabilities,
allowCodeActionLiterals: allowCodeActionLiterals,
allowCommands: allowCommands,
+ allowSnippets: allowSnippets,
analysisOptions: analysisOptions,
),
];
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/dart.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/dart.dart
index 3e743a4..a502b23 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/dart.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/dart.dart
@@ -55,6 +55,7 @@
required super.callerCapabilities,
required super.allowCodeActionLiterals,
required super.allowCommands,
+ required super.allowSnippets,
required super.analysisOptions,
required this.triggerKind,
required this.willBeDeduplicated,
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/plugins.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/plugins.dart
index 54c51a9..8d83dd5 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/plugins.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/plugins.dart
@@ -30,6 +30,7 @@
required super.allowCodeActionLiterals,
required super.allowCommands,
required super.analysisOptions,
+ required super.allowSnippets,
}) : driver = server.getAnalysisDriver(file.path);
@override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart
index 5596c18..1c24c88 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart
@@ -28,6 +28,7 @@
required super.allowCodeActionLiterals,
required super.allowCommands,
required super.analysisOptions,
+ required super.allowSnippets,
});
@override
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/commands/apply_code_action.dart b/pkg/analysis_server/lib/src/lsp/handlers/commands/apply_code_action.dart
index d57ba3a..d0b3d59 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/commands/apply_code_action.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/commands/apply_code_action.dart
@@ -147,6 +147,10 @@
// ability to applyEdit has been checked at the top of this method.
allowCommands: false,
allowCodeActionLiterals: true,
+
+ // We don't support non-standard snippets for `workspace/applyEdit`, only
+ // CodeActionLiterals.
+ allowSnippets: false,
);
var actions = await computer.compute();
diff --git a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
index 0832908..e8b1295 100644
--- a/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
+++ b/pkg/analysis_server/lib/src/lsp/handlers/handler_code_actions.dart
@@ -57,6 +57,7 @@
triggerKind: params.context.triggerKind,
allowCommands: true,
allowCodeActionLiterals: supportsLiterals,
+ allowSnippets: true, // We allow snippets from code actions requests.
performance: performance,
);
diff --git a/pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart b/pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart
index 965fca4..dd235e0 100644
--- a/pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart
+++ b/pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart
@@ -126,7 +126,7 @@
>>>>>>>>>> lib/test.dart
import 'package:flutter/widgets.dart';
Widget build() {
- return Center($0child: Text(''));
+ return Center(child: Text(''));
}
''';
diff --git a/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart b/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart
index a3b0e3a..380dbdb 100644
--- a/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart
+++ b/pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart
@@ -701,6 +701,38 @@
);
}
+ Future<void> test_snippets() async {
+ setSnippetTextEditSupport();
+
+ const content = '''
+abstract class A {
+ void m();
+}
+
+class ^B extends A {}
+''';
+
+ const expectedContent = r'''
+abstract class A {
+ void m();
+}
+
+class B extends A {
+ @override
+ void m() {
+ // TODO: implement m$0
+ }
+}
+''';
+
+ await verifyCodeActionLiteralEdits(
+ content,
+ expectedContent,
+ kind: CodeActionKind('quickfix.create.missingOverrides'),
+ title: 'Create 1 missing override',
+ );
+ }
+
Future<void> test_snippets_createMethod_functionTypeNestedParameters() async {
const content = '''
class A {
@@ -788,6 +820,43 @@
);
}
+ /// The non-standard snippets we supported are only supported for
+ /// [CodeActionLiteral]s and not for [Command]s (which go via
+ /// workspace/applyEdit) so even if enabled, they should not be returned.
+ Future<void> test_snippets_unsupportedForCommands() async {
+ setSupportedCodeActionKinds(null); // no codeActionLiteralSupport
+ setSnippetTextEditSupport(); // will be ignored
+
+ const content = '''
+abstract class A {
+ void m();
+}
+
+class ^B extends A {}
+''';
+
+ // No $0 placeholder in this content (unlike in `test_snippets`).
+ const expectedContent = r'''
+abstract class A {
+ void m();
+}
+
+class B extends A {
+ @override
+ void m() {
+ // TODO: implement m
+ }
+}
+''';
+
+ await verifyCommandCodeActionEdits(
+ content,
+ expectedContent,
+ command: Commands.applyCodeAction,
+ title: 'Create 1 missing override',
+ );
+ }
+
void _enableLints(List<String> lintNames) {
registerLintRules();
var lintsYaml = lintNames.map((name) => ' - $name\n').join();
