Quickfix "Extract to local Variable": sometimes creates "MISSING()" #1136
Comments
|
eclipse.buildId=4.31.0.I20240124-1800 |
|
I'll look at it. |
|
I noticed that as well but as described it sometimes works and sometimes not even for the same code.... |
|
Here is even a video: |
|
Another variant where literally nothing is inserted instead (reproducible eclipse.buildId=4.31.0.I20240130-1800): with another exception logged: |
|
My research so far is frustrating. The problem does not occur when I debug and appears to be a timing issue. I can recreate in an I-build and via remote debugging I can catch the BadLocation being thrown. The error is occurring adding the linked positions for the additional names that are offered for the new variable declaration (in LinkedProposalModelPresenter.enterLinkedMode()). |
- add missing synchronization for getCurrentChange() which accesses fChange which is created in a synchronized method - potential fix for eclipse-jdt#1136
- add missing synchronization for getCurrentChange() which accesses fChange which is created in a synchronized method - potential fix for #1136
|
I believe this is fixed now. I tested with I20240203 and was unable to create the problem. Although the hashCode() method has changed from the original example, I restored it back to the original code for the test. The error occurred for me when hitting enter quickly before the preview was calculated for the extract to local variable (replace all) or extract to local variable for the given method call. If the clicking is delayed or the code debugged with breakpoints, the error did not occur. There appears to be a significant time in calculating the extract to local variable (replace all) and this assist should have its relevance lowered below the single extract (in one instance I got a UI delay registered, but the assist worked successfully). I am going to mark this closed and if a new example causes the BadLocationException, I suggest a new issue with a new scenario. Fixed by #1162 |
|
With I20240204 i can still reproduce |
|
@jukzi I can reproduce. I think this is a separate issue with the code not prepared to handle a conditional statement. I will investigate. |
|
@jukzi This is definitely a separate issue. The ExtractTempRefactoring code will sometimes not be done because the expression should not be moved before the statement it is in. For example, when you have a conditional, the method call in the false path moved before the conditional might change the result of the condition test. The refactoring is figuring this out in final conditions. The QuickAssistProcessor is only checking the initial conditions and creating a proposal that wraps the refactoring. There needs to be a check so that the proposal is not offered to the end-user. I have a patch for this but need to add the same logic for extract constant which also wraps a refactoring. |
|
See #1176 |
|
Closing this with #1177 merged. |
i have seen such behavior multiple times in the last weeks.
here is a reproducer:
in org.eclipse.jdt.internal.core.JavaElement.hashCode() extract the value:
sometimes also paired with BadLocationException ( but i am more concerend about the wrong source code generated):
the source code generated also varies. I tried "rebuild index" and do it again while indexing resulted in (UI freeze and) another order:
Then i tried it again - sucess:
tried it again - error again (indexing did not help!):
that hurts
The text was updated successfully, but these errors were encountered: