Add code fixes to LSP code actions
Change-Id: I5ec0932945cc6215cb671f89e8bed21609894ba2
Reviewed-on: https://dart-review.googlesource.com/c/87066
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
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 cb8ecd3..8fe44b1 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
@@ -7,11 +7,18 @@
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:analysis_server/plugin/edit/fix/fix_core.dart';
import 'package:analysis_server/src/lsp/constants.dart';
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
import 'package:analysis_server/src/lsp/mapping.dart';
+import 'package:analysis_server/src/lsp/source_edits.dart';
+import 'package:analysis_server/src/protocol_server.dart' show SourceChange;
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/correction/fix_internal.dart';
import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/analysis/session.dart'
+ show InconsistentAnalysisException;
import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
typedef ActionHandler = Future<List<Either2<Command, CodeAction>>> Function(
@@ -22,20 +29,6 @@
CodeActionHandler(LspAnalysisServer server) : super(server);
Method get handlesMessage => Method.textDocument_codeAction;
- /// Wraps a command in a CodeAction if the client supports it so that a
- /// CodeActionKind can be supplied.
- Either2<Command, CodeAction> commandOrCodeAction(
- bool clientSupportsLiteralCodeActions,
- CodeActionKind kind,
- Command command,
- ) {
- return clientSupportsLiteralCodeActions
- ? Either2<Command, CodeAction>.t2(
- new CodeAction(command.title, kind, null, null, command),
- )
- : Either2<Command, CodeAction>.t1(command);
- }
-
@override
CodeActionParams convertParams(Map<String, dynamic> json) =>
CodeActionParams.fromJson(json);
@@ -60,6 +53,42 @@
unit));
}
+ /// Wraps a command in a CodeAction if the client supports it so that a
+ /// CodeActionKind can be supplied.
+ Either2<Command, CodeAction> _commandOrCodeAction(
+ bool clientSupportsLiteralCodeActions,
+ CodeActionKind kind,
+ Command command,
+ ) {
+ return clientSupportsLiteralCodeActions
+ ? Either2<Command, CodeAction>.t2(
+ new CodeAction(command.title, kind, null, null, command),
+ )
+ : Either2<Command, CodeAction>.t1(command);
+ }
+
+ Either2<Command, CodeAction> _createFixAction(
+ Fix fix, Diagnostic diagnostic) {
+ return new Either2<Command, CodeAction>.t2(new CodeAction(
+ fix.change.message,
+ CodeActionKind.QuickFix,
+ [diagnostic],
+ _createWorkspaceEdit(fix.change),
+ null,
+ ));
+ }
+
+ WorkspaceEdit _createWorkspaceEdit(SourceChange change) {
+ return toWorkspaceEdit(
+ server.clientCapabilities?.workspace,
+ change.edits
+ .map((e) => new FileEditInformation(
+ server.getVersionedDocumentIdentifier(e.file),
+ server.getLineInfo(e.file),
+ e.edits))
+ .toList());
+ }
+
Future<List<Either2<Command, CodeAction>>> _getAssistActions(
HashSet<CodeActionKind> clientSupportedCodeActionKinds,
bool clientSupportsLiteralCodeActions,
@@ -104,8 +133,40 @@
Range range,
ResolvedUnitResult unit,
) async {
- // TODO(dantup): Implement fixes.
- return [];
+ // TODO(dantup): Is it acceptable not to support these for clients that can't
+ // handle Code Action literals? (Doing so requires we encode this into a
+ // command/arguments set and allow the client to call us back later).
+ if (!clientSupportsLiteralCodeActions) {
+ return const [];
+ }
+ // Keep trying until we run without getting an `InconsistentAnalysisException`.
+ while (true) {
+ final lineInfo = unit.lineInfo;
+ final codeActions = <Either2<Command, CodeAction>>[];
+ final fixContributor = new DartFixContributor();
+ try {
+ for (final error in unit.errors) {
+ // Server lineNumber is one-based so subtract one.
+ int errorLine = lineInfo.getLocation(error.offset).lineNumber - 1;
+ if (errorLine >= range.start.line && errorLine <= range.end.line) {
+ var context = new DartFixContextImpl(unit, error);
+ final fixes = await fixContributor.computeFixes(context);
+ if (fixes.isNotEmpty) {
+ fixes.sort(Fix.SORT_BY_RELEVANCE);
+
+ final diagnostic = toDiagnostic(lineInfo, error);
+ codeActions.addAll(
+ fixes.map((fix) => _createFixAction(fix, diagnostic)),
+ );
+ }
+ }
+ }
+
+ return codeActions;
+ } on InconsistentAnalysisException {
+ // Loop around to try again to compute the fixes.
+ }
+ }
}
Future<List<Either2<Command, CodeAction>>> _getRefactorActions(
@@ -141,12 +202,12 @@
}
return [
- commandOrCodeAction(
+ _commandOrCodeAction(
clientSupportsLiteralCodeActions,
DartCodeActionKind.SortMembers,
new Command('Sort Members', Commands.sortMembers, [path]),
),
- commandOrCodeAction(
+ _commandOrCodeAction(
clientSupportsLiteralCodeActions,
CodeActionKind.SourceOrganizeImports,
new Command('Organize Imports', Commands.organizeImports, [path]),
diff --git a/pkg/analysis_server/test/lsp/code_actions_abstract.dart b/pkg/analysis_server/test/lsp/code_actions_abstract.dart
new file mode 100644
index 0000000..64aea35
--- /dev/null
+++ b/pkg/analysis_server/test/lsp/code_actions_abstract.dart
@@ -0,0 +1,70 @@
+// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
+import 'package:analysis_server/lsp_protocol/protocol_special.dart';
+import 'package:test/test.dart';
+
+import 'server_abstract.dart';
+
+abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
+ Future<void> checkCodeActionAvailable(
+ Uri uri,
+ String command,
+ String title, {
+ bool asCodeActionLiteral = false,
+ bool asCommand = false,
+ }) async {
+ final codeActions = await getCodeActions(uri.toString());
+ final codeAction = findCommand(codeActions, command);
+ expect(codeAction, isNotNull);
+
+ codeAction.map(
+ (command) {
+ if (!asCommand) {
+ throw 'Got Command but expected CodeAction literal';
+ }
+ expect(command.title, equals(title));
+ expect(command.arguments, equals([uri.toFilePath()]));
+ },
+ (codeAction) {
+ if (!asCodeActionLiteral) {
+ throw 'Got CodeAction literal but expected Command';
+ }
+ expect(codeAction, isNotNull);
+ expect(codeAction.title, equals(title));
+ expect(codeAction.command.title, equals(title));
+ expect(codeAction.command.arguments, equals([uri.toFilePath()]));
+ },
+ );
+ }
+
+ Either2<Command, CodeAction> findCommand(
+ List<Either2<Command, CodeAction>> actions, String commandID) {
+ for (var codeAction in actions) {
+ final id = codeAction.map(
+ (cmd) => cmd.command, (action) => action.command.command);
+ if (id == commandID) {
+ return codeAction;
+ }
+ }
+ return null;
+ }
+
+ CodeAction findEditAction(List<Either2<Command, CodeAction>> actions,
+ CodeActionKind actionKind, String title) {
+ for (var codeAction in actions) {
+ final codeActionLiteral =
+ codeAction.map((cmd) => null, (action) => action);
+ if (codeActionLiteral?.kind == actionKind &&
+ codeActionLiteral?.title == title) {
+ // We're specifically looking for an action that contains an edit.
+ assert(codeActionLiteral.command == null);
+ assert(codeActionLiteral.edit != null);
+ return codeActionLiteral;
+ }
+ }
+ return null;
+ }
+}
diff --git a/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart b/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart
new file mode 100644
index 0000000..2f824c3
--- /dev/null
+++ b/pkg/analysis_server/test/lsp/code_actions_fixes_test.dart
@@ -0,0 +1,96 @@
+// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
+import 'package:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'code_actions_abstract.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(FixesCodeActionsTest);
+ });
+}
+
+@reflectiveTest
+class FixesCodeActionsTest extends AbstractCodeActionsTest {
+ test_appliesCorrectEdits_withDocumentChangesSupport() async {
+ // This code should get a fix to remove the unused import.
+ const content = '''
+ import 'dart:async';
+ [[import]] 'dart:convert';
+
+ Future foo;
+ ''';
+
+ const expectedContent = '''
+ import 'dart:async';
+
+ Future foo;
+ ''';
+ await newFile(mainFilePath, content: withoutMarkers(content));
+ await initialize(
+ textDocumentCapabilities: withCodeActionKinds(
+ emptyTextDocumentClientCapabilities, [CodeActionKind.QuickFix]),
+ workspaceCapabilities:
+ withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
+ );
+
+ final codeActions = await getCodeActions(mainFileUri.toString(),
+ range: rangeFromMarkers(content));
+ final fixAction = findEditAction(
+ codeActions, CodeActionKind.QuickFix, 'Remove unused import');
+
+ // Ensure the edit came back, and using documentChanges.
+ expect(fixAction, isNotNull);
+ expect(fixAction.edit.documentChanges, isNotNull);
+ expect(fixAction.edit.changes, isNull);
+
+ // Ensure applying the changes will give us the expected content.
+ final contents = {
+ mainFilePath: content,
+ };
+ applyDocumentChanges(contents, fixAction.edit.documentChanges);
+ expect(contents[mainFilePath], equals(expectedContent));
+ }
+
+ test_appliesCorrectEdits_withoutDocumentChangesSupport() async {
+ // This code should get a fix to remove the unused import.
+ const content = '''
+ import 'dart:async';
+ [[import]] 'dart:convert';
+
+ Future foo;
+ ''';
+
+ const expectedContent = '''
+ import 'dart:async';
+
+ Future foo;
+ ''';
+ await newFile(mainFilePath, content: withoutMarkers(content));
+ await initialize(
+ textDocumentCapabilities: withCodeActionKinds(
+ emptyTextDocumentClientCapabilities, [CodeActionKind.QuickFix]),
+ );
+
+ final codeActions = await getCodeActions(mainFileUri.toString(),
+ range: rangeFromMarkers(content));
+ final fixAction = findEditAction(
+ codeActions, CodeActionKind.QuickFix, 'Remove unused import');
+
+ // Ensure the edit came back, and using changes.
+ expect(fixAction, isNotNull);
+ expect(fixAction.edit.changes, isNotNull);
+ expect(fixAction.edit.documentChanges, isNull);
+
+ // Ensure applying the changes will give us the expected content.
+ final contents = {
+ mainFilePath: content,
+ };
+ applyChanges(contents, fixAction.edit.changes);
+ expect(contents[mainFilePath], equals(expectedContent));
+ }
+}
diff --git a/pkg/analysis_server/test/lsp/code_actions_source_test.dart b/pkg/analysis_server/test/lsp/code_actions_source_test.dart
index 92e6671..e064771 100644
--- a/pkg/analysis_server/test/lsp/code_actions_source_test.dart
+++ b/pkg/analysis_server/test/lsp/code_actions_source_test.dart
@@ -3,13 +3,12 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
-import 'package:analysis_server/lsp_protocol/protocol_special.dart';
import 'package:analysis_server/src/lsp/constants.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import '../tool/lsp_spec/matchers.dart';
-import 'server_abstract.dart';
+import 'code_actions_abstract.dart';
main() {
defineReflectiveSuite(() {
@@ -18,67 +17,6 @@
});
}
-abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
- Future<void> checkCodeActionAvailable(
- Uri uri,
- String command,
- String title, {
- bool asCodeActionLiteral = false,
- bool asCommand = false,
- }) async {
- final codeActions = await getCodeActions(uri.toString());
- final codeAction = findCommand(codeActions, command);
- expect(codeAction, isNotNull);
-
- codeAction.map(
- (command) {
- if (!asCommand) {
- throw 'Got Command but expected CodeAction literal';
- }
- expect(command.title, equals(title));
- expect(command.arguments, equals([uri.toFilePath()]));
- },
- (codeAction) {
- if (!asCodeActionLiteral) {
- throw 'Got CodeAction literal but expected Command';
- }
- expect(codeAction, isNotNull);
- expect(codeAction.title, equals(title));
- expect(codeAction.command.title, equals(title));
- expect(codeAction.command.arguments, equals([uri.toFilePath()]));
- },
- );
- }
-
- Either2<Command, CodeAction> findCommand(
- List<Either2<Command, CodeAction>> actions, String commandID) {
- for (var codeAction in actions) {
- final id = codeAction.map(
- (cmd) => cmd.command, (action) => action.command.command);
- if (id == commandID) {
- return codeAction;
- }
- }
- return null;
- }
-
- CodeAction findEditAction(List<Either2<Command, CodeAction>> actions,
- CodeActionKind actionKind, String title) {
- for (var codeAction in actions) {
- final codeActionLiteral =
- codeAction.map((cmd) => null, (action) => action);
- if (codeActionLiteral?.kind == actionKind &&
- codeActionLiteral?.title == title) {
- // We're specifically looking for an action that contains an edit.
- assert(codeActionLiteral.command == null);
- assert(codeActionLiteral.edit != null);
- return codeActionLiteral;
- }
- }
- return null;
- }
-}
-
@reflectiveTest
class OrganizeImportsSourceCodeActionsTest extends AbstractCodeActionsTest {
test_appliesCorrectEdits_withDocumentChangesSupport() async {
diff --git a/pkg/analysis_server/test/lsp/test_all.dart b/pkg/analysis_server/test/lsp/test_all.dart
index 998542f..daa1b01 100644
--- a/pkg/analysis_server/test/lsp/test_all.dart
+++ b/pkg/analysis_server/test/lsp/test_all.dart
@@ -16,6 +16,7 @@
import 'server_test.dart' as server_test;
import 'signature_help_test.dart' as signature_help_test;
import 'code_actions_source_test.dart' as source_code_action_tests;
+import 'code_actions_fixes_test.dart' as fixes_code_action_tests;
main() {
defineReflectiveSuite(() {
@@ -31,5 +32,6 @@
server_test.main();
signature_help_test.main();
source_code_action_tests.main();
+ fixes_code_action_tests.main();
}, name: 'lsp');
}
