Community
Participate
Working Groups
interface I { public void doit(); } public class X { I i = () -> {}; } Change I.doit()'s signature - the lambda does not compile anymore. Now that declarations in hierarchy correctly tags the lambda as implementing I.doit, expectation is that this should work.
At the moment the JavaElement for LambdaExpression's answers true to isReadOnly. This does not seem to have a brearing though.
@Srikanth: Can you elaborate the steps you performed after which you ended up with a broken lambda? When i tested, Java Model Exception is thrown when the 'OK' button is clicked after adding a method parameter in I#doit() using Alt+Shift+C: Java Model Exception: Java Model Status [lambda$1() [in <lambda> [in i [in X [in [Working copy] X.java [in p5 [in src [in com.test.hover.lambda]]]]]]] does not exist] at org.eclipse.jdt.internal.core.JavaElement.newNotPresentException(JavaElement.java:499) at org.eclipse.jdt.internal.core.JavaElement.openWhenClosed(JavaElement.java:533) at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:259) at org.eclipse.jdt.internal.core.JavaElement.getElementInfo(JavaElement.java:245) at org.eclipse.jdt.internal.core.SourceMethod.getParameterNames(SourceMethod.java:118) at org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor.checkParameterNamesInRippleMethods(ChangeSignatureProcessor.java:1140) at org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor.checkRenamings(ChangeSignatureProcessor.java:1130) at org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor.checkFinalConditions(ChangeSignatureProcessor.java:855) at org.eclipse.ltk.core.refactoring.participants.ProcessorBasedRefactoring.checkFinalConditions(ProcessorBasedRefactoring.java:224) at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121) at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:209) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2333) at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:122)
When we rename(Atl+Shift+R) the method I.doit() also we get a fatal error: Related method 'lambda$1' (declared in 'p5.X.<lambda>') does not exist in the model.
(In reply to Manju Mathew from comment #3) > When we rename(Atl+Shift+R) the method I.doit() also we get a fatal error: > Related method 'lambda$1' (declared in 'p5.X.<lambda>') does not exist in > the model. Manju, please test with HEAD.
After pulling the latest from HEAD, confirming that the lambda is broken after the Change Method Signature(Alt+Shift+C) refactoring modifies (Add/Edit/Remove) method parameter. The rename refactoring (Alt+Shift+R) completes normally. There is no exception from model as stated earlier.
Here is what i found after analysis: package p1; interface I { public void doit(); // Invoke Alt+Shift+C here and add 'int i' as the method parameter } public class X { I i = () -> {}; I i2= new I() { @Override public void doit() { } }; } During Change Method Signature refactoring, RefactoringSearchEngine#internalSearch invoke SearchEngine#search with SearchPattern: "MethodCombinedPattern: p1.I.doit() --> void, exact match, case sensitive, generic erasure match, fine grain: none | MethodCombinedPattern: p1.X.*.Lambda(I).doit() --> void, exact match, case sensitive, generic erasure match, fine grain: none | MethodCombinedPattern: p1.X.*..doit() --> void, exact match, case sensitive, generic erasure match, fine grain: none" Lambda is part of the search pattern, but the search does not return lambda as part of the result and hence lambda is not updated after the refactoring. @Srikanth: Is there any problem with the SeachPattern that UI is sending to the core?
(In reply to Manju Mathew from comment #6) > During Change Method Signature refactoring, > RefactoringSearchEngine#internalSearch invoke SearchEngine#search with > SearchPattern: > "MethodCombinedPattern: p1.I.doit() --> void, exact match, case sensitive, > generic erasure match, fine grain: none > | MethodCombinedPattern: p1.X.*.Lambda(I).doit() --> void, exact match, case > sensitive, generic erasure match, fine grain: none > | MethodCombinedPattern: p1.X.*..doit() --> void, exact match, case > sensitive, generic erasure match, fine grain: none" > Lambda is part of the search pattern, but the search does not return lambda > as part of the result and hence lambda is not updated after the refactoring. > > @Srikanth: Is there any problem with the SeachPattern that UI is sending to > the core? Well, Lambda(I) is a made up name and won't match any index entries. I'll see what can be done.
Folks, this would be nice to show case in ECNA. Appreciate your enabling it if possible. This is blocked by a Core bug for which early patch is posted in bug 430159. With that patch search engine returns the lambda nodes as results, but there is a subsequent exception inside UI code from: org.eclipse.jdt.internal.corext.refactoring.structure.ChangeSignatureProcessor.NullOccurrenceUpdate.updateNode()
Created attachment 240839 [details] WIP Patch @Markus, can you have a look at it and let me know if i am in the right direction. Below are assumptions taken while working on this bug: 1. Ignored method name change for LambdaExpression. 2. Ignored the visibility change(method visibility being changed from private to public etc) possibility as LambdaExpression can come from an interface method which is always public. 3. Ignored Exception change as LambdaExpression do not store this information. 4. This fix is not complete without releasing the fix for bug 408979. 5. Currently when Change Method Signature refactoring is invoked directly on the lambda, due to bug 429814 the LambdaMethod is not found, but the parent SAM from the interface, which is wrong. 6. In the case of ExpressionMethodReference, only the method name change will be considered. Tomorrow i will work on the remaining issues and code polishing.
(In reply to Manju Mathew from comment #9) The general direction looks good. Comments: - #createChangeManager: why "|| update instanceof ExpressionMethodRefUpdate"? - Looks like LambdaExpressionUpdate copies a lot of code from DeclarationUpdate. Try to reduce copied code. Maybe add an "AbstractDeclarationUpdate<N extends VariableDeclaration> extends OccurrenceUpdate<N>", and make DeclarationUpdate and LambdaExpressionUpdate extend that one? - Make sure you have test cases for lambda expressions with and without parameters, and with and without explicit parameter types.
(In reply to Markus Keller from comment #10) > - Make sure you have test cases for lambda expressions with and without > parameters, and with and without explicit parameter types. and with and without parentheses around parameters.
(In reply to Manju Mathew from comment #9) > 4. This fix is not complete without releasing the fix for bug 408979. Done. > 5. Currently when Change Method Signature refactoring is invoked directly on > the lambda, due to bug 429814 the LambdaMethod is not found, but the parent > SAM from the interface, which is wrong. That's no big deal. ChangeSignatureProcessor#checkInitialConditions(..) should anyway check this and make RefactoringExecutionStarter#startChangeSignatureRefactoring(..) ask the user whether it's OK to start the refactoring on the top method.
Created attachment 240895 [details] Patch + Tests Markus, as you had suggested, i have created an abstract class to handle LambdaExpression and MethodDeclaration. 1. When change method declaration is invoked directly on lambda, currently we do not get SAM(as i claimed earlier), but the enclosing method where the lambda is declared. Bug 429785 should solve this issue. 2. When a parameter is deleted which is used in a lambda expression, the message shown needs better wording. 3. Tests are added for lambda expressions with and without parameters, and with and without explicit parameter types. Also with and without parentheses around parameters. 4. Tests are incomplete, need to add more test case for removing parameters and some more cases.
New test suites need to be added to a test suite that is included from test.xml. Added ChangeSignatureTests18 to AllRefactoringTests. Test resources should use the same casing as their test method. I'm pretty sure A_testlambda0_in.java for testLambda0() would not be found on case-sensitive file systems. Renamed the resources. class AbstractDeclarationUpdate<N extends VariableDeclaration> should extend OccurrenceUpdate<N> and class DeclarationUpdate should extend AbstractDeclarationUpdate<SingleVariableDeclaration>. This may not be absolutely necessary right now, but it will become necessary when the ASTRewrite API is generified. And it's the right thing to do anyway. And it helps avoid the strange things you did to DeclarationUpdate#createNewParamgument(..). I didn'd find a caller of LambdaExpressionUpdate#getMethodName() => removed it. But AbstractDeclarationUpdate#checkIfDeletedParametersUsed() now wrongly referred to ChangeSignatureProcessor#getMethodName(). Fixed to use "getMethod().getElementName()". Reverted some other unnecessary edits and reverted order of methods, so that I could easily see the actual changes. Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=7683edb224866dae4aae1d071fdfbb035f7bbdd6
