From 839831feefca6dbfdf75b6bea85145d76865cdf7 Mon Sep 17 00:00:00 2001 From: Snjeza Date: Tue, 19 Nov 2024 20:31:53 +0100 Subject: [PATCH] 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 --- .../jdt/ls/core/internal/JDTUtils.java | 10 +- .../corrections/RefactorProcessor.java | 128 +++++++++++++++--- .../BaseDocumentLifeCycleHandler.java | 4 + .../handlers/ChangeSignatureInfoHandler.java | 97 +++++++++++++ .../internal/handlers/CodeActionHandler.java | 2 +- .../handlers/GetRefactorEditHandler.java | 29 ++-- .../internal/handlers/JDTLanguageServer.java | 7 + .../internal/lsp/JavaProtocolExtensions.java | 13 +- .../text/correction/ChangeSignatureInfo.java | 40 ++++++ .../correction/RefactorProposalUtility.java | 98 ++++++++------ .../handlers/ChangeSignatureHandlerTest.java | 26 ++-- 11 files changed, 355 insertions(+), 99 deletions(-) create mode 100644 org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureInfoHandler.java create mode 100644 org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/ChangeSignatureInfo.java diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JDTUtils.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JDTUtils.java index 393b2efb1b..991af8951a 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JDTUtils.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JDTUtils.java @@ -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()); @@ -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)) { diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java index 67f73b0234..1044e5bf60 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/corrections/RefactorProcessor.java @@ -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; @@ -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; /** @@ -127,38 +130,99 @@ public RefactorProcessor(PreferenceManager preferenceManager) { } public List getProposals(CodeActionParams params, IInvocationContext context, IProblemLocation[] locations) throws CoreException { + return getProposals(params, context, locations, new NullProgressMonitor()); + } + + public List 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 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; } @@ -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; } @@ -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(); @@ -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; } diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java index f94c601c14..054b8e2b7f 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDocumentLifeCycleHandler.java @@ -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); diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureInfoHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureInfoHandler.java new file mode 100644 index 0000000000..af79e46191 --- /dev/null +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureInfoHandler.java @@ -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 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 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); + } + +} diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java index 1c0d73c284..414dab4d7e 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java @@ -202,7 +202,7 @@ public List> getCodeActionCommands(CodeActionParams if (containsKind(codeActionKinds, CodeActionKind.Refactor)) { try { - List refactorProposals = this.refactorProcessor.getProposals(params, context, locations); + List refactorProposals = this.refactorProcessor.getProposals(params, context, locations, monitor); refactorProposals.sort(comparator); proposals.addAll(refactorProposals); } catch (CoreException e) { diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GetRefactorEditHandler.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GetRefactorEditHandler.java index 1e0c7786ee..f477ec5d98 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GetRefactorEditHandler.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/GetRefactorEditHandler.java @@ -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"; @@ -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) { diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java index 3f6f56a3df..477bd2b8f2 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java @@ -84,6 +84,7 @@ import org.eclipse.jdt.ls.core.internal.managers.TelemetryManager; import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager; import org.eclipse.jdt.ls.core.internal.preferences.Preferences; +import org.eclipse.jdt.ls.core.internal.text.correction.ChangeSignatureInfo; import org.eclipse.lsp4j.CallHierarchyIncomingCall; import org.eclipse.lsp4j.CallHierarchyIncomingCallsParams; import org.eclipse.lsp4j.CallHierarchyItem; @@ -1134,6 +1135,12 @@ public CompletableFuture getRefactorEdit(GetRefactorEditP return computeAsync((monitor) -> GetRefactorEditHandler.getEditsForRefactor(params)); } + @Override + public CompletableFuture getChangeSignatureInfo(CodeActionParams params) { + debugTrace(">> java/getChangeSignatureInfo"); + return computeAsync((monitor) -> ChangeSignatureInfoHandler.getChangeSignatureInfo(params, monitor)); + } + @Override public CompletableFuture> inferSelection(InferSelectionParams params) { debugTrace(">> java/inferSelection"); diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java index 1230838497..a22cfcdf0e 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/JavaProtocolExtensions.java @@ -19,32 +19,33 @@ import org.eclipse.jdt.ls.core.internal.codemanipulation.GenerateGetterSetterOperation.AccessorField; import org.eclipse.jdt.ls.core.internal.handlers.ExtractInterfaceHandler.CheckExtractInterfaceResponse; import org.eclipse.jdt.ls.core.internal.handlers.FindLinksHandler.FindLinksParams; -import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.GenerateAccessorsParams; import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.AccessorCodeActionParams; +import org.eclipse.jdt.ls.core.internal.handlers.GenerateAccessorsHandler.GenerateAccessorsParams; import org.eclipse.jdt.ls.core.internal.handlers.GenerateConstructorsHandler.CheckConstructorsResponse; import org.eclipse.jdt.ls.core.internal.handlers.GenerateConstructorsHandler.GenerateConstructorsParams; import org.eclipse.jdt.ls.core.internal.handlers.GenerateDelegateMethodsHandler.CheckDelegateMethodsResponse; import org.eclipse.jdt.ls.core.internal.handlers.GenerateDelegateMethodsHandler.GenerateDelegateMethodsParams; import org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandler.CheckToStringResponse; import org.eclipse.jdt.ls.core.internal.handlers.GenerateToStringHandler.GenerateToStringParams; -import org.eclipse.jdt.ls.core.internal.handlers.InferSelectionHandler.InferSelectionParams; -import org.eclipse.jdt.ls.core.internal.handlers.InferSelectionHandler.SelectionInfo; import org.eclipse.jdt.ls.core.internal.handlers.GetRefactorEditHandler.GetRefactorEditParams; import org.eclipse.jdt.ls.core.internal.handlers.GetRefactorEditHandler.RefactorWorkspaceEdit; import org.eclipse.jdt.ls.core.internal.handlers.HashCodeEqualsHandler.CheckHashCodeEqualsResponse; import org.eclipse.jdt.ls.core.internal.handlers.HashCodeEqualsHandler.GenerateHashCodeEqualsParams; +import org.eclipse.jdt.ls.core.internal.handlers.InferSelectionHandler.InferSelectionParams; +import org.eclipse.jdt.ls.core.internal.handlers.InferSelectionHandler.SelectionInfo; import org.eclipse.jdt.ls.core.internal.handlers.MoveHandler.MoveDestinationsResponse; import org.eclipse.jdt.ls.core.internal.handlers.MoveHandler.MoveParams; import org.eclipse.jdt.ls.core.internal.handlers.OverrideMethodsHandler.AddOverridableMethodParams; import org.eclipse.jdt.ls.core.internal.handlers.OverrideMethodsHandler.OverridableMethodsResponse; import org.eclipse.jdt.ls.core.internal.handlers.WorkspaceSymbolHandler.SearchSymbolParams; +import org.eclipse.jdt.ls.core.internal.text.correction.ChangeSignatureInfo; import org.eclipse.lsp4j.CodeActionParams; import org.eclipse.lsp4j.Location; import org.eclipse.lsp4j.SymbolInformation; import org.eclipse.lsp4j.TextDocumentIdentifier; import org.eclipse.lsp4j.WorkspaceEdit; -import org.eclipse.lsp4j.extended.ProjectConfigurationsUpdateParam; import org.eclipse.lsp4j.extended.ProjectBuildParams; +import org.eclipse.lsp4j.extended.ProjectConfigurationsUpdateParam; import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.eclipse.lsp4j.jsonrpc.services.JsonNotification; import org.eclipse.lsp4j.jsonrpc.services.JsonRequest; @@ -68,6 +69,7 @@ public interface JavaProtocolExtensions { * @deprecated Please use {@link #projectConfigurationsUpdate(TextDocumentIdentifier)}. * @param documentUri the document from which the project configuration will be updated */ + @Deprecated @JsonNotification void projectConfigurationUpdate(TextDocumentIdentifier documentUri); @@ -129,6 +131,9 @@ public interface JavaProtocolExtensions { @JsonRequest CompletableFuture getRefactorEdit(GetRefactorEditParams params); + @JsonRequest + CompletableFuture getChangeSignatureInfo(CodeActionParams params); + @JsonRequest CompletableFuture> inferSelection(InferSelectionParams params); diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/ChangeSignatureInfo.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/ChangeSignatureInfo.java new file mode 100644 index 0000000000..c7fe1f3045 --- /dev/null +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/ChangeSignatureInfo.java @@ -0,0 +1,40 @@ +/******************************************************************************* + * Copyright (c) 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 + * http://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.text.correction; + +import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodException; +import org.eclipse.jdt.ls.core.internal.handlers.ChangeSignatureHandler.MethodParameter; + +public class ChangeSignatureInfo { + + public String methodIdentifier; + public String modifier; + public String returnType; + public String methodName; + public MethodParameter[] parameters; + public MethodException[] exceptions; + public String errorMessage; + + public ChangeSignatureInfo(String methodIdentifier, String modifier, String returnType, String methodName, MethodParameter[] parameters, MethodException[] exceptions) { + this.methodIdentifier = methodIdentifier; + this.modifier = modifier; + this.returnType = returnType; + this.methodName = methodName; + this.parameters = parameters; + this.exceptions = exceptions; + } + + public ChangeSignatureInfo(String errorMessage) { + this.errorMessage = errorMessage; + } +} \ No newline at end of file diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/RefactorProposalUtility.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/RefactorProposalUtility.java index af683e366a..f5eb61a42e 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/RefactorProposalUtility.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/text/correction/RefactorProposalUtility.java @@ -64,8 +64,6 @@ import org.eclipse.jdt.internal.corext.dom.Selection; import org.eclipse.jdt.internal.corext.dom.SelectionAnalyzer; import org.eclipse.jdt.internal.corext.fix.LinkedProposalModelCore; -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.RefactoringCoreMessages; import org.eclipse.jdt.internal.corext.refactoring.code.ExtractConstantRefactoring; @@ -73,7 +71,6 @@ import org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring; import org.eclipse.jdt.internal.corext.refactoring.code.IntroduceParameterRefactoring; import org.eclipse.jdt.internal.corext.refactoring.code.PromoteTempToFieldRefactoring; -import org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor; import org.eclipse.jdt.internal.corext.refactoring.structure.ExtractInterfaceProcessor; import org.eclipse.jdt.internal.corext.util.JdtFlags; import org.eclipse.jdt.internal.ui.text.correction.proposals.AssignToVariableAssistProposalCore; @@ -86,8 +83,6 @@ import org.eclipse.jdt.ls.core.internal.corrections.CorrectionMessages; import org.eclipse.jdt.ls.core.internal.corrections.ProposalKindWrapper; import org.eclipse.jdt.ls.core.internal.corrections.proposals.IProposalRelevance; -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.handlers.CodeActionHandler; import org.eclipse.jdt.ls.core.internal.preferences.PreferenceManager; import org.eclipse.jdt.ui.text.java.IInvocationContext; @@ -919,20 +914,63 @@ public static ProposalKindWrapper getIntroduceParameterRefactoringProposals(Code if (isUnnamedClass) { return null; } - final IntroduceParameterRefactoring introduceParameterRefactoring = new IntroduceParameterRefactoring(cu, context.getSelectionOffset(), context.getSelectionLength()); - LinkedProposalModelCore linkedProposalModel = new LinkedProposalModelCore(); - introduceParameterRefactoring.setLinkedProposalModel(linkedProposalModel); - if (introduceParameterRefactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) { - introduceParameterRefactoring.setParameterName(introduceParameterRefactoring.guessedParameterName()); + CompilationUnit root = context.getASTRoot(); + if (root == null) { + return null; + } + Selection ds = Selection.createFromStartLength(context.getSelectionOffset(), context.getSelectionLength()); + SelectionAnalyzer analyzer = new SelectionAnalyzer(ds, false); + root.accept(analyzer); + ASTNode[] selectedNodes = analyzer.getSelectedNodes(); + coveringNode = analyzer.getLastCoveringNode(); + ASTNode node; + // resolved in method body + if (selectedNodes != null && selectedNodes.length > 0) { + node = selectedNodes[0]; + } else { + node = coveringNode; + } + boolean inMethodBody = false; + if (node == null) { + inMethodBody = true; + } else { + while (node != null) { + int nodeType = node.getNodeType(); + if (nodeType == ASTNode.BLOCK && node.getParent() instanceof BodyDeclaration) { + inMethodBody = node.getParent().getNodeType() == ASTNode.METHOD_DECLARATION; + break; + } else if (nodeType == ASTNode.ANONYMOUS_CLASS_DECLARATION) { + break; + } + node = node.getParent(); + } + } + boolean inAnnotation = false; + if (inMethodBody) { + while (node != null) { + if (node instanceof org.eclipse.jdt.core.dom.Annotation) { + inAnnotation = true; + break; + } + node = node.getParent(); + } + } + if (inMethodBody && !inAnnotation && RefactoringAvailabilityTesterCore.isIntroduceParameterAvailable(selectedNodes, coveringNode)) { String label = RefactoringCoreMessages.IntroduceParameterRefactoring_name + "..."; int relevance = (problemLocations != null && problemLocations.length > 0) ? IProposalRelevance.EXTRACT_CONSTANT_ERROR : IProposalRelevance.EXTRACT_CONSTANT; if (returnAsCommand) { CUCorrectionCommandProposal proposal = new CUCorrectionCommandProposal(label, cu, relevance, APPLY_REFACTORING_COMMAND_ID, Arrays.asList(INTRODUCE_PARAMETER_COMMAND, params)); return CodeActionHandler.wrap(proposal, JavaCodeActionKind.REFACTOR_INTRODUCE_PARAMETER); } - RefactoringCorrectionProposalCore proposal = new RefactoringCorrectionProposalCore(label, cu, introduceParameterRefactoring, relevance); - proposal.setLinkedProposalModel(linkedProposalModel); - return CodeActionHandler.wrap(proposal, JavaCodeActionKind.REFACTOR_INTRODUCE_PARAMETER); + final IntroduceParameterRefactoring introduceParameterRefactoring = new IntroduceParameterRefactoring(cu, context.getSelectionOffset(), context.getSelectionLength()); + LinkedProposalModelCore linkedProposalModel = new LinkedProposalModelCore(); + introduceParameterRefactoring.setLinkedProposalModel(linkedProposalModel); + if (introduceParameterRefactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) { + introduceParameterRefactoring.setParameterName(introduceParameterRefactoring.guessedParameterName()); + RefactoringCorrectionProposalCore proposal = new RefactoringCorrectionProposalCore(label, cu, introduceParameterRefactoring, relevance); + proposal.setLinkedProposalModel(linkedProposalModel); + return CodeActionHandler.wrap(proposal, JavaCodeActionKind.REFACTOR_INTRODUCE_PARAMETER); + } } return null; } @@ -976,21 +1014,10 @@ public static ProposalKindWrapper getChangeSignatureProposal(CodeActionParams pa IJavaElement element = methodBinding.getJavaElement(); if (element instanceof IMethod method) { try { - ChangeSignatureProcessor processor = new ChangeSignatureProcessor(method); - if (RefactoringAvailabilityTesterCore.isChangeSignatureAvailable(method) && processor.checkInitialConditions(new NullProgressMonitor()).isOK()) { - List 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 exceptions = new ArrayList<>(); - for (ExceptionInfo info : processor.getExceptionInfos()) { - exceptions.add(new MethodException(info.getFullyQualifiedName(), info.getElement().getHandleIdentifier())); - } - ChangeSignatureInfo info = new ChangeSignatureInfo(method.getHandleIdentifier(), JdtFlags.getVisibilityString(processor.getVisibility()), processor.getReturnTypeString(), method.getElementName(), - parameters.toArray(MethodParameter[]::new), exceptions.toArray(MethodException[]::new)); + if (RefactoringAvailabilityTesterCore.isChangeSignatureAvailable(method)) { String label = Messages.format(org.eclipse.jdt.ls.core.internal.corext.refactoring.RefactoringCoreMessages.ChangeSignatureRefactoring_change_signature_for, new String[] { method.getElementName() }); CUCorrectionCommandProposal p1 = new CUCorrectionCommandProposal(label, cu, IProposalRelevance.CHANGE_METHOD_SIGNATURE, RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, - Arrays.asList(RefactorProposalUtility.CHANGE_SIGNATURE_COMMAND, params, info)); + Arrays.asList(RefactorProposalUtility.CHANGE_SIGNATURE_COMMAND, params)); return CodeActionHandler.wrap(p1, JavaCodeActionKind.REFACTOR_CHANGE_SIGNATURE); } } catch (CoreException e) { @@ -1000,25 +1027,6 @@ public static ProposalKindWrapper getChangeSignatureProposal(CodeActionParams pa return null; } - public static class ChangeSignatureInfo { - - public String methodIdentifier; - public String modifier; - public String returnType; - public String methodName; - public MethodParameter[] parameters; - public MethodException[] exceptions; - - public ChangeSignatureInfo(String methodIdentifier, String modifier, String returnType, String methodName, MethodParameter[] parameters, MethodException[] exceptions) { - this.methodIdentifier = methodIdentifier; - this.modifier = modifier; - this.returnType = returnType; - this.methodName = methodName; - this.parameters = parameters; - this.exceptions = exceptions; - } - } - public static String getUniqueMethodName(ASTNode astNode, String suggestedName) throws JavaModelException { while (astNode != null && !(astNode instanceof TypeDeclaration || astNode instanceof AnonymousClassDeclaration)) { astNode = astNode.getParent(); diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandlerTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandlerTest.java index 34bef311c0..12baf379b8 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandlerTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/handlers/ChangeSignatureHandlerTest.java @@ -36,8 +36,8 @@ import org.eclipse.jdt.ls.core.internal.LanguageServerWorkingCopyOwner; 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.RefactorProposalUtility; -import org.eclipse.jdt.ls.core.internal.text.correction.RefactorProposalUtility.ChangeSignatureInfo; import org.eclipse.lsp4j.CodeAction; import org.eclipse.lsp4j.CodeActionParams; import org.eclipse.lsp4j.Command; @@ -91,15 +91,13 @@ public void testChangeSignatureRefactoringExists() throws JavaModelException { Assert.assertNotNull(changeSignatureCommand); Assert.assertEquals(RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, changeSignatureCommand.getCommand()); List arguments = changeSignatureCommand.getArguments(); - Assert.assertEquals(3, arguments.size()); + Assert.assertEquals(2, arguments.size()); Object arg0 = arguments.get(0); assertEquals(true, arg0 instanceof String); assertEquals("changeSignature", arg0); Object arg1 = arguments.get(1); assertEquals(true, arg1 instanceof CodeActionParams); - Object arg2 = arguments.get(2); - assertEquals(true, arg2 instanceof ChangeSignatureInfo); - ChangeSignatureInfo info = (ChangeSignatureInfo) arg2; + ChangeSignatureInfo info = server.getChangeSignatureInfo((CodeActionParams) arg1).join(); assertEquals("public", info.modifier); assertEquals(0, info.exceptions.length); assertEquals("=TestProject/src arguments = changeSignatureCommand.getArguments(); - Assert.assertEquals(3, arguments.size()); + Assert.assertEquals(2, arguments.size()); Object arg1 = arguments.get(1); assertEquals(true, arg1 instanceof CodeActionParams); - Object arg2 = arguments.get(2); - assertEquals(true, arg2 instanceof ChangeSignatureInfo); - ChangeSignatureInfo info = (ChangeSignatureInfo) arg2; + ChangeSignatureInfo info = server.getChangeSignatureInfo((CodeActionParams) arg1).join(); IJavaElement element = JavaCore.create(info.methodIdentifier); assertEquals(true, element instanceof IMethod); List parameters = List.of(info.parameters[0], new MethodParameter("String", "input1", "null", ParameterInfo.INDEX_FOR_ADDED)); @@ -185,12 +181,10 @@ public void testChangeSignatureRefactoringJavadocAddDelete() throws CoreExceptio Assert.assertNotNull(changeSignatureCommand); Assert.assertEquals(RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, changeSignatureCommand.getCommand()); List arguments = changeSignatureCommand.getArguments(); - Assert.assertEquals(3, arguments.size()); + Assert.assertEquals(2, arguments.size()); Object arg1 = arguments.get(1); assertEquals(true, arg1 instanceof CodeActionParams); - Object arg2 = arguments.get(2); - assertEquals(true, arg2 instanceof ChangeSignatureInfo); - ChangeSignatureInfo info = (ChangeSignatureInfo) arg2; + ChangeSignatureInfo info = server.getChangeSignatureInfo((CodeActionParams) arg1).join(); IJavaElement element = JavaCore.create(info.methodIdentifier); assertEquals(true, element instanceof IMethod); List parameters = List.of(info.parameters[0], new MethodParameter("String", "input1", "null", ParameterInfo.INDEX_FOR_ADDED)); @@ -228,12 +222,10 @@ public void testChangeSignatureRefactoringJavadocAddDelete() throws CoreExceptio Assert.assertNotNull(changeSignatureCommand); Assert.assertEquals(RefactorProposalUtility.APPLY_REFACTORING_COMMAND_ID, changeSignatureCommand.getCommand()); arguments = changeSignatureCommand.getArguments(); - Assert.assertEquals(3, arguments.size()); + Assert.assertEquals(2, arguments.size()); arg1 = arguments.get(1); assertEquals(true, arg1 instanceof CodeActionParams); - arg2 = arguments.get(2); - assertEquals(true, arg2 instanceof ChangeSignatureInfo); - info = (ChangeSignatureInfo) arg2; + info = server.getChangeSignatureInfo((CodeActionParams) arg1).join(); element = JavaCore.create(info.methodIdentifier); assertEquals(true, element instanceof IMethod); parameters = List.of(info.parameters[1]);