From 9e2d7465cb1304e7fbecf5273099dca8b0ab1f75 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Fri, 10 Jul 2026 14:29:00 -0400 Subject: [PATCH 1/2] Fix add missing method declaration when target is another CU - modify AddMissingMethodDeclarationFixCore to properly find the type of the ExpressionMethodReference instead of looking for an ancestor of the node - verify that the method binding doesn't already exist - use the target compilation unit rather than the compilation unit where the assist is started - modify QuickAssistProcessor.getAddMethodDeclaration() to create a special FixCorrectionProposal which specifies the target CU rather than the one in the assist context - add new test to AssistQuickFixTest1d8 - fixes #3061 --- .../AddMissingMethodDeclarationFixCore.java | 121 ++++++++++++++++-- .../tests/quickfix/AssistQuickFixTest1d8.java | 60 ++++++++- .../text/correction/QuickAssistProcessor.java | 14 +- 3 files changed, 176 insertions(+), 19 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java index e825dbd24fb..54125b988c0 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2024 IBM Corporation and others. + * Copyright (c) 2024, 2026 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -16,9 +16,13 @@ import java.util.List; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.jdt.core.ICompilationUnit; +import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.dom.AST; import org.eclipse.jdt.core.dom.ASTNode; +import org.eclipse.jdt.core.dom.ASTVisitor; import org.eclipse.jdt.core.dom.Assignment; import org.eclipse.jdt.core.dom.Block; import org.eclipse.jdt.core.dom.CompilationUnit; @@ -33,7 +37,6 @@ import org.eclipse.jdt.core.dom.ParameterizedType; import org.eclipse.jdt.core.dom.PrimitiveType; import org.eclipse.jdt.core.dom.ReturnStatement; -import org.eclipse.jdt.core.dom.SimpleName; import org.eclipse.jdt.core.dom.SingleVariableDeclaration; import org.eclipse.jdt.core.dom.Type; import org.eclipse.jdt.core.dom.TypeDeclaration; @@ -42,9 +45,12 @@ import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; import org.eclipse.jdt.core.dom.rewrite.ImportRewrite; import org.eclipse.jdt.core.dom.rewrite.ListRewrite; +import org.eclipse.jdt.core.refactoring.CompilationUnitChange; import org.eclipse.jdt.internal.corext.dom.ASTNodes; +import org.eclipse.jdt.internal.corext.dom.AbortSearchException; import org.eclipse.jdt.internal.corext.dom.Bindings; +import org.eclipse.jdt.internal.corext.refactoring.Checks; import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite; import org.eclipse.jdt.internal.ui.text.correction.CorrectionMessages; @@ -52,8 +58,32 @@ public class AddMissingMethodDeclarationFixCore extends CompilationUnitRewriteOperationsFixCore { + private final CompilationUnit fCompilationUnit; + public AddMissingMethodDeclarationFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation operation) { super(name, compilationUnit, operation); + fCompilationUnit= compilationUnit; + } + + /** + * Returns the compilation unit being modified by this fix. + * This may be different from the compilation unit containing the quick assist invocation. + * + * @return the target compilation unit + */ + public ICompilationUnit getCompilationUnit() { + return (ICompilationUnit) fCompilationUnit.getJavaElement(); + } + + @Override + public CompilationUnitChange createChange(IProgressMonitor progressMonitor) throws CoreException { + CompilationUnitChange change= super.createChange(progressMonitor); + if (change != null) { + // When modifying a file that may not be open in an editor, we need to ensure + // the change is saved. The default LEAVE_DIRTY mode doesn't work for unopened files. + change.setSaveMode(org.eclipse.ltk.core.refactoring.TextFileChange.FORCE_SAVE); + } + return change; } public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclaration(CompilationUnit compilationUnit, ASTNode node) { @@ -64,7 +94,37 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati return null; } - TypeDeclaration typeDeclaration= ASTNodes.getParent(methodReferenceNode, TypeDeclaration.class); + IMethodBinding refBinding= methodReferenceNode.resolveMethodBinding(); + if (refBinding != null) { + return null; + } + + Expression exp= methodReferenceNode.getExpression(); + ITypeBinding expBinding= exp.resolveTypeBinding(); + if (expBinding == null) { + return null; + } + + IJavaElement element= expBinding.getJavaElement(); + CompilationUnit cu= null; + if (element == null) { + return null; + } + element= element.getAncestor(IJavaElement.COMPILATION_UNIT); + if (element instanceof ICompilationUnit icu) { + cu= Checks.convertICUtoCU(icu); + } + if (cu == null) { + return null; + } + + TypeDeclaration typeDeclaration= null; + TypeDeclarationFinder finder= new TypeDeclarationFinder(expBinding); + try { + cu.accept(finder); + } catch (AbortSearchException e) { + typeDeclaration= finder.getTypeDeclaration(); + } if (typeDeclaration == null) { return null; } @@ -85,7 +145,7 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati Assignment variableAssignment= ASTNodes.getParent(methodReferenceNode, Assignment.class); String label= Messages.format(CorrectionMessages.AddUnimplementedMethodReferenceOperation_AddMissingMethod_group, - new String[] { methodReferenceNode.getName().getIdentifier(), typeDeclaration.getName().getIdentifier() }); + new String[] { methodReferenceNode.getName().getIdentifier(), expBinding.getName() }); if ((variableAssignment != null || variableDeclarationStatement != null) && methodInvocationNode == null) { /* @@ -109,7 +169,9 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati if (returnType.binding == null) { return null; } - return new AddMissingMethodDeclarationFixCore(label, compilationUnit, new AddMissingMethodDeclarationProposalOperation(methodReferenceNode, returnType, null)); + // Note: passing cu (the target compilation unit) instead of compilationUnit (the current one) + // This fix modifies the target type's compilation unit, which may be different from the current file + return new AddMissingMethodDeclarationFixCore(label, cu, new AddMissingMethodDeclarationProposalOperation(methodReferenceNode, typeDeclaration, returnType, null)); } else { if (methodInvocationNode == null) { return null; @@ -132,7 +194,9 @@ public static AddMissingMethodDeclarationFixCore createAddMissingMethodDeclarati // node not found return null; } - return new AddMissingMethodDeclarationFixCore(label, compilationUnit, new AddMissingMethodDeclarationProposalOperation(methodReferenceNode, null, methodBinding)); + // Note: passing cu (the target compilation unit) instead of compilationUnit (the current one) + // This fix modifies the target type's compilation unit, which may be different from the current file + return new AddMissingMethodDeclarationFixCore(label, cu, new AddMissingMethodDeclarationProposalOperation(methodReferenceNode, typeDeclaration, null, methodBinding)); } } @@ -160,6 +224,28 @@ private static class ReturnType { public ITypeBinding binding; } + private static class TypeDeclarationFinder extends ASTVisitor { + private TypeDeclaration foundDeclaration; + private final ITypeBinding fTypeBinding; + + public TypeDeclarationFinder(ITypeBinding typeBinding) { + this.fTypeBinding= typeBinding; + } + + public TypeDeclaration getTypeDeclaration() { + return foundDeclaration; + } + + @Override + public boolean visit(TypeDeclaration node) { + if (this.fTypeBinding.isEqualTo(node.resolveBinding())) { + foundDeclaration= node; + throw new AbortSearchException(); + } + return true; + } + } + private static class AddMissingMethodDeclarationProposalOperation extends CompilationUnitRewriteOperation { @@ -167,9 +253,12 @@ private static class AddMissingMethodDeclarationProposalOperation extends Compil private ReturnType returnType; + private TypeDeclaration typeDeclaration; + private IMethodBinding methodBinding; - public AddMissingMethodDeclarationProposalOperation(ExpressionMethodReference methodReferenceNode, ReturnType returnType, IMethodBinding methodBinding) { + + public AddMissingMethodDeclarationProposalOperation(ExpressionMethodReference methodReferenceNode, TypeDeclaration typeDeclaration, ReturnType returnType, IMethodBinding methodBinding) { if (returnType == null && methodBinding == null) { throw new IllegalArgumentException("both returnType and methodBinding cannot be null."); //$NON-NLS-1$ } @@ -177,12 +266,13 @@ public AddMissingMethodDeclarationProposalOperation(ExpressionMethodReference me this.methodReferenceNode= methodReferenceNode; this.returnType= returnType; this.methodBinding= methodBinding; + this.typeDeclaration= typeDeclaration; } + @Override public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException { boolean addStaticModifier= false; - TypeDeclaration typeDeclaration= ASTNodes.getParent(this.methodReferenceNode, TypeDeclaration.class); if (QuickAssistProcessorUtil.isTypeReferenceToInstanceMethod(methodReferenceNode)) { addStaticModifier= true; @@ -201,7 +291,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore returnType.type= importRewrite.addImport(returnType.binding, ast); MethodDeclaration newMethodDeclaration= ast.newMethodDeclaration(); - newMethodDeclaration.setName((SimpleName) rewrite.createCopyTarget(methodReferenceNode.getName())); + newMethodDeclaration.setName(ast.newSimpleName(methodReferenceNode.getName().getFullyQualifiedName())); newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PRIVATE_KEYWORD)); if (addStaticModifier) { newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.STATIC_KEYWORD)); @@ -241,7 +331,16 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore ITypeBinding[] parameterTypesFunctionalInterface= parameterTypes[index].getFunctionalInterfaceMethod().getParameterTypes(); ITypeBinding returnTypeBindingFunctionalInterface= parameterTypes[index].getFunctionalInterfaceMethod().getReturnType(); MethodDeclaration newMethodDeclaration= ast.newMethodDeclaration(); - newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PRIVATE_KEYWORD)); +// CompilationUnit root= (CompilationUnit) methodReferenceNode.getRoot(); + newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PUBLIC_KEYWORD)); +// if (root.subtreeMatch(new ASTMatcher(), cuRewrite.getRoot())) { +// newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PRIVATE_KEYWORD)); +// } else { +// PackageDeclaration pkgdecl= root.getPackage(); +// if (!pkgdecl.subtreeMatch(new ASTMatcher(), cuRewrite.getRoot().getPackage())) { +// newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PUBLIC_KEYWORD)); +// } +// } if (addStaticModifier) { newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.STATIC_KEYWORD)); } @@ -263,7 +362,7 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore addIfMissing(newMethodDeclaration, newTypeParameter); } } - newMethodDeclaration.setName((SimpleName) rewrite.createCopyTarget(methodReferenceNode.getName())); + newMethodDeclaration.setName(ast.newSimpleName(methodReferenceNode.getName().getFullyQualifiedName())); newMethodDeclaration.setReturnType2(newReturnType); pLoop: for (int i= 0; i < parameterTypesFunctionalInterface.length; i++) { ITypeBinding parameterType2= parameterTypesFunctionalInterface[i]; diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java index f9da4508bca..228fe201564 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2025 IBM Corporation and others. + * Copyright (c) 2013, 2026 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -3945,7 +3945,7 @@ static String method4() { AssistContext context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); List proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 4); + assertNumberOfProposals(proposals, 3); assertCorrectLabels(proposals); String expected= """ @@ -3998,7 +3998,7 @@ static String method4() { context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 4); + assertNumberOfProposals(proposals, 3); assertCorrectLabels(proposals); expected= """ @@ -4051,7 +4051,7 @@ static String method4() { context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 5); + assertNumberOfProposals(proposals, 4); assertCorrectLabels(proposals); expected= """ @@ -4104,7 +4104,7 @@ static String method4() { context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 5); + assertNumberOfProposals(proposals, 4); assertCorrectLabels(proposals); expected= """ @@ -4157,7 +4157,7 @@ static String method4() { context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 5); + assertNumberOfProposals(proposals, 4); assertCorrectLabels(proposals); expected= """ @@ -4210,7 +4210,7 @@ static String method4() { context= getCorrectionContext(cu, offset, 0); assertNoErrors(context); proposals= collectAssists(context, false); - assertNumberOfProposals(proposals, 5); + assertNumberOfProposals(proposals, 4); assertCorrectLabels(proposals); expected= """ @@ -7884,5 +7884,51 @@ public testExample() { assertNumberOfProposals(proposals, 0); } + @Test + public void testIssue3061() throws Exception { + // We have a superclass that contains another TestClass as package private. No proposal expected. + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String src1= """ + package test.test1; + public class Second { + + public void consume1(String s) { + } + } + """; + pack1.createCompilationUnit("Second.java", src1, false, null); + + String src= """ + package test.test1; + import java.util.function.Consumer; + + public class First { + void f(Consumer s) { + } + + void g() { + Second second = new Second(); + f(second::consume1); + f(second::consume2); + } + } + """; + ICompilationUnit cu1= pack1.createCompilationUnit("First.java", src, false, null); + int offset= src.indexOf("second::consume2"); + AssistContext context= getCorrectionContext(cu1, offset, 0); + List proposals= collectAssists(context, false); + String expected= """ + package test.test1; + public class Second { + + public void consume1(String s) { + } + + public void consume2(String string1) { + } + } + """; + assertExpectedExistInProposals(proposals, new String[] { expected }); + } } diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java index 77e4d9a2f32..5139304bc93 100644 --- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java +++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java @@ -227,6 +227,7 @@ import org.eclipse.jdt.internal.ui.text.correction.proposals.AssignToVariableAssistProposal; import org.eclipse.jdt.internal.ui.text.correction.proposals.ConvertFieldNamingConventionProposal; import org.eclipse.jdt.internal.ui.text.correction.proposals.FixCorrectionProposal; +import org.eclipse.jdt.internal.ui.text.correction.proposals.FixCorrectionProposalCore; import org.eclipse.jdt.internal.ui.text.correction.proposals.LinkedCorrectionProposal; import org.eclipse.jdt.internal.ui.text.correction.proposals.LinkedNamesAssistProposal; import org.eclipse.jdt.internal.ui.text.correction.proposals.NewDefiningMethodProposal; @@ -1247,7 +1248,18 @@ private static boolean getAddMethodDeclaration(IInvocationContext context, ASTNo } Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_CHANGE); - FixCorrectionProposal proposal= new FixCorrectionProposal(fix, null, IProposalRelevance.ADD_INFERRED_LAMBDA_PARAMETER_TYPES, image, context); + // Create a proposal that properly handles the change to the target compilation unit + // The fix may modify a different file than the current one, so we need special handling + ICompilationUnit targetCU= fix.getCompilationUnit(); + FixCorrectionProposal proposal= new FixCorrectionProposal(fix, null, IProposalRelevance.ADD_INFERRED_LAMBDA_PARAMETER_TYPES, image, context, + new FixCorrectionProposalCore(fix, null, IProposalRelevance.ADD_INFERRED_LAMBDA_PARAMETER_TYPES, context) { + @Override + public ICompilationUnit getCompilationUnit() { + // Return the target compilation unit, not the current one from the context + return targetCU; + } + }) { + }; resultingCollections.add(proposal); return true; } From b078e1854691ebffbbaff2409a90ad08e16ad286 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Fri, 10 Jul 2026 16:03:58 -0400 Subject: [PATCH 2/2] Fix modifier logic --- .../fix/AddMissingMethodDeclarationFixCore.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java index 54125b988c0..c700383c82e 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/AddMissingMethodDeclarationFixCore.java @@ -21,6 +21,7 @@ import org.eclipse.jdt.core.ICompilationUnit; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.core.dom.AST; +import org.eclipse.jdt.core.dom.ASTMatcher; import org.eclipse.jdt.core.dom.ASTNode; import org.eclipse.jdt.core.dom.ASTVisitor; import org.eclipse.jdt.core.dom.Assignment; @@ -331,16 +332,12 @@ public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore ITypeBinding[] parameterTypesFunctionalInterface= parameterTypes[index].getFunctionalInterfaceMethod().getParameterTypes(); ITypeBinding returnTypeBindingFunctionalInterface= parameterTypes[index].getFunctionalInterfaceMethod().getReturnType(); MethodDeclaration newMethodDeclaration= ast.newMethodDeclaration(); -// CompilationUnit root= (CompilationUnit) methodReferenceNode.getRoot(); - newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PUBLIC_KEYWORD)); -// if (root.subtreeMatch(new ASTMatcher(), cuRewrite.getRoot())) { -// newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PRIVATE_KEYWORD)); -// } else { -// PackageDeclaration pkgdecl= root.getPackage(); -// if (!pkgdecl.subtreeMatch(new ASTMatcher(), cuRewrite.getRoot().getPackage())) { -// newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PUBLIC_KEYWORD)); -// } -// } + CompilationUnit root= (CompilationUnit) methodReferenceNode.getRoot(); + if (root.subtreeMatch(new ASTMatcher(), cuRewrite.getRoot())) { + newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PRIVATE_KEYWORD)); + } else { + newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.PUBLIC_KEYWORD)); + } if (addStaticModifier) { newMethodDeclaration.modifiers().add(ast.newModifier(ModifierKeyword.STATIC_KEYWORD)); }