-
Notifications
You must be signed in to change notification settings - Fork 8
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
API and implementation of codefixes and code actions for Rascal itself #478
Open
jurgenvinju
wants to merge
30
commits into
main
Choose a base branch
from
code-actions-for-rascal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
fac1420
scaffolded an initial implementation of codefixes and code actioons f…
jurgenvinju 05c9f9a
added first example code action for Rascal
jurgenvinju f60cff5
added default
jurgenvinju 62f627f
final fixes
jurgenvinju ea0991d
added sort imports action
jurgenvinju 46f76ea
fixed broken indentation
jurgenvinju 55f6845
improved asyncronous command handling
jurgenvinju f08530e
removed superfluous newlines in sorted imports and extends
jurgenvinju 0d0dcac
better grouping for imports and extends
jurgenvinju 6be8c16
syntax better spaced
jurgenvinju 0ab364f
added \'add missing license\' action for Rascal
jurgenvinju 4a83365
grammar rules have 1 line distance
jurgenvinju 0eea6fc
added UI test for Rascal code actions; factored out common assertLine…
jurgenvinju d799dc2
changed order of imports for testing purposes
jurgenvinju 1e074bc
use arrow keys to skip the other actions
jurgenvinju 17953b9
fixed typo
jurgenvinju 4eecdaa
Merge branch 'main' into code-actions-for-rascal
jurgenvinju 214dd88
reintroduced method lost in merge
jurgenvinju ed75af4
changed test because the action is not currently focused, it is the t…
jurgenvinju 31f88d8
bringing mo to the mountain, just test the first action instead of th…
jurgenvinju e853737
added LICENSE to test project for testing purposes of the code action…
jurgenvinju eec1297
added LICENSE to original files
jurgenvinju e021fa5
make sure to revert changes made by code actions before we continue t…
jurgenvinju 78250cd
factoring common functionality between Rascal LSP and Parametric DSL …
jurgenvinju f8b0702
acted on suggestions in review by @davylandman
jurgenvinju c07ba2e
fixed bug with command going to the wrong LSP server
jurgenvinju 58997ec
adding license action only appears on module name now
jurgenvinju 0644f00
factoring constants
jurgenvinju bbba2cb
fixed commands for Rascal itself
jurgenvinju d5362ac
fixed indentation
jurgenvinju File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,11 +29,9 @@ | |
import java.io.IOException; | ||
import java.io.Reader; | ||
import java.time.Duration; | ||
import java.util.Arrays; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.CompletionException; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
|
@@ -50,7 +48,6 @@ | |
import org.checkerframework.checker.nullness.qual.MonotonicNonNull; | ||
import org.checkerframework.checker.nullness.qual.Nullable; | ||
import org.eclipse.lsp4j.CodeAction; | ||
import org.eclipse.lsp4j.CodeActionKind; | ||
import org.eclipse.lsp4j.CodeActionParams; | ||
import org.eclipse.lsp4j.CodeLens; | ||
import org.eclipse.lsp4j.CodeLensOptions; | ||
|
@@ -90,7 +87,6 @@ | |
import org.eclipse.lsp4j.TextDocumentItem; | ||
import org.eclipse.lsp4j.TextDocumentSyncKind; | ||
import org.eclipse.lsp4j.VersionedTextDocumentIdentifier; | ||
import org.eclipse.lsp4j.WorkspaceEdit; | ||
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException; | ||
import org.eclipse.lsp4j.jsonrpc.messages.Either; | ||
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError; | ||
|
@@ -108,11 +104,10 @@ | |
import org.rascalmpl.vscode.lsp.TextDocumentState; | ||
import org.rascalmpl.vscode.lsp.parametric.model.ParametricFileFacts; | ||
import org.rascalmpl.vscode.lsp.parametric.model.ParametricSummary; | ||
import org.rascalmpl.vscode.lsp.parametric.model.RascalADTs; | ||
import org.rascalmpl.vscode.lsp.parametric.model.ParametricSummary.SummaryLookup; | ||
import org.rascalmpl.vscode.lsp.terminal.ITerminalIDEServer.LanguageParameter; | ||
import org.rascalmpl.vscode.lsp.util.CodeActions; | ||
import org.rascalmpl.vscode.lsp.util.Diagnostics; | ||
import org.rascalmpl.vscode.lsp.util.DocumentChanges; | ||
import org.rascalmpl.vscode.lsp.util.FoldingRanges; | ||
import org.rascalmpl.vscode.lsp.util.DocumentSymbols; | ||
import org.rascalmpl.vscode.lsp.util.SemanticTokenizer; | ||
|
@@ -123,16 +118,13 @@ | |
import org.rascalmpl.vscode.lsp.util.locations.Locations; | ||
import org.rascalmpl.vscode.lsp.util.locations.impl.TreeSearch; | ||
|
||
import com.google.gson.JsonPrimitive; | ||
|
||
import io.usethesource.vallang.IBool; | ||
import io.usethesource.vallang.IConstructor; | ||
import io.usethesource.vallang.IList; | ||
import io.usethesource.vallang.ISourceLocation; | ||
import io.usethesource.vallang.IString; | ||
import io.usethesource.vallang.ITuple; | ||
import io.usethesource.vallang.IValue; | ||
import io.usethesource.vallang.IWithKeywordParameters; | ||
import io.usethesource.vallang.exceptions.FactParseError; | ||
|
||
public class ParametricTextDocumentService implements IBaseTextDocumentService, LanguageClientAware { | ||
|
@@ -386,80 +378,10 @@ private CodeLens locCommandTupleToCodeLense(String languageName, IValue v) { | |
ISourceLocation loc = (ISourceLocation) t.get(0); | ||
IConstructor command = (IConstructor) t.get(1); | ||
|
||
return new CodeLens(Locations.toRange(loc, columns), constructorToCommand(languageName, command), null); | ||
} | ||
|
||
private CodeAction constructorToCodeAction(String languageName, IConstructor codeAction) { | ||
IWithKeywordParameters<?> kw = codeAction.asWithKeywordParameters(); | ||
IConstructor command = (IConstructor) kw.getParameter(RascalADTs.CodeActionFields.COMMAND); | ||
IString title = (IString) kw.getParameter(RascalADTs.CodeActionFields.TITLE); | ||
IList edits = (IList) kw.getParameter(RascalADTs.CodeActionFields.EDITS); | ||
IConstructor kind = (IConstructor) kw.getParameter(RascalADTs.CodeActionFields.KIND); | ||
|
||
// first deal with the defaults. Must mimick what's in util::LanguageServer with the `data CodeAction` declaration | ||
if (title == null) { | ||
if (command != null) { | ||
title = (IString) command.asWithKeywordParameters().getParameter(RascalADTs.CommandFields.TITLE); | ||
} | ||
|
||
if (title == null) { | ||
title = IRascalValueFactory.getInstance().string(""); | ||
} | ||
} | ||
|
||
CodeAction result = new CodeAction(title.getValue()); | ||
|
||
if (command != null) { | ||
result.setCommand(constructorToCommand(languageName, command)); | ||
} | ||
|
||
if (edits != null) { | ||
result.setEdit(new WorkspaceEdit(DocumentChanges.translateDocumentChanges(this, edits))); | ||
} | ||
|
||
result.setKind(constructorToCodeActionKind(kind)); | ||
|
||
return result; | ||
return new CodeLens(Locations.toRange(loc, columns), CodeActions.constructorToCommand(dedicatedLanguageName, languageName, command), null); | ||
} | ||
|
||
/** | ||
* Translates `refactor(inline())` to `"refactor.inline"` and `empty()` to `""`, etc. | ||
* `kind == null` signals absence of the optional parameter. This is factorede into | ||
* this private function because otherwise every call has to check it. | ||
*/ | ||
private String constructorToCodeActionKind(@Nullable IConstructor kind) { | ||
if (kind == null) { | ||
return CodeActionKind.QuickFix; | ||
} | ||
|
||
String name = kind.getName(); | ||
|
||
if (name.isEmpty()) { | ||
return ""; | ||
} | ||
else if (name.length() == 1) { | ||
return name.toUpperCase(); | ||
} | ||
else if ("empty".equals(name)) { | ||
return ""; | ||
} | ||
else { | ||
var kw = kind.asWithKeywordParameters(); | ||
for (String kwn : kw.getParameterNames()) { | ||
String nestedName = constructorToCodeActionKind((IConstructor) kw.getParameter(kwn)); | ||
name = name + (nestedName.isEmpty() ? "" : ("." + nestedName)); | ||
} | ||
} | ||
|
||
return name; | ||
} | ||
|
||
private Command constructorToCommand(String languageName, IConstructor command) { | ||
IWithKeywordParameters<?> kw = command.asWithKeywordParameters(); | ||
IString possibleTitle = (IString) kw.getParameter(RascalADTs.CommandFields.TITLE); | ||
|
||
return new Command(possibleTitle != null ? possibleTitle.getValue() : command.toString(), getRascalMetaCommandName(), Arrays.asList(languageName, command.toString())); | ||
} | ||
|
||
private void handleParsingErrors(TextDocumentState file) { | ||
handleParsingErrors(file, file.getCurrentTreeAsync()); | ||
|
@@ -584,34 +506,21 @@ public CompletableFuture<SemanticTokens> semanticTokensRange(SemanticTokensRange | |
|
||
@Override | ||
public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActionParams params) { | ||
logger.debug("codeActions: {}", params); | ||
logger.debug("codeAction: {}", params); | ||
|
||
final ILanguageContributions contribs = contributions(params.getTextDocument()); | ||
|
||
final var loc = Locations.toLoc(params.getTextDocument()); | ||
final var start = params.getRange().getStart(); | ||
// convert to Rascal 1-based line | ||
final var startLine = start.getLine() + 1; | ||
// convert to Rascal UTF-32 column width | ||
final var startColumn = columns.get(loc).translateInverseColumn(start.getLine(), start.getCharacter(), false); | ||
final var emptyListFuture = CompletableFuture.completedFuture(IRascalValueFactory.getInstance().list()); | ||
|
||
// first we make a future stream for filtering out the "fixes" that were optionally sent along with earlier diagnostics | ||
// and which came back with the codeAction's list of relevant (in scope) diagnostics: | ||
// CompletableFuture<Stream<IValue>> | ||
CompletableFuture<Stream<IValue>> quickfixes | ||
= params.getContext().getDiagnostics() | ||
.stream() | ||
.map(Diagnostic::getData) | ||
.filter(Objects::nonNull) | ||
.filter(JsonPrimitive.class::isInstance) | ||
.map(JsonPrimitive.class::cast) | ||
.map(JsonPrimitive::getAsString) | ||
// this is the "magic" resurrection of command terms from the JSON data field | ||
.map(contribs::parseCodeActions) | ||
// this serializes the stream of futures and accumulates their results as a flat list again | ||
.reduce(emptyListFuture, (acc, next) -> acc.thenCombine(next, IList::concat)) | ||
.thenApply(IList::stream) | ||
; | ||
var quickfixes = CodeActions.extractActionsFromDiagnostics(params, contribs::parseCodeActions); | ||
|
||
// here we dynamically ask the contributions for more actions, | ||
// based on the cursor position in the file and the current parse tree | ||
|
@@ -625,13 +534,7 @@ public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActio | |
; | ||
|
||
// final merging the two streams of commmands, and their conversion to LSP Command data-type | ||
return codeActions.thenCombine(quickfixes, (actions, quicks) -> | ||
Stream.concat(quicks, actions) | ||
.map(IConstructor.class::cast) | ||
.map(cons -> constructorToCodeAction(contribs.getName(), cons)) | ||
.map(cmd -> Either.<Command,CodeAction>forRight(cmd)) | ||
.collect(Collectors.toList()) | ||
); | ||
return CodeActions.mergeAndConvertCodeActions(this, dedicatedLanguageName, contribs.getName(), quickfixes, codeActions); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good that these got extracted out into a helper class. |
||
} | ||
|
||
private CompletableFuture<IList> computeCodeActions(final ILanguageContributions contribs, final int startLine, final int startColumn, ITree tree) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if for
RASCAL_COMMAND
we also want to do this? as this is about delegating to the sub-language, but that doesn't apply for the rascal commands.Shouldn't that be a separate branch? that only runs
documentService.execute
command directly? or are we always passing in the rascal-langauge name everytime?