Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 341353 - invalid for loop in JEM utility method
Summary: invalid for loop in JEM utility method
Status: RESOLVED FIXED
Alias: None
Product: WTP Java EE Tools
Classification: WebTools
Component: jst.j2ee (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.3 M7   Edit
Assignee: David Williams CLA
QA Contact: Chuck Bridgham CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-30 09:20 EDT by David Williams CLA
Modified: 2011-03-30 17:15 EDT (History)
1 user (show)

See Also:


Attachments
patch to implement presumed intent (1.24 KB, patch)
2011-03-30 09:26 EDT, David Williams CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Williams CLA 2011-03-30 09:20:44 EDT
By chance, I came across a method in JavaModelListener in 
org.eclipse.jem.workbench.utility
named
protected boolean isInClasspath(IJavaProject testProject, List someJavaProjects, boolean isFirstLevel, Set visited) 

Not sure about earlier version, but in 3.7 M6 the compiler flags i++ in following 'for' statement as "dead code". I thought this was a error in compiler, but on second look, I think it is deadcode, since the 'return' is in the loop, so 'i' would never be incremented. In other words, the method only ever checks 1 java project, instead of the whole list. This seems like a bad bug, if the method is used (and, does seem to be used). 

for (int i = 0; i < size; i++) {
  javaProj = (IJavaProject) someJavaProjects.get(i);
  return isInClasspath(testProject, javaProj, isFirstLevel, visited);
}
Comment 1 David Williams CLA 2011-03-30 09:26:19 EDT
Created attachment 192184 [details]
patch to implement presumed intent

Assuming the intent is to look at whole list of Java Projects, this patch implements the for loop correctly. If the intent is really to just look at first Java Project in list, then code should be modified to note use a loop, since that's pretty confusing.
Comment 2 David Williams CLA 2011-03-30 17:15:18 EDT
I went ahead and released to head. If you think important, you might want to consider "porting" this back to maintenance stream, but I know it has been running this (incorrect) way for a long time, so may not matter? Or, even if my proposed fix is "correct", it may break some other cases just because they weren't expecting it? 

I incremented service field, so version is now 2.0.400. Apparently this, and null checks (in bug 341367) are only changes from maintenance. 

Hope you don't mind me releasing it ... I know Carl usually releases everything? ... just let me know if you want me to adhere to different procedure. 

Thanks,