Community
Participate
Working Groups
20060426 We want to make the JUNIT_HOME variable as deprecated as we introduced a new JUnit continer. Same probably for JRE_LIB where the JRE_CONTAINER should be used. I suggest to add a new (optional) attribute to the 'classpathVariableInitializer' extension point: 'deprecated="JUNIT_HOME is deprecated, use the JUNIT container instead"' That way we can also offer quick fixes that update the classpath. See discussion in bug 137575
Unfortunately, I had not enough time to implement this requirement for 3.3 M3. Need to make decision for M4 if this bug will still stay in plan or not...
Martin, Do you expect specific status for this while validating classpath entry using JavaConventions or you just need an API method on JavaCore?
On both: JavaConventions.validateClasspath should warn if it contains a deprecated variable. But we also want renderer deprecated variables different in the seletion dialog and therefore need the info before applying it in a classpath. With the request to make varibales read-only (bug 156226), I would introduce a new class ClasspathVariableDescription JavaCore.getClasspathVariableDescription(String variable) : ClasspathVariableDescription ClasspathVariableDescription getName() : String getValue() : String isDeprecated() : boolean isModifiable() : boolean If a deprecated variable is in a classpath, the Java builder should also create a problem marker. We will then contribute a quick fix to -for example- replace the variable with a new container.
Marker creation will automatically arise from status change. Problem is that currently we do only have OK status and ERROR status. WARNING status is more tricky to achieve, though as Frederic suggested, this can be achieved through a multistatus. Not sure about ClasspathVariableDescription. This seems like a bigger change than we need for a deprecated solution anyway. Can't we simply have managed variables be able to answer the question, and if so, simply add these new APIs on classpath variable initializers ? And these would be the APIs, we would use to compute the status anyway.
Yes, instead of having the deprecated/non modifiable information on the extension point, we could also have it on the ClasspathVariableInitializer, that's a good idea. The only thing we must make sure is that ealy plugin-load is not a problem. In the variable selection UI, we would like to show all variables (an their values) and mark deprecated ones. So I guess as we already show values, the initializer would be loaded anyways.
Created attachment 56721 [details] Proposed patch So, here is the API proposal for this enhancement and bug 156226... With this patch, I've verified that if I set added attributes for JUNIT_HOME and JUNIT_SRC_HOME as follows in org.eclipse.jdt.junit project: <extension point="org.eclipse.jdt.core.classpathVariableInitializer"> <classpathVariableInitializer class="org.eclipse.jdt.internal.junit.buildpath.JUnitHomeInitializer" deprecated="JUNIT_HOME is deprecated, use the JUNIT container instead" readOnly="true" variable="JUNIT_HOME"> </classpathVariableInitializer> <classpathVariableInitializer class="org.eclipse.jdt.internal.junit.buildpath.JUnitHomeInitializer" deprecated="JUNIT_SRC_HOME is deprecated, use the JUNIT container instead" readOnly="true" variable="JUNIT_SRC_HOME"> </classpathVariableInitializer> </extension> Then, ClasspathEntry.validateClasspathEntry(...) return an OK multi-status (ie isOK() returns true) which has 2 children warning status as children (ie. each status severity is equals to IStatus.WARNING). Note that you can get the deprecated/read-only info through ClasspathInitializer using isDeprecated()/isReadOnly() or JavaCore API method isClasspathVariableDeprecated(String)/isClasspathVariableReadOnly(String) I've run JDT/Core model tests and JDT/UI tests and everything is fine with this patch. But I have not written specific tests for these enhancements yet.
- ClasspathVariableInitializer#isDeprecated() and isReadOnly() are missing "@since 3.3" tags. - JavaCore#isClasspathVariableDeprecated() and isClasspathVariableReadOnly() don't have Javadocs. Are these methods intended to be API ?
Created attachment 56879 [details] Complete proposed patch Here's implementation which should address these requirements including all feedback: - change returned multi-status to have with severity set to 'warning' when variable is deprecated or read-only (although previous version was still 'ok'). Also modified message to concatenate all status messages (separated with semi-colon - Note that OK status message is just removed as it is overridden by warning messages) - add missing javadoc and @since tags - add tests in ClasspathVariableInitializer All JDT tests are OK. Martin, please let me know if it's ok with you to release this implementation or if you think it needs more changes, thx
Created attachment 56978 [details] Fixed complete proposed patch This last fixes problem with message and code of multi-status + improve existing tests + add another one...
Created attachment 57080 [details] New proposed patch Here's a new patch written after last Martin's review. Note that ClasspathVariableInitializer is now unchanged. Only JavaCore API methods have been kept and modified to return the deprecation message: /** * Returns deprecation message of a given classpath variable. * * @param variableName * @return A string if the classpath variable is deprecated, <code>null</code> otherwise. * @since 3.3 */ public static String getClasspathVariableDeprecationMessage(String variableName) /** * Returns whether a given classpath variable is read-only or not. * * @param variableName * @return <code>true</code> if the classpath variable is read-only, * <code>false</code> otherwise. * @since 3.3 */ public static boolean isClasspathVariableReadOnly(String variableName)
Created attachment 57134 [details] Released patch Patch which will be released. It fixes last issue with read-only classpath variable which should not be warned when verified...
I filed bug 171049 against JDT/UI to use the new features.
Released for 3.3 M5 in HEAD stream.
Verified using build I20070205-1824.
Verified for 3.3 M5 using build I20070205-1824.
