Bug 468454 - Proposal for loop variable resolution of enhanced for-loop fails if variable used in body
Summary: Proposal for loop variable resolution of enhanced for-loop fails if variable ...
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4.2   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2015-05-27 06:07 EDT by Dominik Wezel CLA
Modified: 2023-05-04 06:29 EDT (History)
2 users (show)

See Also:


Attachments
Correct behaviour (loop variable not used in the code) (32.62 KB, image/png)
2015-05-27 06:10 EDT, Dominik Wezel CLA
no flags Details
Wrong behaviour (loop variable used inside the loop body, or even after) (54.17 KB, image/png)
2015-05-27 06:10 EDT, Dominik Wezel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Wezel CLA 2015-05-27 06:07:08 EDT
When I write code, the pattern

for (var: collection) {
    ...
}

is quite a common situation.  Pressing Alt-1 on the loop header

for (entry: getMap().entrySet()) {
    ...
}

correctly produces the proposal

for (Map.Entry<K, V> entry: getMap().entrySet()) {
   ...
}

as evidenced in attachment 1 [details].

If the loop variable however appears inside the code following the loop header (event if outside the loop body, and particularly when the variable is involved in an expression where its type is not 100% clear, then the code completion is no longer able to produce the correct proposal and instead yields

Object entry;
for (entry: getMap().entrySet()) {
   ...  entry ...
}

as evidenced in attachment 2 [details].

This is strange, because the loop variable should be primarily derived from the expression it gets assigned from.  I can see why this happens when the variable is in use, but the influence of the USE of the variable should have much less weight than the ASSIGNMENT in the loop head, particularly if the use does NOT contradict the assignment.  But even then will I rather adjust the use of the variable later on than cast or change the assignment (particularly when I invoke code completion on the loop header).
Comment 1 Dominik Wezel CLA 2015-05-27 06:10:00 EDT
Created attachment 253831 [details]
Correct behaviour (loop variable not used in the code)
Comment 2 Dominik Wezel CLA 2015-05-27 06:10:43 EDT
Created attachment 253832 [details]
Wrong behaviour (loop variable used inside the loop body, or even after)
Comment 3 Jay Arthanareeswaran CLA 2015-07-07 01:10:19 EDT
Moving to UI for comment.
Comment 4 Eclipse Genie CLA 2019-04-02 00:30:43 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 5 Stephan Herrmann CLA 2019-04-02 08:39:12 EDT
Still relevant in today's HEAD.

The problem is in org.eclipse.jdt.internal.ui.text.correction.proposals.NewVariableCorrectionProposal.doAddLocal(CompilationUnit):

As soon as a reference to the undeclared variable exists, the ASTNode 'dominant' is set to the EnhancedForStatement, causing the first part of this test to fail:

	} else if ((dominant != dominantStatement) && isEnhancedForStatementVariable(dominantStatement, node)) {

Some research on why the first condition should be relevant:

This line is unchanged from its initial commit via bug 167472 (does that bug have an explanation? I didn't check).

A corresponding check for old for statements was edited via https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=3608d8efb7f9fbeb85f62a2bdeaaae806580ef4e (commit comment "fix"). The only clue we get here is from the changes in 
UnresolvedVariablesQuickFixTest.java

I quickly tested removing the first part of the condition, which fixed the main issue in this bug. If a same named variable was already declared in the enclosing block, an illegal duplicate variable will be created.

It should be fairly straight forward to pick it up from here: analyze the associated tests, see what breaks if the offending check is removed (in either branch ForStatement and/or EnhancedForStatement).

Any volunteer?
Comment 6 Eclipse Genie CLA 2021-03-24 09:08:55 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.
Comment 7 Eclipse Genie CLA 2023-05-04 06:29:50 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.