[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 <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/handlers/code_actions/abstract_code_actions_producer.dart

@@ -39,6 +39,14 @@ abstract class AbstractCodeActionsProducer
   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 @@ abstract class AbstractCodeActionsProducer
     required this.editorCapabilities,
     required this.allowCommands,
     required this.allowCodeActionLiterals,
+    required this.allowSnippets,
     required this.analysisOptions,
   });
 
@@ -139,7 +148,7 @@ abstract class AbstractCodeActionsProducer
         server,
         editorCapabilities,
         change,
-        allowSnippets: true,
+        allowSnippets: allowSnippets,
         filePath: path,
         lineInfo: lineInfo,
       ),

pkg/analysis_server/lib/src/lsp/handlers/code_actions/analysis_options.dart

@@ -31,6 +31,7 @@ class AnalysisOptionsCodeActionsProducer extends AbstractCodeActionsProducer {
     required super.allowCodeActionLiterals,
     required super.allowCommands,
     required super.analysisOptions,
+    required super.allowSnippets,
   });
 
   @override

pkg/analysis_server/lib/src/lsp/handlers/code_actions/code_action_computer.dart

@@ -46,6 +46,14 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
   /// 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 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
     required this.triggerKind,
     required this.allowCommands,
     required this.allowCodeActionLiterals,
+    required this.allowSnippets,
     required this.performance,
   });
 
@@ -237,8 +246,9 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
           callerCapabilities: callerCapabilities,
           allowCodeActionLiterals: allowCodeActionLiterals,
           allowCommands: allowCommands,
-          triggerKind: triggerKind,
+          allowSnippets: allowSnippets,
           analysisOptions: analysisOptions,
+          triggerKind: triggerKind,
           willBeDeduplicated: true,
         ),
       if (isPubspec)
@@ -254,6 +264,7 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
           callerCapabilities: callerCapabilities,
           allowCodeActionLiterals: allowCodeActionLiterals,
           allowCommands: allowCommands,
+          allowSnippets: allowSnippets,
           analysisOptions: analysisOptions,
         ),
       if (isAnalysisOptions)
@@ -269,6 +280,7 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
           callerCapabilities: callerCapabilities,
           allowCodeActionLiterals: allowCodeActionLiterals,
           allowCommands: allowCommands,
+          allowSnippets: allowSnippets,
           analysisOptions: analysisOptions,
         ),
       PluginCodeActionsProducer(
@@ -283,6 +295,7 @@ class CodeActionComputer with HandlerHelperMixin<AnalysisServer> {
         callerCapabilities: callerCapabilities,
         allowCodeActionLiterals: allowCodeActionLiterals,
         allowCommands: allowCommands,
+        allowSnippets: allowSnippets,
         analysisOptions: analysisOptions,
       ),
     ];

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

@@ -55,6 +55,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
     required super.callerCapabilities,
     required super.allowCodeActionLiterals,
     required super.allowCommands,
+    required super.allowSnippets,
     required super.analysisOptions,
     required this.triggerKind,
     required this.willBeDeduplicated,

pkg/analysis_server/lib/src/lsp/handlers/code_actions/plugins.dart

@@ -30,6 +30,7 @@ class PluginCodeActionsProducer extends AbstractCodeActionsProducer {
     required super.allowCodeActionLiterals,
     required super.allowCommands,
     required super.analysisOptions,
+    required super.allowSnippets,
   }) : driver = server.getAnalysisDriver(file.path);
 
   @override

pkg/analysis_server/lib/src/lsp/handlers/code_actions/pubspec.dart

@@ -28,6 +28,7 @@ class PubspecCodeActionsProducer extends AbstractCodeActionsProducer {
     required super.allowCodeActionLiterals,
     required super.allowCommands,
     required super.analysisOptions,
+    required super.allowSnippets,
   });
 
   @override

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

@@ -147,6 +147,10 @@ class ApplyCodeActionCommandHandler
       // 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();
 

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

@@ -57,6 +57,7 @@ class CodeActionHandler
       triggerKind: params.context.triggerKind,
       allowCommands: true,
       allowCodeActionLiterals: supportsLiterals,
+      allowSnippets: true, // We allow snippets from code actions requests.
       performance: performance,
     );
 

pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart

@@ -126,7 +126,7 @@ Widget build() {
 >>>>>>>>>> lib/test.dart
 import 'package:flutter/widgets.dart';
 Widget build() {
-  return Center($0child: Text(''));
+  return Center(child: Text(''));
 }
 ''';
 

pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart

@@ -701,6 +701,38 @@ name: $testPackageName
     );
   }
 
+  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 @@ useFunction(int g(a, b)) {}
     );
   }
 
+  /// 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();