Skip to content

Commit

Permalink
Improvements to code action performance (#3322)
Browse files Browse the repository at this point in the history
- 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 <[email protected]>
  • Loading branch information
snjeza authored Nov 19, 2024
1 parent 30d99b1 commit 839831f
Show file tree
Hide file tree
Showing 11 changed files with 355 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,6 @@ public static ICompilationUnit resolveCompilationUnit(URI uri) {

IFile resource = (IFile) findResource(uri, ResourcesPlugin.getWorkspace().getRoot()::findFilesForLocationURI);
if(resource != null) {
for (ICompilationUnit workingCopy : JavaCore.getWorkingCopies(null)) {
if (uri.equals(toURI(toURI(workingCopy)))) {
return workingCopy;
}
}
return resolveCompilationUnit(resource);
} else {
return getFakeCompilationUnit(uri, new NullProgressMonitor());
Expand All @@ -212,6 +207,11 @@ public static ICompilationUnit resolveCompilationUnit(IFile resource) {
if(!ProjectUtils.isJavaProject(resource.getProject())){
return null;
}
for (ICompilationUnit workingCopy : JavaCore.getWorkingCopies(null)) {
if (resource.equals(workingCopy.getResource())) {
return workingCopy;
}
}
if (resource.getFileExtension() != null) {
String name = resource.getName();
if (org.eclipse.jdt.internal.core.util.Util.isJavaLikeFileName(name)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Map;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IField;
Expand Down Expand Up @@ -110,8 +111,10 @@
import org.eclipse.jdt.ui.text.java.IProblemLocation;
import org.eclipse.lsp4j.CodeActionKind;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.ltk.core.refactoring.Change;
import org.eclipse.ltk.core.refactoring.CheckConditionsOperation;
import org.eclipse.ltk.core.refactoring.CreateChangeOperation;
import org.eclipse.ltk.core.refactoring.NullChange;
import org.eclipse.ltk.core.refactoring.RefactoringStatus;

/**
Expand All @@ -127,38 +130,99 @@ public RefactorProcessor(PreferenceManager preferenceManager) {
}

public List<ProposalKindWrapper> getProposals(CodeActionParams params, IInvocationContext context, IProblemLocation[] locations) throws CoreException {
return getProposals(params, context, locations, new NullProgressMonitor());
}

public List<ProposalKindWrapper> getProposals(CodeActionParams params, IInvocationContext context, IProblemLocation[] locations, IProgressMonitor monitor) throws CoreException {
ASTNode coveringNode = context.getCoveringNode();
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
if (coveringNode != null) {
ArrayList<ProposalKindWrapper> proposals = new ArrayList<>();

InvertBooleanUtility.getInverseConditionProposals(params, context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getInverseLocalVariableProposals(params, context, coveringNode, proposals);

if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getMoveRefactoringProposals(params, context, coveringNode, proposals);

if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
boolean noErrorsAtLocation = noErrorsAtLocation(locations, coveringNode);
if (noErrorsAtLocation) {
boolean problemsAtLocation = locations.length != 0;
getExtractVariableProposal(params, context, problemsAtLocation, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getExtractMethodProposal(params, context, coveringNode, problemsAtLocation, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getExtractFieldProposal(params, context, problemsAtLocation, proposals);
getInlineProposal(context, coveringNode, proposals);

if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getConvertAnonymousToNestedProposals(params, context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getConvertAnonymousClassCreationsToLambdaProposals(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getConvertLambdaToAnonymousClassCreationsProposals(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}

getConvertVarTypeToResolvedTypeProposal(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getConvertResolvedTypeToVarTypeProposal(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}

getAddStaticImportProposals(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}

getConvertForLoopProposal(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getAssignToVariableProposals(context, coveringNode, locations, proposals, params);
getIntroduceParameterProposals(params, context, coveringNode, locations, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getExtractInterfaceProposal(params, context, proposals);
getChangeSignatureProposal(params, context, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getSurroundWithTryCatchProposal(context, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getChangeSignatureProposal(params, context, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getIntroduceParameterProposals(params, context, coveringNode, locations, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
getInlineProposal(context, coveringNode, proposals);
if (monitor != null && monitor.isCanceled()) {
return Collections.emptyList();
}
}
return proposals;
}
Expand Down Expand Up @@ -334,15 +398,27 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
// Inline Constant (static final field)
if (RefactoringAvailabilityTesterCore.isInlineConstantAvailable((IField) varBinding.getJavaElement())) {
InlineConstantRefactoring refactoring = new InlineConstantRefactoring(context.getCompilationUnit(), context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
if (refactoring != null && refactoring.checkInitialConditions(new NullProgressMonitor()).isOK() && refactoring.getReferences(new NullProgressMonitor(), new RefactoringStatus()).length > 0) {
refactoring.setRemoveDeclaration(refactoring.isDeclarationSelected());
refactoring.setReplaceAllReferences(refactoring.isDeclarationSelected());
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
create.run(new NullProgressMonitor());
if (refactoring != null) {
String label = ActionMessages.InlineConstantRefactoringAction_label;
int relevance = IProposalRelevance.INLINE_LOCAL;
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, create.getChange(), relevance);
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, null /*create.getChange()*/, relevance) {
@Override
public Change getChange() throws CoreException {
if (refactoring.checkInitialConditions(new NullProgressMonitor()).isOK() && refactoring.getReferences(new NullProgressMonitor(), new RefactoringStatus()).length > 0) {
refactoring.setRemoveDeclaration(refactoring.isDeclarationSelected());
refactoring.setReplaceAllReferences(refactoring.isDeclarationSelected());
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
try {
create.run(new NullProgressMonitor());
return create.getChange();
} catch (CoreException e) {
JavaLanguageServerPlugin.log(e);
}
}
return new NullChange();
}
};
resultingCollections.add(CodeActionHandler.wrap(proposal, CodeActionKind.RefactorInline));
return true;
}
Expand All @@ -357,8 +433,10 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
}

// Inline Local Variable
// https://github.com/eclipse-jdtls/eclipse.jdt.ls/issues/3321
// I haven't enhanced it because InlineVariableTest.testInlineLocalVariableWithNoReferences() fails; can be enhanced
if (binding.getJavaElement() instanceof ILocalVariable localVar && RefactoringAvailabilityTesterCore.isInlineTempAvailable(localVar)) {
InlineTempRefactoring refactoring= new InlineTempRefactoring((VariableDeclaration) decl);
InlineTempRefactoring refactoring = new InlineTempRefactoring((VariableDeclaration) decl);
boolean status;
try {
status = refactoring.checkAllConditions(new NullProgressMonitor()).isOK();
Expand All @@ -382,13 +460,25 @@ private boolean getInlineProposal(IInvocationContext context, ASTNode node, Coll
// Inline Method
if (RefactoringAvailabilityTesterCore.isInlineMethodAvailable((IMethod) binding.getJavaElement())) {
InlineMethodRefactoring refactoring = InlineMethodRefactoring.create(context.getCompilationUnit(), context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
if (refactoring != null && refactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
create.run(new NullProgressMonitor());
if (refactoring != null) {
String label = ActionMessages.InlineMethodRefactoringAction_label;
int relevance = IProposalRelevance.INLINE_LOCAL;
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, create.getChange(), relevance);
ChangeCorrectionProposalCore proposal = new ChangeCorrectionProposalCore(label, null /*create.getChange()*/, relevance) {
@Override
public Change getChange() throws CoreException {
if (refactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
CheckConditionsOperation check = new CheckConditionsOperation(refactoring, CheckConditionsOperation.FINAL_CONDITIONS);
final CreateChangeOperation create = new CreateChangeOperation(check, RefactoringStatus.FATAL);
try {
create.run(new NullProgressMonitor());
return create.getChange();
} catch (CoreException e) {
JavaLanguageServerPlugin.log(e);
}
}
return new NullChange();
}
};
resultingCollections.add(CodeActionHandler.wrap(proposal, CodeActionKind.RefactorInline));
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,10 @@ public IStatus validateDocument(String uri, boolean debounce, IProgressMonitor m
}

toValidate.add(unit);
if (!unit.equals(sharedASTProvider.getActiveJavaElement())) {
sharedASTProvider.disposeAST();
}
sharedASTProvider.setActiveJavaElement(unit);
if (debounce && publishDiagnosticsJob != null) {
publishDiagnosticsJob.cancel();
publishDiagnosticsJob.setRule(null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*******************************************************************************
* Copyright (c) 2016-2024 Red Hat Inc. and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat Inc. - initial API and implementation
*******************************************************************************/
package org.eclipse.jdt.ls.core.internal.handlers;

import java.util.ArrayList;
import java.util.List;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IMethod;
import org.eclipse.jdt.core.dom.ASTNode;
import org.eclipse.jdt.core.dom.CompilationUnit;
import org.eclipse.jdt.core.dom.IMethodBinding;
import org.eclipse.jdt.core.dom.MethodDeclaration;
import org.eclipse.jdt.core.manipulation.CoreASTProvider;
import org.eclipse.jdt.internal.corext.refactoring.ExceptionInfo;
import org.eclipse.jdt.internal.corext.refactoring.ParameterInfo;
import org.eclipse.jdt.internal.corext.refactoring.RefactoringAvailabilityTesterCore;
import org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor;
import org.eclipse.jdt.internal.corext.util.JdtFlags;
import org.eclipse.jdt.ls.core.internal.JDTUtils;
import org.eclipse.jdt.ls.core.internal.JavaLanguageServerPlugin;
import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodException;
import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodParameter;
import org.eclipse.jdt.ls.core.internal.text.correction.ChangeSignatureInfo;
import org.eclipse.jdt.ls.core.internal.text.correction.CodeActionUtility;
import org.eclipse.jdt.ui.text.java.IInvocationContext;
import org.eclipse.lsp4j.CodeActionParams;
import org.eclipse.ltk.core.refactoring.RefactoringStatus;

public class ChangeSignatureInfoHandler {

private static final String CANNOT_CHANGE_SIGNATURE = "Cannot change signature.";

public static ChangeSignatureInfo getChangeSignatureInfo(CodeActionParams params, IProgressMonitor monitor) {
if (monitor.isCanceled()) {
return null;
}
final ICompilationUnit unit = JDTUtils.resolveCompilationUnit(params.getTextDocument().getUri());
if (unit == null || monitor.isCanceled()) {
return null;
}
CompilationUnit astRoot = CoreASTProvider.getInstance().getAST(unit, CoreASTProvider.WAIT_YES, monitor);
if (astRoot == null) {
return null;
}
IInvocationContext context = CodeActionHandler.getContext(unit, astRoot, params.getRange());
ASTNode methodNode = CodeActionUtility.inferASTNode(context.getCoveringNode(), MethodDeclaration.class);
if (methodNode == null) {
return null;
}
IMethodBinding methodBinding = ((MethodDeclaration) methodNode).resolveBinding();
if (methodBinding == null) {
return null;
}
IJavaElement element = methodBinding.getJavaElement();
if (element instanceof IMethod method) {
try {
ChangeSignatureProcessor processor = new ChangeSignatureProcessor(method);
if (RefactoringAvailabilityTesterCore.isChangeSignatureAvailable(method)) {
RefactoringStatus status = processor.checkInitialConditions(new NullProgressMonitor());
if (status.isOK()) {
List<MethodParameter> parameters = new ArrayList<>();
for (ParameterInfo info : processor.getParameterInfos()) {
parameters.add(new MethodParameter(info.getOldTypeName(), info.getOldName(), info.getDefaultValue() == null ? "null" : info.getDefaultValue(), info.getOldIndex()));
}
List<MethodException> exceptions = new ArrayList<>();
for (ExceptionInfo info : processor.getExceptionInfos()) {
exceptions.add(new MethodException(info.getFullyQualifiedName(), info.getElement().getHandleIdentifier()));
}
return new ChangeSignatureInfo(method.getHandleIdentifier(), JdtFlags.getVisibilityString(processor.getVisibility()), processor.getReturnTypeString(), method.getElementName(),
parameters.toArray(MethodParameter[]::new), exceptions.toArray(MethodException[]::new));
} else {
return new ChangeSignatureInfo(CANNOT_CHANGE_SIGNATURE + status.getMessageMatchingSeverity(status.getSeverity()));
}
}
} catch (CoreException e) {
JavaLanguageServerPlugin.logException(e);
}
}
return new ChangeSignatureInfo(CANNOT_CHANGE_SIGNATURE);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public List<Either<Command, CodeAction>> getCodeActionCommands(CodeActionParams

if (containsKind(codeActionKinds, CodeActionKind.Refactor)) {
try {
List<ProposalKindWrapper> refactorProposals = this.refactorProcessor.getProposals(params, context, locations);
List<ProposalKindWrapper> refactorProposals = this.refactorProcessor.getProposals(params, context, locations, monitor);
refactorProposals.sort(comparator);
proposals.addAll(refactorProposals);
} catch (CoreException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
import org.eclipse.lsp4j.WorkspaceEdit;
import org.eclipse.ltk.core.refactoring.Change;
import org.eclipse.ltk.core.refactoring.Refactoring;
import org.eclipse.ltk.core.refactoring.RefactoringStatus;
import org.eclipse.ltk.core.refactoring.RefactoringStatusEntry;

public class GetRefactorEditHandler {
public static final String RENAME_COMMAND = "java.action.rename";
Expand Down Expand Up @@ -114,15 +116,26 @@ public static RefactorWorkspaceEdit getEditsForRefactor(GetRefactorEditParams pa
proposal = RefactorProcessor.getConvertAnonymousToNestedProposal(params.context, context, context.getCoveringNode(), false);
positionKey = "type_name";
} else if (RefactorProposalUtility.INTRODUCE_PARAMETER_COMMAND.equals(params.command)) {
// String initializeIn = (params.commandArguments != null && !params.commandArguments.isEmpty()) ? JSONUtility.toModel(params.commandArguments.get(0), String.class) : null;
proposal = RefactorProposalUtility.getIntroduceParameterRefactoringProposals(params.context, context, context.getCoveringNode(), false, locations);
positionKey = null;
if (proposal.getProposal() instanceof RefactoringCorrectionProposalCore rcp) {
IntroduceParameterRefactoring refactoring = (IntroduceParameterRefactoring) rcp.getRefactoring();
ParameterInfo parameterInfo = refactoring.getAddedParameterInfo();
if (parameterInfo != null) {
positionKey = parameterInfo.getNewName();
final IntroduceParameterRefactoring introduceParameterRefactoring = new IntroduceParameterRefactoring(context.getCompilationUnit(), context.getSelectionOffset(), context.getSelectionLength());
LinkedProposalModelCore linkedProposalModel = new LinkedProposalModelCore();
introduceParameterRefactoring.setLinkedProposalModel(linkedProposalModel);
RefactoringStatus refactoringStatus = introduceParameterRefactoring.checkInitialConditions(new NullProgressMonitor());
if (refactoringStatus.isOK()) {
proposal = RefactorProposalUtility.getIntroduceParameterRefactoringProposals(params.context, context, context.getCoveringNode(), false, locations);
positionKey = null;
if (proposal.getProposal() instanceof RefactoringCorrectionProposalCore rcp) {
IntroduceParameterRefactoring refactoring = (IntroduceParameterRefactoring) rcp.getRefactoring();
ParameterInfo parameterInfo = refactoring.getAddedParameterInfo();
if (parameterInfo != null) {
positionKey = parameterInfo.getNewName();
}
}
} else {
StringBuilder buff = new StringBuilder();
for (RefactoringStatusEntry refactoringStatusEntry : refactoringStatus.getEntries()) {
buff.append(refactoringStatusEntry.toStatus().getMessage());
}
return new RefactorWorkspaceEdit(buff.toString());
}
} else if (RefactorProposalUtility.EXTRACT_INTERFACE_COMMAND.equals(params.command)) {
if (params.commandArguments != null && params.commandArguments.size() == 3) {
Expand Down
Loading

0 comments on commit 839831f

Please sign in to comment.