Bug 231575 - [clean up] Convert for loop gets lost with array being qualified name
Summary: [clean up] Convert for loop gets lost with array being qualified name
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.4 RC1   Edit
Assignee: Benno Baumgartner CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-05-12 10:48 EDT by Jacek Pospychala CLA
Modified: 2008-05-21 11:02 EDT (History)
2 users (show)

See Also:
martinae: review+


Attachments
proposed fix (1015 bytes, patch)
2008-05-12 10:48 EDT, Jacek Pospychala CLA
no flags Details | Diff
fix (2.90 KB, patch)
2008-05-13 05:17 EDT, Benno Baumgartner CLA
no flags Details | Diff
r322_v20070124-patch (997 bytes, patch)
2008-05-21 10:56 EDT, Jacek Pospychala CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Pospychala CLA 2008-05-12 10:48:30 EDT
Created attachment 99720 [details]
proposed fix

With 3.4M7 and following code:

public class MyArray2 {

	private Object[] array;
	
	public void method() {
		MyArray2 copy = null;
		
		for (int i = 0; i < copy.array.length; i++) {
			array[i].equals(null);
		}
	}
}

Using "Clean Up" with only option "Convert for loops to enhanced for loops", I get:

public class MyArray2 {

	private Object[] array;
	
	public void method() {
		MyArray2 copy = null;
		
		for (Object element : copy.array) {
			element.equals(null);
		}
	}
}

Should be: no conversion, as index variable is used to iterate other array.
Comment 1 Jacek Pospychala CLA 2008-05-12 10:51:13 EDT
Note, analogous case when loop array is simple name (e.g. "array"), and access inside the block is qualified name (e.g. "copy.array") works correctly now.
Comment 2 Benno Baumgartner CLA 2008-05-13 04:42:32 EDT
Major, silent semantic change, let's fix it for 3.4...

Fix for bug 214340 was not good enough.
Comment 3 Benno Baumgartner CLA 2008-05-13 05:17:10 EDT
Created attachment 99909 [details]
fix

proposed fix + test case
Comment 4 Benno Baumgartner CLA 2008-05-13 05:19:46 EDT
Martin? Can you please review? Is this an external contribution?
Comment 5 Martin Aeschlimann CLA 2008-05-13 08:43:48 EDT
Patch looks good.

I think we only need to set the 'contributed' keyword on this bug as Jacek is not a 'Eclipse project' contributer (is this correct, Jacek?). No change to the copyright is required as he is included in the 'IBM' comment.
Comment 6 Jacek Pospychala CLA 2008-05-13 08:53:05 EDT
(In reply to comment #5)
> Patch looks good.
> 
> I think we only need to set the 'contributed' keyword on this bug as Jacek is
> not a 'Eclipse project' contributer (is this correct, Jacek?). No change to the
> copyright is required as he is included in the 'IBM' comment.

Yes i fall under IBM umbrella.
And am only PDE incubator committer, so in all other projects I must be contributor. :)

However for example PDE sets "contributed" and comment in copyright, so it's up to you and JDT rules.
I'm ok with whatever you set, as long as bug is fixed.
Comment 7 Benno Baumgartner CLA 2008-05-13 10:13:48 EDT
Fixed without changes to copyright header > I20080510-2000

Thanks for the patch and the bug report!
Comment 8 Martin Aeschlimann CLA 2008-05-19 06:07:09 EDT
start verifying
Comment 9 Martin Aeschlimann CLA 2008-05-19 06:32:28 EDT
verified in I20080516-1333
Comment 10 Jacek Pospychala CLA 2008-05-21 10:56:49 EDT
Created attachment 101281 [details]
r322_v20070124-patch

patch for the same issue against Eclipse 3.2.2 (r322_v20070124).
Comment 11 Martin Aeschlimann CLA 2008-05-21 11:02:20 EDT
We no plans to back port this to 3.2.2...