Skip to content

Keep result of calls to codeAction in slots, so that they can be more safely resolved later. #9744

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.

Sign up for GitHub

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

Open
lahodaj wants to merge 1 commit into
base: master
Choose a base branch
from
+154 −10
Open

Keep result of calls to codeAction in slots, so that they can be more safely resolved later. #9744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 31 additions & 10 deletions ...lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/TextDocumentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ public class TextDocumentServiceImpl implements TextDocumentService, LanguageCli
private static final String NETBEANS_JAVA_COMPLETION_INSERT_TEXT_PARAMS = "completion-insert-text-parameters";// NOI18N
private static final String URL = "url";// NOI18N
private static final String INDEX = "index";// NOI18N
private static final String SLOT = "slot";// NOI18N

private static final RequestProcessor BACKGROUND_TASKS = new RequestProcessor(TextDocumentServiceImpl.class.getName(), 1, false, false);
private static final RequestProcessor WORKER = new RequestProcessor(TextDocumentServiceImpl.class.getName(), 1, false, false);
Expand Down Expand Up @@ -1083,13 +1084,12 @@ static DocumentSymbol structureElement2DocumentSymbol (StyledDocument doc, Struc
return ds;
}

private List<LazyCodeAction> lastCodeActions = null;
static final int MAX_CODE_ACTION_SLOTS = 2;
private final AtomicInteger codeActionsSlot = new AtomicInteger();
private final Map<Integer, List<LazyCodeAction>> slot2CodeActions = new LinkedHashMap<>();

@Override
public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActionParams params) {
lastCodeActions = new ArrayList<>();
AtomicInteger index = new AtomicInteger(0);

// shortcut: if the projects are not yet initialized, return empty:
if (server.openedProjects().getNow(null) == null) {
return CompletableFuture.completedFuture(Collections.emptyList());
Expand All @@ -1100,6 +1100,9 @@ public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActio
return CompletableFuture.completedFuture(Collections.emptyList());
}
StyledDocument doc = (StyledDocument)rawDoc;
int slot = codeActionsSlot.getAndIncrement();
AtomicInteger codeActionIndex = new AtomicInteger(0);
List<LazyCodeAction> codeActions = new ArrayList<>();

List<Either<Command, CodeAction>> result = new ArrayList<>();
Range range = params.getRange();
Expand Down Expand Up @@ -1168,12 +1171,13 @@ public CompletableFuture<List<Either<Command, CodeAction>>> codeAction(CodeActio

action.setCommand(new Command(inputAction.getCommand().getTitle(), Utils.encodeCommand(inputAction.getCommand().getCommand(), client.getNbCodeCapabilities()), commandParams));
}
if (inputAction instanceof LazyCodeAction && ((LazyCodeAction) inputAction).getLazyEdit() != null) {
lastCodeActions.add((LazyCodeAction) inputAction);
if (inputAction instanceof LazyCodeAction lazyInputAction && lazyInputAction.getLazyEdit() != null) {
Map<String, Object> data = new HashMap<>();
data.put(URL, uri);
data.put(INDEX, index.getAndIncrement());
data.put(SLOT, slot);
data.put(INDEX, codeActionIndex.getAndIncrement());
action.setData(data);
codeActions.add(lazyInputAction);
} else if (inputAction.getEdit() != null) {
action.setEdit(Utils.workspaceEditFromApi(inputAction.getEdit(), uri, client));
}
Expand Down Expand Up @@ -1222,7 +1226,18 @@ public void run(ResultIterator resultIterator) throws Exception {
resultFuture.complete(result);
}
});
return resultFuture;
return resultFuture.thenApply(actions -> {
synchronized (slot2CodeActions) {
slot2CodeActions.put(slot, codeActions);
while (slot2CodeActions.size() > MAX_CODE_ACTION_SLOTS) {
Iterator<Integer> it = slot2CodeActions.keySet().iterator();

it.next();
it.remove();
}
}
return actions;
});
}

@Override
Expand All @@ -1245,8 +1260,14 @@ public CompletableFuture<CodeAction> resolveCodeAction(CodeAction unresolved) {
return;
}
}
} else if (data.has(URL) && data.has(INDEX)) {
LazyCodeAction inputAction = lastCodeActions.get(data.getAsJsonPrimitive(INDEX).getAsInt());
} else if (data.has(URL) && data.has(SLOT) && data.has(INDEX)) {
List<LazyCodeAction> codeActions;
synchronized (slot2CodeActions) {
codeActions = slot2CodeActions.get(data.getAsJsonPrimitive(SLOT).getAsInt());
}
LazyCodeAction inputAction =
codeActions != null ? codeActions.get(data.getAsJsonPrimitive(INDEX).getAsInt())
: null;
if (inputAction != null) {
try {
unresolved.setEdit(Utils.workspaceEditFromApi(inputAction.getLazyEdit().get(), data.getAsJsonPrimitive(URL).getAsString(), client));
Expand Down
123 changes: 123 additions & 0 deletions ...va.lsp.server/test/unit/src/org/netbeans/modules/java/lsp/server/protocol/ServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6172,6 +6172,129 @@ public CompletableFuture<ApplyWorkspaceEditResponse> applyEdit(ApplyWorkspaceEdi
assertEquals(new HashSet<>(Arrays.asList(CodeActionKind.RefactorExtract)), presentKinds);
}

public void testCodeActionsResolveMismatch() throws Exception {
File src = new File(getWorkDir(), "Test.java");
src.getParentFile().mkdirs();
String code = "public class Test {\n" +
" public Hello list;\n" +
"}\n";
try (Writer w = new FileWriter(src)) {
w.write(code);
}

List<Diagnostic>[] diags = new List[1];
Launcher<LanguageServer> serverLauncher = createClientLauncherWithLogging(new LspClient() {
@Override
public void telemetryEvent(Object arg0) {
}

@Override
public void publishDiagnostics(PublishDiagnosticsParams params) {
synchronized (diags) {
diags[0] = params.getDiagnostics();
diags.notifyAll();
}
}

@Override
public void showMessage(MessageParams arg0) {
}

@Override
public CompletableFuture<MessageActionItem> showMessageRequest(ShowMessageRequestParams arg0) {
return CompletableFuture.completedFuture(new MessageActionItem(arg0.getActions().get(0).getTitle()));
}

@Override
public void logMessage(MessageParams arg0) {
throw new UnsupportedOperationException("Not supported yet.");
}

@Override
public CompletableFuture<ApplyWorkspaceEditResponse> applyEdit(ApplyWorkspaceEditParams params) {
throw new UnsupportedOperationException("Not supported yet.");
}

}, client.getInputStream(), client.getOutputStream());
serverLauncher.startListening();
LanguageServer server = serverLauncher.getRemoteProxy();
server.initialize(new InitializeParams()).get();
String uri = src.toURI().toString();
server.getTextDocumentService().didOpen(new DidOpenTextDocumentParams(new TextDocumentItem(uri, "java", 0, code)));
synchronized (diags) {
while (diags[0] == null) {
try {
diags.wait();
} catch (InterruptedException ex) {
}
}
}
VersionedTextDocumentIdentifier id = new VersionedTextDocumentIdentifier(src.toURI().toString(), 1);
//first verify that sequence codeActions - codeActions - resolveCodeAction(<action from the first call>) works sensibly:
{
List<Either<Command, CodeAction>> codeActions = server.getTextDocumentService().codeAction(new CodeActionParams(id, new Range(new Position(1, 14), new Position(1, 14)), new CodeActionContext(diags[0]))).get();
Optional<CodeAction> generateClass =
codeActions.stream()
.filter(Either::isRight)
.map(Either::getRight)
.filter(a -> "Create class \"Hello\" in Test".equals(a.getTitle()))
.findAny();
assertTrue(generateClass.isPresent());
CodeAction action = generateClass.get();
assertEquals(action.getKind(), CodeActionKind.QuickFix);
assertNull(action.getEdit());

//intervening code action computation:
List<Either<Command, CodeAction>> intervening = server.getTextDocumentService().codeAction(new CodeActionParams(id, new Range(new Position(0, 0), new Position(0, 0)), new CodeActionContext(List.of()))).get();
assertEquals(List.of(), intervening);

action = server.getTextDocumentService().resolveCodeAction(action).get();
assertNotNull(action.getEdit());
List<Either<TextDocumentEdit, ResourceOperation>> changes = action.getEdit().getDocumentChanges();
assertEquals(1, changes.size());
assertTrue(changes.get(0).isLeft());
TextDocumentEdit edit = changes.get(0).getLeft();
assertEquals(edit.getTextDocument().getUri(), uri);
List<TextEdit> fileChanges = edit.getEdits();
assertNotNull(fileChanges);
assertEquals(1, fileChanges.size());
assertEquals(new Range(new Position(2, 0),
new Position(2, 0)),
fileChanges.get(0).getRange());
assertEquals("\n" +
" private static class Hello {\n" +
"\n" +
" public Hello() {\n" +
" }\n" +
" }\n",
fileChanges.get(0).getNewText());
}
//verify that codeActions-codeActions-codeActions-resolveCodeAction(<action from the first call>) does not resolve, but does not crash:
{
List<Either<Command, CodeAction>> codeActions = server.getTextDocumentService().codeAction(new CodeActionParams(id, new Range(new Position(1, 14), new Position(1, 14)), new CodeActionContext(diags[0]))).get();
Optional<CodeAction> generateClass =
codeActions.stream()
.filter(Either::isRight)
.map(Either::getRight)
.filter(a -> "Create class \"Hello\" in Test".equals(a.getTitle()))
.findAny();
assertTrue(generateClass.isPresent());
CodeAction action = generateClass.get();
assertEquals(action.getKind(), CodeActionKind.QuickFix);
assertNull(action.getEdit());

//intervening code action computations:
for (int i = 0; i < TextDocumentServiceImpl.MAX_CODE_ACTION_SLOTS; i++) {
List<Either<Command, CodeAction>> intervening =
server.getTextDocumentService().codeAction(new CodeActionParams(id, new Range(new Position(0, 0), new Position(0, 0)), new CodeActionContext(List.of()))).get();
assertEquals(List.of(), intervening);
}

action = server.getTextDocumentService().resolveCodeAction(action).get();
assertNull(action.getEdit());
}
}

static {
System.setProperty("SourcePath.no.source.filter", "true");
JavacParser.DISABLE_SOURCE_LEVEL_DOWNGRADE = true;
Expand Down
Loading