Skip to content

Commit 839831f

Browse files
snjezasnjeza
authored
Improvements to code action performance (#3322)
- React to cancellation requests while computing code actions - Avoid expensive calls for "Inline" & "Change Signature" refactorings - Ensure the active source file is updated when between multiple opened files Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
1 parent 30d99b1 commit 839831f

11 files changed

Lines changed: 355 additions & 99 deletions

File tree

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JDTUtils.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,11 +196,6 @@ public static ICompilationUnit resolveCompilationUnit(URI uri) {
196196

197197
IFile resource = (IFile) findResource(uri, ResourcesPlugin.getWorkspace().getRoot()::findFilesForLocationURI);
198198
if(resource != null) {
199-
for (ICompilationUnit workingCopy : JavaCore.getWorkingCopies(null)) {
200-
if (uri.equals(toURI(toURI(workingCopy)))) {
201-
return workingCopy;
202-
}
203-
}
204199
return resolveCompilationUnit(resource);
205200
} else {
206201
return getFakeCompilationUnit(uri, new NullProgressMonitor());
@@ -212,6 +207,11 @@ public static ICompilationUnit resolveCompilationUnit(IFile resource) {
212207
if(!ProjectUtils.isJavaProject(resource.getProject())){
213208
return null;
214209
}
210+
for (ICompilationUnit workingCopy : JavaCore.getWorkingCopies(null)) {
211+
if (resource.equals(workingCopy.getResource())) {
212+
return workingCopy;
213+
}
214+
}
215215
if (resource.getFileExtension() != null) {
216216
String name = resource.getName();
217217
if (org.eclipse.jdt.internal.core.util.Util.isJavaLikeFileName(name)) {

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java‎

Lines changed: 109 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Map;
2323

2424
import org.eclipse.core.runtime.CoreException;
25+
import org.eclipse.core.runtime.IProgressMonitor;
2526
import org.eclipse.core.runtime.NullProgressMonitor;
2627
import org.eclipse.jdt.core.ICompilationUnit;
2728
import org.eclipse.jdt.core.IField;
@@ -110,8 +111,10 @@
110111
import org.eclipse.jdt.ui.text.java.IProblemLocation;
111112
import org.eclipse.lsp4j.CodeActionKind;
112113
import org.eclipse.lsp4j.CodeActionParams;
114+
import org.eclipse.ltk.core.refactoring.Change;
113115
import org.eclipse.ltk.core.refactoring.CheckConditionsOperation;
114116
import org.eclipse.ltk.core.refactoring.CreateChangeOperation;
117+
import org.eclipse.ltk.core.refactoring.NullChange;
115118
import org.eclipse.ltk.core.refactoring.RefactoringStatus;
116119

117120
/**
@@ -127,38 +130,99 @@ public RefactorProcessor(PreferenceManager preferenceManager) {
127130
}
128131

129132
public List<ProposalKindWrapper> getProposals(CodeActionParams params, IInvocationContext context, IProblemLocation[] locations) throws CoreException {
133+
return getProposals(params, context, locations, new NullProgressMonitor());
134+
}
135+
136+
public List<ProposalKindWrapper> getProposals(CodeActionParams params, IInvocationContext context, IProblemLocation[] locations, IProgressMonitor monitor) throws CoreException {
130137
ASTNode coveringNode = context.getCoveringNode();
138+
if (monitor != null && monitor.isCanceled()) {
139+
return Collections.emptyList();
140+
}
131141
if (coveringNode != null) {
132142
ArrayList<ProposalKindWrapper> proposals = new ArrayList<>();
133143

134144
InvertBooleanUtility.getInverseConditionProposals(params, context, coveringNode, proposals);
145+
if (monitor != null && monitor.isCanceled()) {
146+
return Collections.emptyList();
147+
}
135148
getInverseLocalVariableProposals(params, context, coveringNode, proposals);
136-
149+
if (monitor != null && monitor.isCanceled()) {
150+
return Collections.emptyList();
151+
}
137152
getMoveRefactoringProposals(params, context, coveringNode, proposals);
138-
153+
if (monitor != null && monitor.isCanceled()) {
154+
return Collections.emptyList();
155+
}
139156
boolean noErrorsAtLocation = noErrorsAtLocation(locations, coveringNode);
140157
if (noErrorsAtLocation) {
141158
boolean problemsAtLocation = locations.length != 0;
142159
getExtractVariableProposal(params, context, problemsAtLocation, proposals);
160+
if (monitor != null && monitor.isCanceled()) {
161+
return Collections.emptyList();
162+
}
143163
getExtractMethodProposal(params, context, coveringNode, problemsAtLocation, proposals);
164+
if (monitor != null && monitor.isCanceled()) {
165+
return Collections.emptyList();
166+
}
144167
getExtractFieldProposal(params, context, problemsAtLocation, proposals);
145-
getInlineProposal(context, coveringNode, proposals);
146-
168+
if (monitor != null && monitor.isCanceled()) {
169+
return Collections.emptyList();
170+
}
147171
getConvertAnonymousToNestedProposals(params, context, coveringNode, proposals);
172+
if (monitor != null && monitor.isCanceled()) {
173+
return Collections.emptyList();
174+
}
148175
getConvertAnonymousClassCreationsToLambdaProposals(context, coveringNode, proposals);
176+
if (monitor != null && monitor.isCanceled()) {
177+
return Collections.emptyList();
178+
}
149179
getConvertLambdaToAnonymousClassCreationsProposals(context, coveringNode, proposals);
180+
if (monitor != null && monitor.isCanceled()) {
181+
return Collections.emptyList();
182+
}
150183

151184
getConvertVarTypeToResolvedTypeProposal(context, coveringNode, proposals);
185+
if (monitor != null && monitor.isCanceled()) {
186+
return Collections.emptyList();
187+
}
152188
getConvertResolvedTypeToVarTypeProposal(context, coveringNode, proposals);
189+
if (monitor != null && monitor.isCanceled()) {
190+
return Collections.emptyList();
191+
}
153192

154193
getAddStaticImportProposals(context, coveringNode, proposals);
194+
if (monitor != null && monitor.isCanceled()) {
195+
return Collections.emptyList();
196+
}
155197

156198
getConvertForLoopProposal(context, coveringNode, proposals);
199+
if (monitor != null && monitor.isCanceled()) {
200+
return Collections.emptyList();
201+
}
157202
getAssignToVariableProposals(context, coveringNode, locations, proposals, params);
158-
getIntroduceParameterProposals(params, context, coveringNode, locations, proposals);
203+
if (monitor != null && monitor.isCanceled()) {
204+
return Collections.emptyList();
205+
}
159206
getExtractInterfaceProposal(params, context, proposals);
160-
getChangeSignatureProposal(params, context, proposals);
207+
if (monitor != null && monitor.isCanceled()) {
208+
return Collections.emptyList();
209+
}
161210
getSurroundWithTryCatchProposal(context, proposals);
211+
if (monitor != null && monitor.isCanceled()) {
212+
return Collections.emptyList();
213+
}
214+
getChangeSignatureProposal(params, context, proposals);
215+
if (monitor != null && monitor.isCanceled()) {
216+
return Collections.emptyList();
217+
}
218+
getIntroduceParameterProposals(params, context, coveringNode, locations, proposals);
219+
if (monitor != null && monitor.isCanceled()) {
220+
return Collections.emptyList();
221+
}
222+
getInlineProposal(context, coveringNode, proposals);
223+
if (monitor != null && monitor.isCanceled()) {
224+
return Collections.emptyList();
225+
}
162226
}
163227
return proposals;
164228
}
@@ -334,15 +398,27 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
334398
// Inline Constant (static final field)
335399
if (RefactoringAvailabilityTesterCore.isInlineConstantAvailable((IField) varBinding.getJavaElement())) {
336400
InlineConstantRefactoring refactoring = new InlineConstantRefactoring(context.getCompilationUnit(), context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
337-
if (refactoring != null && refactoring.checkInitialConditions(new NullProgressMonitor()).isOK() && refactoring.getReferences(new NullProgressMonitor(), new RefactoringStatus()).length > 0) {
338-
refactoring.setRemoveDeclaration(refactoring.isDeclarationSelected());
339-
refactoring.setReplaceAllReferences(refactoring.isDeclarationSelected());
340-
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
341-
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
342-
create.run(new NullProgressMonitor());
401+
if (refactoring != null) {
343402
String label = ActionMessages.InlineConstantRefactoringAction_label;
344403
int relevance = IProposalRelevance.INLINE_LOCAL;
345-
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, create.getChange(), relevance);
404+
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, null /*create.getChange()*/, relevance) {
405+
@Override
406+
public Change getChange() throws CoreException {
407+
if (refactoring.checkInitialConditions(new NullProgressMonitor()).isOK() && refactoring.getReferences(new NullProgressMonitor(), new RefactoringStatus()).length > 0) {
408+
refactoring.setRemoveDeclaration(refactoring.isDeclarationSelected());
409+
refactoring.setReplaceAllReferences(refactoring.isDeclarationSelected());
410+
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
411+
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
412+
try {
413+
create.run(new NullProgressMonitor());
414+
return create.getChange();
415+
} catch (CoreException e) {
416+
JavaLanguageServerPlugin.log(e);
417+
}
418+
}
419+
return new NullChange();
420+
}
421+
};
346422
resultingCollections.add(CodeActionHandler.wrap(proposal, CodeActionKind.RefactorInline));
347423
return true;
348424
}
@@ -357,8 +433,10 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
357433
}
358434

359435
// Inline Local Variable
436+
// https://github.com/eclipse-jdtls/eclipse.jdt.ls/issues/3321
437+
// I haven't enhanced it because InlineVariableTest.testInlineLocalVariableWithNoReferences() fails; can be enhanced
360438
if (binding.getJavaElement() instanceof ILocalVariable localVar && RefactoringAvailabilityTesterCore.isInlineTempAvailable(localVar)) {
361-
InlineTempRefactoring refactoring= new InlineTempRefactoring((VariableDeclaration) decl);
439+
InlineTempRefactoring refactoring = new InlineTempRefactoring((VariableDeclaration) decl);
362440
boolean status;
363441
try {
364442
status = refactoring.checkAllConditions(new NullProgressMonitor()).isOK();
@@ -382,13 +460,25 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
382460
// Inline Method
383461
if (RefactoringAvailabilityTesterCore.isInlineMethodAvailable((IMethod) binding.getJavaElement())) {
384462
InlineMethodRefactoring refactoring = InlineMethodRefactoring.create(context.getCompilationUnit(), context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
385-
if (refactoring != null && refactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
386-
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
387-
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
388-
create.run(new NullProgressMonitor());
463+
if (refactoring != null) {
389464
String label = ActionMessages.InlineMethodRefactoringAction_label;
390465
int relevance = IProposalRelevance.INLINE_LOCAL;
391-
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, create.getChange(), relevance);
466+
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, null /*create.getChange()*/, relevance) {
467+
@Override
468+
public Change getChange() throws CoreException {
469+
if (refactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
470+
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
471+
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
472+
try {
473+
create.run(new NullProgressMonitor());
474+
return create.getChange();
475+
} catch (CoreException e) {
476+
JavaLanguageServerPlugin.log(e);
477+
}
478+
}
479+
return new NullChange();
480+
}
481+
};
392482
resultingCollections.add(CodeActionHandler.wrap(proposal, CodeActionKind.RefactorInline));
393483
return true;
394484
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,10 @@ public IStatus validateDocument(String uri, boolean debounce, IProgressMonitor m
258258
}
259259

260260
toValidate.add(unit);
261+
if (!unit.equals(sharedASTProvider.getActiveJavaElement())) {
262+
sharedASTProvider.disposeAST();
263+
}
264+
sharedASTProvider.setActiveJavaElement(unit);
261265
if (debounce && publishDiagnosticsJob != null) {
262266
publishDiagnosticsJob.cancel();
263267
publishDiagnosticsJob.setRule(null);

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureInfoHandler.java‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2016-2024 Red Hat Inc. and others.
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License 2.0
5+
* which accompanies this distribution, and is available at
6+
* https://www.eclipse.org/legal/epl-2.0/
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*
10+
* Contributors:
11+
* Red Hat Inc. - initial API and implementation
12+
*******************************************************************************/
13+
package org.eclipse.jdt.ls.core.internal.handlers;
14+
15+
import java.util.ArrayList;
16+
import java.util.List;
17+
18+
import org.eclipse.core.runtime.CoreException;
19+
import org.eclipse.core.runtime.IProgressMonitor;
20+
import org.eclipse.core.runtime.NullProgressMonitor;
21+
import org.eclipse.jdt.core.ICompilationUnit;
22+
import org.eclipse.jdt.core.IJavaElement;
23+
import org.eclipse.jdt.core.IMethod;
24+
import org.eclipse.jdt.core.dom.ASTNode;
25+
import org.eclipse.jdt.core.dom.CompilationUnit;
26+
import org.eclipse.jdt.core.dom.IMethodBinding;
27+
import org.eclipse.jdt.core.dom.MethodDeclaration;
28+
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
29+
import org.eclipse.jdt.internal.corext.refactoring.ExceptionInfo;
30+
import org.eclipse.jdt.internal.corext.refactoring.ParameterInfo;
31+
import org.eclipse.jdt.internal.corext.refactoring.RefactoringAvailabilityTesterCore;
32+
import org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor;
33+
import org.eclipse.jdt.internal.corext.util.JdtFlags;
34+
import org.eclipse.jdt.ls.core.internal.JDTUtils;
35+
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
36+
import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodException;
37+
import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodParameter;
38+
import org.eclipse.jdt.ls.core.internal.text.correction.ChangeSignatureInfo;
39+
import org.eclipse.jdt.ls.core.internal.text.correction.CodeActionUtility;
40+
import org.eclipse.jdt.ui.text.java.IInvocationContext;
41+
import org.eclipse.lsp4j.CodeActionParams;
42+
import org.eclipse.ltk.core.refactoring.RefactoringStatus;
43+
44+
public class ChangeSignatureInfoHandler {
45+
46+
private static final String CANNOT_CHANGE_SIGNATURE = "Cannot change signature.";
47+
48+
public static ChangeSignatureInfo getChangeSignatureInfo(CodeActionParams params, IProgressMonitor monitor) {
49+
if (monitor.isCanceled()) {
50+
return null;
51+
}
52+
final ICompilationUnit unit = JDTUtils.resolveCompilationUnit(params.getTextDocument().getUri());
53+
if (unit == null || monitor.isCanceled()) {
54+
return null;
55+
}
56+
CompilationUnit astRoot = CoreASTProvider.getInstance().getAST(unit, CoreASTProvider.WAIT_YES, monitor);
57+
if (astRoot == null) {
58+
return null;
59+
}
60+
IInvocationContext context = CodeActionHandler.getContext(unit, astRoot, params.getRange());
61+
ASTNode methodNode = CodeActionUtility.inferASTNode(context.getCoveringNode(), MethodDeclaration.class);
62+
if (methodNode == null) {
63+
return null;
64+
}
65+
IMethodBinding methodBinding = ((MethodDeclaration) methodNode).resolveBinding();
66+
if (methodBinding == null) {
67+
return null;
68+
}
69+
IJavaElement element = methodBinding.getJavaElement();
70+
if (element instanceof IMethod method) {
71+
try {
72+
ChangeSignatureProcessor processor = new ChangeSignatureProcessor(method);
73+
if (RefactoringAvailabilityTesterCore.isChangeSignatureAvailable(method)) {
74+
RefactoringStatus status = processor.checkInitialConditions(new NullProgressMonitor());
75+
if (status.isOK()) {
76+
List<MethodParameter> parameters = new ArrayList<>();
77+
for (ParameterInfo info : processor.getParameterInfos()) {
78+
parameters.add(new MethodParameter(info.getOldTypeName(), info.getOldName(), info.getDefaultValue() == null ? "null" : info.getDefaultValue(), info.getOldIndex()));
79+
}
80+
List<MethodException> exceptions = new ArrayList<>();
81+
for (ExceptionInfo info : processor.getExceptionInfos()) {
82+
exceptions.add(new MethodException(info.getFullyQualifiedName(), info.getElement().getHandleIdentifier()));
83+
}
84+
return new ChangeSignatureInfo(method.getHandleIdentifier(), JdtFlags.getVisibilityString(processor.getVisibility()), processor.getReturnTypeString(), method.getElementName(),
85+
parameters.toArray(MethodParameter[]::new), exceptions.toArray(MethodException[]::new));
86+
} else {
87+
return new ChangeSignatureInfo(CANNOT_CHANGE_SIGNATURE + status.getMessageMatchingSeverity(status.getSeverity()));
88+
}
89+
}
90+
} catch (CoreException e) {
91+
JavaLanguageServerPlugin.logException(e);
92+
}
93+
}
94+
return new ChangeSignatureInfo(CANNOT_CHANGE_SIGNATURE);
95+
}
96+
97+
}

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public List<Either<Command, CodeAction>> getCodeActionCommands(CodeActionParams
202202

203203
if (containsKind(codeActionKinds, CodeActionKind.Refactor)) {
204204
try {
205-
List<ProposalKindWrapper> refactorProposals = this.refactorProcessor.getProposals(params, context, locations);
205+
List<ProposalKindWrapper> refactorProposals = this.refactorProcessor.getProposals(params, context, locations, monitor);
206206
refactorProposals.sort(comparator);
207207
proposals.addAll(refactorProposals);
208208
} catch (CoreException e) {

‎org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GetRefactorEditHandler.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
import org.eclipse.lsp4j.WorkspaceEdit;
5757
import org.eclipse.ltk.core.refactoring.Change;
5858
import org.eclipse.ltk.core.refactoring.Refactoring;
59+
import org.eclipse.ltk.core.refactoring.RefactoringStatus;
60+
import org.eclipse.ltk.core.refactoring.RefactoringStatusEntry;
5961

6062
public class GetRefactorEditHandler {
6163
public static final String RENAME_COMMAND = "java.action.rename";
@@ -114,15 +116,26 @@ public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams pa
114116
proposal = RefactorProcessor.getConvertAnonymousToNestedProposal(params.context, context, context.getCoveringNode(), false);
115117
positionKey = "type_name";
116118
} else if (RefactorProposalUtility.INTRODUCE_PARAMETER_COMMAND.equals(params.command)) {
117-
// String initializeIn = (params.commandArguments != null && !params.commandArguments.isEmpty()) ? JSONUtility.toModel(params.commandArguments.get(0), String.class) : null;
118-
proposal = RefactorProposalUtility.getIntroduceParameterRefactoringProposals(params.context, context, context.getCoveringNode(), false, locations);
119-
positionKey = null;
120-
if (proposal.getProposal() instanceof RefactoringCorrectionProposalCore rcp) {
121-
IntroduceParameterRefactoring refactoring = (IntroduceParameterRefactoring) rcp.getRefactoring();
122-
ParameterInfo parameterInfo = refactoring.getAddedParameterInfo();
123-
if (parameterInfo != null) {
124-
positionKey = parameterInfo.getNewName();
119+
final IntroduceParameterRefactoring introduceParameterRefactoring = new IntroduceParameterRefactoring(context.getCompilationUnit(), context.getSelectionOffset(), context.getSelectionLength());
120+
LinkedProposalModelCore linkedProposalModel = new LinkedProposalModelCore();
121+
introduceParameterRefactoring.setLinkedProposalModel(linkedProposalModel);
122+
RefactoringStatus refactoringStatus = introduceParameterRefactoring.checkInitialConditions(new NullProgressMonitor());
123+
if (refactoringStatus.isOK()) {
124+
proposal = RefactorProposalUtility.getIntroduceParameterRefactoringProposals(params.context, context, context.getCoveringNode(), false, locations);
125+
positionKey = null;
126+
if (proposal.getProposal() instanceof RefactoringCorrectionProposalCore rcp) {
127+
IntroduceParameterRefactoring refactoring = (IntroduceParameterRefactoring) rcp.getRefactoring();
128+
ParameterInfo parameterInfo = refactoring.getAddedParameterInfo();
129+
if (parameterInfo != null) {
130+
positionKey = parameterInfo.getNewName();
131+
}
132+
}
133+
} else {
134+
StringBuilder buff = new StringBuilder();
135+
for (RefactoringStatusEntry refactoringStatusEntry : refactoringStatus.getEntries()) {
136+
buff.append(refactoringStatusEntry.toStatus().getMessage());
125137
}
138+
return new RefactorWorkspaceEdit(buff.toString());
126139
}
127140
} else if (RefactorProposalUtility.EXTRACT_INTERFACE_COMMAND.equals(params.command)) {
128141
if (params.commandArguments != null && params.commandArguments.size() == 3) {

0 commit comments

Comments
 (0)