Summary: | classpathentry OPTIONAL attribute not honored for var entries | ||||||
---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Jared Burns <jared_burns> | ||||
Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||
Status: | VERIFIED FIXED | QA Contact: | |||||
Severity: | normal | ||||||
Priority: | P3 | CC: | jarthana, Olivier_Thomann, satyam.kandula, srikanth_sankaran | ||||
Version: | 3.4.2 | Flags: | srikanth_sankaran:
review+
|
||||
Target Milestone: | 3.6 M6 | ||||||
Hardware: | PC | ||||||
OS: | Windows XP | ||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Jared Burns
2010-01-20 00:37:39 EST
Jay, can you please take a look. I debugged into this a bit and it looks like it's happening because the optional attribute is only checked if the classpath entry's error code is the generic IJavaModelStatusConstants.INVALID_CLASSPATH. If it's a more specific code like IJavaModeLStatusConstants.CP_VARIABLE_PATH_UNBOUND, the "optional" check isn't done. From ClasspathValidation#validate(): for (int i = 0; i < rawClasspath.length; i++) { status = ClasspathEntry.validateClasspathEntry(this.project, rawClasspath[i], false/*src attach*/, true /*recurse in container*/); if (!status.isOK()) { if (status.getCode() == IJavaModelStatusConstants.INVALID_CLASSPATH && ((ClasspathEntry) rawClasspath[i]).isOptional()) continue; // ignore this entry this.project.createClasspathProblemMarker(status); } } status = ClasspathEntry.validateClasspath(this.project, rawClasspath, outputLocation); if (!status.isOK()) this.project.createClasspathProblemMarker(status); } I believe the fourth line should be checking for additional status codes. Probably INVALID_CLASSPATH, CP_CONTAINER_PATH_UNBOUND, or CP_VARIABLE_PATH_UNBOUND. (In reply to comment #2) > I debugged into this a bit and it looks like it's happening because the > optional attribute is only checked if the classpath entry's error code is the > generic IJavaModelStatusConstants.INVALID_CLASSPATH. If it's a more specific > code like IJavaModeLStatusConstants.CP_VARIABLE_PATH_UNBOUND, the "optional" > check isn't done. > Yes, that seems to be the code responsible for this behavior. Only that that particular code has been moved to ClasspathEntry#validateLibraryEntry in HEAD. I will look further into it and see if we can accommodate such a change for libraries. Created attachment 157369 [details]
Proposed Patch
Added 3 other error conditions to be exempted for optional classpath entries. Test cases added (ClasspathTests#testBug300136 and testBug300136a)
(In reply to comment #4) > Created an attachment (id=157369) [details] > Proposed Patch > > Added 3 other error conditions to be exempted for optional classpath entries. > Test cases added (ClasspathTests#testBug300136 and testBug300136a) (1) Jay, can you see if similar code is needed in org.eclipse.jdt.internal.core.ClasspathEntry.validateClasspathEntry(IJavaProject, IClasspathEntry, IClasspathContainer, boolean, boolean) (search for 171136) (2) Do you think it is reasonable to simply say if (((ClasspathEntry) entry).isOptional()) return JavaModelStatus.VERIFIED_OK; (In reply to comment #5) > > (1) Jay, can you see if similar code is needed in > org.eclipse.jdt.internal.core.ClasspathEntry.validateClasspathEntry(IJavaProject, > IClasspathEntry, IClasspathContainer, boolean, boolean) > > (search for 171136) The other place uses ClasspathEntry#validateLibraryEntry, which is specific for CPE_LIBRARY. So, I am not sure if we need to worry about the other error conditions there. > (2) Do you think it is reasonable to simply say I realized that we had to handle multiple error conditions. But I see there is at least one error condition that I wasn't sure about ignoring - NAME_COLLISION. We can probably change the check to exclude that Status. I agree that would be programmatically more efficient. But from maintenance aspect, the current approach looks more appropriate. Ok, thanks for the explanation. Released in HEAD for 3.6M6 Verified for 3.6M6 using build I20100305-101 Verified. |