Skip to content

Commit e399178

Browse files
DanTupcommit-bot@chromium.org
DanTup
authored and
[email protected]
committed
Add code fixes to LSP code actions
Change-Id: I5ec0932945cc6215cb671f89e8bed21609894ba2 Reviewed-on: https://dart-review.googlesource.com/c/87066 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Danny Tuppeny <[email protected]>
1 parent 7d4a6cc commit e399178

File tree

5 files changed

+248
-81
lines changed

5 files changed

+248
-81
lines changed

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

Lines changed: 79 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,18 @@ import 'dart:collection';
77

88
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
99
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
10+
import 'package:analysis_server/plugin/edit/fix/fix_core.dart';
1011
import 'package:analysis_server/src/lsp/constants.dart';
1112
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
1213
import 'package:analysis_server/src/lsp/lsp_analysis_server.dart';
1314
import 'package:analysis_server/src/lsp/mapping.dart';
15+
import 'package:analysis_server/src/lsp/source_edits.dart';
16+
import 'package:analysis_server/src/protocol_server.dart' show SourceChange;
17+
import 'package:analysis_server/src/services/correction/fix.dart';
18+
import 'package:analysis_server/src/services/correction/fix_internal.dart';
1419
import 'package:analyzer/dart/analysis/results.dart';
20+
import 'package:analyzer/dart/analysis/session.dart'
21+
show InconsistentAnalysisException;
1522
import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
1623

1724
typedef ActionHandler = Future<List<Either2<Command, CodeAction>>> Function(
@@ -22,20 +29,6 @@ class CodeActionHandler extends MessageHandler<CodeActionParams,
2229
CodeActionHandler(LspAnalysisServer server) : super(server);
2330
Method get handlesMessage => Method.textDocument_codeAction;
2431

25-
/// Wraps a command in a CodeAction if the client supports it so that a
26-
/// CodeActionKind can be supplied.
27-
Either2<Command, CodeAction> commandOrCodeAction(
28-
bool clientSupportsLiteralCodeActions,
29-
CodeActionKind kind,
30-
Command command,
31-
) {
32-
return clientSupportsLiteralCodeActions
33-
? Either2<Command, CodeAction>.t2(
34-
new CodeAction(command.title, kind, null, null, command),
35-
)
36-
: Either2<Command, CodeAction>.t1(command);
37-
}
38-
3932
@override
4033
CodeActionParams convertParams(Map<String, dynamic> json) =>
4134
CodeActionParams.fromJson(json);
@@ -60,6 +53,42 @@ class CodeActionHandler extends MessageHandler<CodeActionParams,
6053
unit));
6154
}
6255

56+
/// Wraps a command in a CodeAction if the client supports it so that a
57+
/// CodeActionKind can be supplied.
58+
Either2<Command, CodeAction> _commandOrCodeAction(
59+
bool clientSupportsLiteralCodeActions,
60+
CodeActionKind kind,
61+
Command command,
62+
) {
63+
return clientSupportsLiteralCodeActions
64+
? Either2<Command, CodeAction>.t2(
65+
new CodeAction(command.title, kind, null, null, command),
66+
)
67+
: Either2<Command, CodeAction>.t1(command);
68+
}
69+
70+
Either2<Command, CodeAction> _createFixAction(
71+
Fix fix, Diagnostic diagnostic) {
72+
return new Either2<Command, CodeAction>.t2(new CodeAction(
73+
fix.change.message,
74+
CodeActionKind.QuickFix,
75+
[diagnostic],
76+
_createWorkspaceEdit(fix.change),
77+
null,
78+
));
79+
}
80+
81+
WorkspaceEdit _createWorkspaceEdit(SourceChange change) {
82+
return toWorkspaceEdit(
83+
server.clientCapabilities?.workspace,
84+
change.edits
85+
.map((e) => new FileEditInformation(
86+
server.getVersionedDocumentIdentifier(e.file),
87+
server.getLineInfo(e.file),
88+
e.edits))
89+
.toList());
90+
}
91+
6392
Future<List<Either2<Command, CodeAction>>> _getAssistActions(
6493
HashSet<CodeActionKind> clientSupportedCodeActionKinds,
6594
bool clientSupportsLiteralCodeActions,
@@ -104,8 +133,40 @@ class CodeActionHandler extends MessageHandler<CodeActionParams,
104133
Range range,
105134
ResolvedUnitResult unit,
106135
) async {
107-
// TODO(dantup): Implement fixes.
108-
return [];
136+
// TODO(dantup): Is it acceptable not to support these for clients that can't
137+
// handle Code Action literals? (Doing so requires we encode this into a
138+
// command/arguments set and allow the client to call us back later).
139+
if (!clientSupportsLiteralCodeActions) {
140+
return const [];
141+
}
142+
// Keep trying until we run without getting an `InconsistentAnalysisException`.
143+
while (true) {
144+
final lineInfo = unit.lineInfo;
145+
final codeActions = <Either2<Command, CodeAction>>[];
146+
final fixContributor = new DartFixContributor();
147+
try {
148+
for (final error in unit.errors) {
149+
// Server lineNumber is one-based so subtract one.
150+
int errorLine = lineInfo.getLocation(error.offset).lineNumber - 1;
151+
if (errorLine >= range.start.line && errorLine <= range.end.line) {
152+
var context = new DartFixContextImpl(unit, error);
153+
final fixes = await fixContributor.computeFixes(context);
154+
if (fixes.isNotEmpty) {
155+
fixes.sort(Fix.SORT_BY_RELEVANCE);
156+
157+
final diagnostic = toDiagnostic(lineInfo, error);
158+
codeActions.addAll(
159+
fixes.map((fix) => _createFixAction(fix, diagnostic)),
160+
);
161+
}
162+
}
163+
}
164+
165+
return codeActions;
166+
} on InconsistentAnalysisException {
167+
// Loop around to try again to compute the fixes.
168+
}
169+
}
109170
}
110171

111172
Future<List<Either2<Command, CodeAction>>> _getRefactorActions(
@@ -141,12 +202,12 @@ class CodeActionHandler extends MessageHandler<CodeActionParams,
141202
}
142203

143204
return [
144-
commandOrCodeAction(
205+
_commandOrCodeAction(
145206
clientSupportsLiteralCodeActions,
146207
DartCodeActionKind.SortMembers,
147208
new Command('Sort Members', Commands.sortMembers, [path]),
148209
),
149-
commandOrCodeAction(
210+
_commandOrCodeAction(
150211
clientSupportsLiteralCodeActions,
151212
CodeActionKind.SourceOrganizeImports,
152213
new Command('Organize Imports', Commands.organizeImports, [path]),

‎pkg/analysis_server/test/lsp/code_actions_abstract.dart

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright (c) 2018, 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:analysis_server/lsp_protocol/protocol_generated.dart';
6+
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
7+
import 'package:test/test.dart';
8+
9+
import 'server_abstract.dart';
10+
11+
abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
12+
Future<void> checkCodeActionAvailable(
13+
Uri uri,
14+
String command,
15+
String title, {
16+
bool asCodeActionLiteral = false,
17+
bool asCommand = false,
18+
}) async {
19+
final codeActions = await getCodeActions(uri.toString());
20+
final codeAction = findCommand(codeActions, command);
21+
expect(codeAction, isNotNull);
22+
23+
codeAction.map(
24+
(command) {
25+
if (!asCommand) {
26+
throw 'Got Command but expected CodeAction literal';
27+
}
28+
expect(command.title, equals(title));
29+
expect(command.arguments, equals([uri.toFilePath()]));
30+
},
31+
(codeAction) {
32+
if (!asCodeActionLiteral) {
33+
throw 'Got CodeAction literal but expected Command';
34+
}
35+
expect(codeAction, isNotNull);
36+
expect(codeAction.title, equals(title));
37+
expect(codeAction.command.title, equals(title));
38+
expect(codeAction.command.arguments, equals([uri.toFilePath()]));
39+
},
40+
);
41+
}
42+
43+
Either2<Command, CodeAction> findCommand(
44+
List<Either2<Command, CodeAction>> actions, String commandID) {
45+
for (var codeAction in actions) {
46+
final id = codeAction.map(
47+
(cmd) => cmd.command, (action) => action.command.command);
48+
if (id == commandID) {
49+
return codeAction;
50+
}
51+
}
52+
return null;
53+
}
54+
55+
CodeAction findEditAction(List<Either2<Command, CodeAction>> actions,
56+
CodeActionKind actionKind, String title) {
57+
for (var codeAction in actions) {
58+
final codeActionLiteral =
59+
codeAction.map((cmd) => null, (action) => action);
60+
if (codeActionLiteral?.kind == actionKind &&
61+
codeActionLiteral?.title == title) {
62+
// We're specifically looking for an action that contains an edit.
63+
assert(codeActionLiteral.command == null);
64+
assert(codeActionLiteral.edit != null);
65+
return codeActionLiteral;
66+
}
67+
}
68+
return null;
69+
}
70+
}

‎pkg/analysis_server/test/lsp/code_actions_fixes_test.dart

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// Copyright (c) 2018, 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:analysis_server/lsp_protocol/protocol_generated.dart';
6+
import 'package:test/test.dart';
7+
import 'package:test_reflective_loader/test_reflective_loader.dart';
8+
9+
import 'code_actions_abstract.dart';
10+
11+
main() {
12+
defineReflectiveSuite(() {
13+
defineReflectiveTests(FixesCodeActionsTest);
14+
});
15+
}
16+
17+
@reflectiveTest
18+
class FixesCodeActionsTest extends AbstractCodeActionsTest {
19+
test_appliesCorrectEdits_withDocumentChangesSupport() async {
20+
// This code should get a fix to remove the unused import.
21+
const content = '''
22+
import 'dart:async';
23+
[[import]] 'dart:convert';
24+
25+
Future foo;
26+
''';
27+
28+
const expectedContent = '''
29+
import 'dart:async';
30+
31+
Future foo;
32+
''';
33+
await newFile(mainFilePath, content: withoutMarkers(content));
34+
await initialize(
35+
textDocumentCapabilities: withCodeActionKinds(
36+
emptyTextDocumentClientCapabilities, [CodeActionKind.QuickFix]),
37+
workspaceCapabilities:
38+
withDocumentChangesSupport(emptyWorkspaceClientCapabilities),
39+
);
40+
41+
final codeActions = await getCodeActions(mainFileUri.toString(),
42+
range: rangeFromMarkers(content));
43+
final fixAction = findEditAction(
44+
codeActions, CodeActionKind.QuickFix, 'Remove unused import');
45+
46+
// Ensure the edit came back, and using documentChanges.
47+
expect(fixAction, isNotNull);
48+
expect(fixAction.edit.documentChanges, isNotNull);
49+
expect(fixAction.edit.changes, isNull);
50+
51+
// Ensure applying the changes will give us the expected content.
52+
final contents = {
53+
mainFilePath: content,
54+
};
55+
applyDocumentChanges(contents, fixAction.edit.documentChanges);
56+
expect(contents[mainFilePath], equals(expectedContent));
57+
}
58+
59+
test_appliesCorrectEdits_withoutDocumentChangesSupport() async {
60+
// This code should get a fix to remove the unused import.
61+
const content = '''
62+
import 'dart:async';
63+
[[import]] 'dart:convert';
64+
65+
Future foo;
66+
''';
67+
68+
const expectedContent = '''
69+
import 'dart:async';
70+
71+
Future foo;
72+
''';
73+
await newFile(mainFilePath, content: withoutMarkers(content));
74+
await initialize(
75+
textDocumentCapabilities: withCodeActionKinds(
76+
emptyTextDocumentClientCapabilities, [CodeActionKind.QuickFix]),
77+
);
78+
79+
final codeActions = await getCodeActions(mainFileUri.toString(),
80+
range: rangeFromMarkers(content));
81+
final fixAction = findEditAction(
82+
codeActions, CodeActionKind.QuickFix, 'Remove unused import');
83+
84+
// Ensure the edit came back, and using changes.
85+
expect(fixAction, isNotNull);
86+
expect(fixAction.edit.changes, isNotNull);
87+
expect(fixAction.edit.documentChanges, isNull);
88+
89+
// Ensure applying the changes will give us the expected content.
90+
final contents = {
91+
mainFilePath: content,
92+
};
93+
applyChanges(contents, fixAction.edit.changes);
94+
expect(contents[mainFilePath], equals(expectedContent));
95+
}
96+
}

‎pkg/analysis_server/test/lsp/code_actions_source_test.dart

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

55
import 'package:analysis_server/lsp_protocol/protocol_generated.dart';
6-
import 'package:analysis_server/lsp_protocol/protocol_special.dart';
76
import 'package:analysis_server/src/lsp/constants.dart';
87
import 'package:test/test.dart';
98
import 'package:test_reflective_loader/test_reflective_loader.dart';
109

1110
import '../tool/lsp_spec/matchers.dart';
12-
import 'server_abstract.dart';
11+
import 'code_actions_abstract.dart';
1312

1413
main() {
1514
defineReflectiveSuite(() {
@@ -18,67 +17,6 @@ main() {
1817
});
1918
}
2019

21-
abstract class AbstractCodeActionsTest extends AbstractLspAnalysisServerTest {
22-
Future<void> checkCodeActionAvailable(
23-
Uri uri,
24-
String command,
25-
String title, {
26-
bool asCodeActionLiteral = false,
27-
bool asCommand = false,
28-
}) async {
29-
final codeActions = await getCodeActions(uri.toString());
30-
final codeAction = findCommand(codeActions, command);
31-
expect(codeAction, isNotNull);
32-
33-
codeAction.map(
34-
(command) {
35-
if (!asCommand) {
36-
throw 'Got Command but expected CodeAction literal';
37-
}
38-
expect(command.title, equals(title));
39-
expect(command.arguments, equals([uri.toFilePath()]));
40-
},
41-
(codeAction) {
42-
if (!asCodeActionLiteral) {
43-
throw 'Got CodeAction literal but expected Command';
44-
}
45-
expect(codeAction, isNotNull);
46-
expect(codeAction.title, equals(title));
47-
expect(codeAction.command.title, equals(title));
48-
expect(codeAction.command.arguments, equals([uri.toFilePath()]));
49-
},
50-
);
51-
}
52-
53-
Either2<Command, CodeAction> findCommand(
54-
List<Either2<Command, CodeAction>> actions, String commandID) {
55-
for (var codeAction in actions) {
56-
final id = codeAction.map(
57-
(cmd) => cmd.command, (action) => action.command.command);
58-
if (id == commandID) {
59-
return codeAction;
60-
}
61-
}
62-
return null;
63-
}
64-
65-
CodeAction findEditAction(List<Either2<Command, CodeAction>> actions,
66-
CodeActionKind actionKind, String title) {
67-
for (var codeAction in actions) {
68-
final codeActionLiteral =
69-
codeAction.map((cmd) => null, (action) => action);
70-
if (codeActionLiteral?.kind == actionKind &&
71-
codeActionLiteral?.title == title) {
72-
// We're specifically looking for an action that contains an edit.
73-
assert(codeActionLiteral.command == null);
74-
assert(codeActionLiteral.edit != null);
75-
return codeActionLiteral;
76-
}
77-
}
78-
return null;
79-
}
80-
}
81-
8220
@reflectiveTest
8321
class OrganizeImportsSourceCodeActionsTest extends AbstractCodeActionsTest {
8422
test_appliesCorrectEdits_withDocumentChangesSupport() async {

0 commit comments

Comments
 (0)