Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Code action support #2021

Merged
merged 59 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
071a50f
Initial support for code actions on the LSP
MikaelMayer Apr 13, 2022
ca200df
Support for plugin system, default dummy quick fix to wrap errors wit…
MikaelMayer Apr 13, 2022
b6b0e95
Insert the failing error at the beginning of the block
MikaelMayer Apr 13, 2022
741174f
Insert failing postcondition at the end.
MikaelMayer Apr 14, 2022
2f51467
Refactored to add comments
MikaelMayer Apr 14, 2022
caf2971
More refactoring
MikaelMayer Apr 14, 2022
5495a40
First code action test working
MikaelMayer Apr 14, 2022
e14f3ed
Added code actions tests
MikaelMayer Apr 14, 2022
62b05c1
Added two tests to verify indentation is kept in the presence of tabs
MikaelMayer Apr 14, 2022
0526479
Verify that code actions work inside an if else statement
MikaelMayer Apr 14, 2022
8530409
Fixed warnings
MikaelMayer Apr 14, 2022
f6578d7
Make it work with anonymous files as well
MikaelMayer Apr 15, 2022
4fb9ac6
Merge branch 'master' into feat-code-actions
MikaelMayer May 2, 2022
dff29f3
Refactored verification quick fix into its own QuickFix class
MikaelMayer May 2, 2022
6e8b10b
WIP on quick fixer
MikaelMayer May 10, 2022
2213f74
Lazy computation of code actions
MikaelMayer May 11, 2022
0a4bbd7
Fixed the tests
MikaelMayer May 11, 2022
5c32824
Two more tests to ensure we have a default increasing indentation.
MikaelMayer May 11, 2022
6c24c17
Better support for simple cases
MikaelMayer May 11, 2022
1526720
Deep support for code action plugins + tests
MikaelMayer May 11, 2022
9c8da89
Fixed a crash that happens from time to time.
MikaelMayer May 11, 2022
459e67d
Merge branch 'master' into feat-code-actions
MikaelMayer May 11, 2022
d781d01
Fixed a test
MikaelMayer May 18, 2022
410e571
Merge branch 'feat-code-actions' of https://github.com/dafny-lang/daf…
MikaelMayer May 18, 2022
d263cbf
Merge branch 'master' into feat-code-actions
MikaelMayer May 18, 2022
1c6cb46
Fixed merge
MikaelMayer May 18, 2022
5240d50
Merge branch 'master' into feat-code-actions
MikaelMayer May 18, 2022
2aaea69
Separate QuickFixer from the other Dafny plugins
MikaelMayer May 25, 2022
815337f
Refactored classes
MikaelMayer May 25, 2022
4a4a99b
Updated doc
MikaelMayer May 25, 2022
b4306d7
More refactoring
MikaelMayer May 25, 2022
e015b0a
More refactoring
MikaelMayer May 25, 2022
b45e006
More refactorings
MikaelMayer May 25, 2022
e6aa3d2
Get latest document for code actions
MikaelMayer May 25, 2022
9e270af
More English fix
MikaelMayer May 26, 2022
e11951f
Merge branch 'master' into feat-code-actions
MikaelMayer May 27, 2022
0c57999
Review comments + tutorial
MikaelMayer May 27, 2022
6ba73f3
More review comments
MikaelMayer May 27, 2022
05c3a2f
Merge branch 'master' into feat-code-actions
MikaelMayer Jun 7, 2022
d9f32ce
Merge branch 'master' into feat-code-actions
MikaelMayer Jun 27, 2022
82bc0ec
Merge branch 'master' into feat-code-actions
MikaelMayer Jul 1, 2022
02cd82a
Apply suggestions from code review
MikaelMayer Jul 18, 2022
e8cb5bb
Fixed warnings
MikaelMayer Jul 18, 2022
4f48b93
IEnumerable instead of arrays.
MikaelMayer Aug 5, 2022
a5d767f
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 5, 2022
e9d574c
Fixed the merge commit.
MikaelMayer Aug 5, 2022
77e8085
Review 1
MikaelMayer Aug 9, 2022
29ab977
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 9, 2022
af8ac08
Review comments
MikaelMayer Aug 10, 2022
06421ea
Review comments
MikaelMayer Aug 10, 2022
c962e0e
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 10, 2022
274e314
Line check to avoid traversing all method.
MikaelMayer Aug 15, 2022
b8199cc
Fixed formatting warning
MikaelMayer Aug 25, 2022
92a029a
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 25, 2022
71a42f6
Fixed the newline problem on windows machines.
MikaelMayer Aug 26, 2022
bba15f6
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 26, 2022
44da9fe
Review comments
MikaelMayer Aug 26, 2022
d66b4d2
Update release notes
MikaelMayer Aug 26, 2022
5a1c545
Merge branch 'master' into feat-code-actions
MikaelMayer Aug 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Lazy computation of code actions
  • Loading branch information
MikaelMayer committed May 11, 2022
commit 2213f7440719d5aa91b6aa638f19ddd90b5aeb97
143 changes: 90 additions & 53 deletions Source/DafnyLanguageServer/Handlers/DafnyCodeActionHandler.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,53 @@
using Microsoft.Dafny.LanguageServer.Language;
using Microsoft.Dafny.LanguageServer.Language.Symbols;
using Microsoft.Dafny.LanguageServer.Util;
using System;
MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
using System.Collections.Concurrent;
using Microsoft.Dafny.LanguageServer.Language;
using Microsoft.Dafny.LanguageServer.Workspace;
using Microsoft.Extensions.Logging;
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using MediatR;
using Microsoft.Boogie;
using Microsoft.Dafny.Plugins;
using Newtonsoft.Json.Linq;
using OmniSharp.Extensions.LanguageServer.Protocol;
using OmniSharp.Extensions.LanguageServer.Protocol.Workspace;

namespace Microsoft.Dafny.LanguageServer.Handlers;

public class DafnyCodeActionHandler : CodeActionHandlerBase {
private readonly ILogger logger;
private readonly ILogger<DafnyCodeActionHandler> logger;
private readonly IDocumentDatabase documents;
private readonly QuickFixer[] quickFixers;
/*
class Test : CodeActionPartialHandlerBase {
public Test(Guid id, [NotNull] IProgressManager progressManager) : base(id, progressManager)
{
}

public Test([NotNull] IProgressManager progressManager) : base(progressManager)
{
}

protected override CodeActionRegistrationOptions CreateRegistrationOptions(CodeActionCapability capability,
ClientCapabilities clientCapabilities) {
throw new NotImplementedException();
}

protected override void Handle(CodeActionParams request, IObserver<IEnumerable<CommandOrCodeAction>> results, CancellationToken cancellationToken) {
throw new NotImplementedException();
}

public override Task<CodeAction> Handle(CodeAction request, CancellationToken cancellationToken) {
throw new NotImplementedException();
}
}*/

public DafnyCodeActionHandler(ILogger<DafnyCompletionHandler> logger, IDocumentDatabase documents, ISymbolGuesser symbolGuesser) {
public DafnyCodeActionHandler(ILogger<DafnyCodeActionHandler> logger, IDocumentDatabase documents, ISymbolGuesser symbolGuesser) {
this.logger = logger;
this.documents = documents;
this.quickFixers =
Expand Down Expand Up @@ -67,91 +93,100 @@ private class CodeActionProcessor {
this.documentUri = document.Uri.GetFileSystemPath();
}

public (string, WorkspaceEdit)[] ToWorkspaceEdit(params
(string, TextEdit[])[] edits) {
return edits.Select(titleEdit =>
(titleEdit.Item1, new WorkspaceEdit() {
DocumentChanges = new Container<WorkspaceEditDocumentChange>(
new WorkspaceEditDocumentChange(new TextDocumentEdit() {
TextDocument = new OptionalVersionedTextDocumentIdentifier() {
Uri = document.Uri,
Version = document.Version
},
Edits = new TextEditContainer(titleEdit.Item2)
})
)
})).ToArray();
}

public CommandOrCodeActionContainer GetCommandOrCodeActionContainer() {
var edits = GetPossibleFixes();
var workspaceEdit = ToWorkspaceEdit(edits);
var codeActions = workspaceEdit.Select(titleEdit => {
CommandOrCodeAction t = new CodeAction() {
Kind = CodeActionKind.QuickFix,
Title = titleEdit.Item1,
Edit = titleEdit.Item2
};
return t;
}
).ToArray();
return new CommandOrCodeActionContainer(codeActions);
}

/// <summary>
/// Returns the fixes as set up by plugins
/// </summary>
/// <param name="uri">The URI of the document, used as an unique key</param>
private IEnumerable<(string, TextEdit[])> GetPluginFixes() {
public IEnumerable<PluginQuickFix> GetPluginFixes() {
var ID = 0;
foreach (var fixer in fixers) {
// Maybe we could set the program only once, when resolved, instead of for every code action?
var fixerInput = new VerificationQuickFixerInput(document);
fixer.SetQuickFixInput(fixerInput.DocumentUri, fixerInput, CancellationToken.None);
var fixRange = request.Range.ToBoogieToken(documentText);
var quickFixes = fixer.GetQuickFixes(documentUri, fixRange);
var quickFixes = fixer.GetQuickFixes(fixerInput, fixRange);
var fixerCodeActions = quickFixes.Select(quickFix =>
CodeAction(quickFix.Title, quickFix.Edits));
new PluginQuickFix(quickFix, ID++));
foreach (var codeAction in fixerCodeActions) {
yield return codeAction;
}
}
}

/// <summary>
/// Returns a built-in list of possible code actions
/// Includes plugin-created code actions
/// </summary>
private (string, TextEdit[])[] GetPossibleFixes() {
var possibleFixes = new List<(string, TextEdit[])>() { };
possibleFixes.AddRange(GetPluginFixes());
return possibleFixes.ToArray();
}
}


public record PluginQuickFix(QuickFix QuickFix, int Id);

protected override CodeActionRegistrationOptions CreateRegistrationOptions(CodeActionCapability capability,
ClientCapabilities clientCapabilities) {
return new CodeActionRegistrationOptions {
DocumentSelector = DocumentSelector.ForLanguage("dafny"),
ResolveProvider = false,
ResolveProvider = true,
CodeActionKinds = Container<CodeActionKind>.From(
CodeActionKind.QuickFix
),
WorkDoneProgress = false
};
}

public ConcurrentDictionary<string, IReadOnlyList<PluginQuickFix>> ConcurrentDictionary = new();

public override async Task<CommandOrCodeActionContainer> Handle(CodeActionParams request, CancellationToken cancellationToken) {
var document = await documents.GetDocumentAsync(request.TextDocument);
if (document == null) {
logger.LogWarning("quick fixes requested for unloaded document {DocumentUri}", request.TextDocument.Uri);
return new CommandOrCodeActionContainer();
}
return new CodeActionProcessor(this.quickFixers, document, request, cancellationToken).GetCommandOrCodeActionContainer();
var pluginQuickFixes = new CodeActionProcessor(this.quickFixers, document, request, cancellationToken).GetPluginFixes().ToArray();

var documentUri = document.Uri.ToString();
ConcurrentDictionary.AddOrUpdate(documentUri,
_ => pluginQuickFixes, (_, _) => pluginQuickFixes);
var codeActions = pluginQuickFixes.Select(pluginQuickFix => {
CommandOrCodeAction t = new CodeAction {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention of the CodeAction API is to use the field command or edit to define what happens when a user triggers the action, so here you could have

Command = new Command(fixWithId.QuickFix.Title, "dafnyCodeAction", new JArray(documentUri, fixWithId.Id))

Instead of the Data field

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about what you mean.
The code that handles the code action is this one:

public override Task<CodeAction> Handle(CodeAction request..., which explicitly requires an unresolved code action. So I don't understand why I should send a command?

Copy link
Member

@keyboardDrummer keyboardDrummer Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm saying you could return a CodeAction that includes a Command instead of one that includes Data, since this seems how the API is intended to be used. From the documentation:

A CodeAction must set either edit and/or a command. If both are supplied, the edit is applied first, then the command is executed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the documentation of the API is incomplete. Although what you say works well theoretically, it has the side-effect that it creates an error in VSCode.
image
And that's what I thought: Code actions can either give edits on the text or more general actions.
However, the API documentation you describe don't talk about the construction of code actions which is two phases, which I've seen while browing the code online

  1. Creation of the code action with possible data attached
  2. Resolution of the code action when it's being clicked.

In 1) there is no need to create an edit or a command if all the edits and commands are going to be computed on 2.

As a conclusion, adding a "command" would require us to change the VSCode extension to support this command, which would have to manually invoke the code action resolver... which would be useless and hard to maintain
So I'm going to revert my changes there and just use the regular data field which is there for this purpose - the data between code action creation and code action resolution.

Copy link
Member

@keyboardDrummer keyboardDrummer Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a conclusion, adding a "command" would require us to change the VSCode extension to support this command, which would have to manually invoke the code action resolver... which would be useless and hard to maintain

That indeed seems bad.

OK, let's keep this as it is. Thanks for trying.

Title = pluginQuickFix.QuickFix.Title,
Data = new JArray(documentUri, pluginQuickFix.Id)
};
return t;
}
).ToArray();
return new CommandOrCodeActionContainer(codeActions);
}

public override Task<CodeAction> Handle(CodeAction request, CancellationToken cancellationToken) {
var command = request.Data;
string documentUri = command[0].Value<string>();
int id = command[1].Value<int>();
if (ConcurrentDictionary.TryGetValue(documentUri, out var quickFixes)) {
var selectedQuickFix = quickFixes.Where(pluginQuickFix => pluginQuickFix.Id == id).FirstOrDefault((PluginQuickFix)null!);
if (selectedQuickFix != null) {
return Task.FromResult(new CodeAction() {
Edit = new WorkspaceEdit() {
DocumentChanges = new Container<WorkspaceEditDocumentChange>(
new WorkspaceEditDocumentChange(new TextDocumentEdit() {
TextDocument = new OptionalVersionedTextDocumentIdentifier() {
Uri = documentUri
},
Edits = new TextEditContainer(GetTextEdits(selectedQuickFix.QuickFix.GetEdits()))
}))
}
});
}
}

return Task.FromResult(request);
}

private IEnumerable<TextEdit> GetTextEdits(QuickFixEdit[] quickFixEdits) {
var edits = new List<TextEdit>();
foreach (var (token, toReplace) in quickFixEdits) {
var range = token.GetLspRange();
edits.Add(new TextEdit() {
NewText = toReplace,
Range = range
});
}
return edits.ToArray();
}
}

internal class VerificationQuickFixerInput : IQuickFixInput {
Expand All @@ -160,6 +195,8 @@ internal class VerificationQuickFixerInput : IQuickFixInput {
Document = document;
}

public string Uri => Document.Uri.GetFileSystemPath();
public int Version => Document.Version;
public string Code => Document.Text.Text;
public Dafny.Program Program => Document.Program;
public DafnyDocument Document { get; }
Expand Down
22 changes: 9 additions & 13 deletions Source/DafnyLanguageServer/Language/VerificationQuickFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,17 @@

namespace Microsoft.Dafny.LanguageServer.Language;

class VerificationQuickFixer : CachedQuickFixer<VerificationQuickFixerInput> {
class VerificationQuickFixer : QuickFixer {
private readonly IDocumentDatabase documents;
private readonly ILogger<DafnyCompletionHandler> logger;
private readonly ILogger<DafnyCodeActionHandler> logger;

public VerificationQuickFixer(IDocumentDatabase documents, ILogger<DafnyCompletionHandler> logger) {
public VerificationQuickFixer(IDocumentDatabase documents, ILogger<DafnyCodeActionHandler> logger) {
this.documents = documents;
this.logger = logger;
}

public override VerificationQuickFixerInput
ComputeCache(IQuickFixInput input, CancellationToken cancellationToken) {
return (VerificationQuickFixerInput)input;
}

protected override QuickFix[] GetQuickFixes(VerificationQuickFixerInput cachedData, IToken selection) {
public override QuickFix[] GetQuickFixes(IQuickFixInput input, IToken selection) {
var cachedData = (VerificationQuickFixerInput)input;
string uri = cachedData.DocumentUri;
var document = cachedData.Document;
var diagnostics = cachedData.Diagnostics;
Expand All @@ -40,13 +36,13 @@ public override VerificationQuickFixerInput
var endToken = GetMatchingEndToken(document, uri, diagnostic.Range.Start.ToBoogieToken(cachedData.Code));
if (endToken != null) {
var (indentation, indentationBrace) = GetIndentationBefore(endToken, cachedData.Code);
result.Add(new QuickFix {
Title = "Explicit the failing assert",
Edits = new[] {
result.Add(new InstantQuickFix(
"Explicit the failing assert",
new[] {
new QuickFixEdit(
endToken.Start(), $"{indentation}assert {expression};\n{indentationBrace}")
}
});
));
}
}
}
Expand Down