Test uniqueness of problem IDs #1754
Conversation
- exclude deprecated constants (from pre Java 5 time / 2004) - mark deprecation using annotation (was not possible in 2004)
|
@srikanth-sankaran @jarthana @iloveeclipse FYI. In particular let me know if you see problems with:
|
| @@ -829,25 +829,35 @@ public interface IProblem { | |||
| int AbstractMethodsInConcreteClass = TypeRelated + 333; | |||
|
|
|||
| /** @deprecated - problem is no longer generated, use {@link #UndefinedType} instead */ | |||
| @Deprecated | |||
There was a problem hiding this comment.
when those constants are not used anymore, can you please add "forRemoval"?
like
@Deprecated(forRemoval = true, since = "3.37.0 2024-03")
There was a problem hiding this comment.
The deprecation (in javadoc) is actually much older (older than the annotation type? likely older than "forRemoval" :) ) - should we still use the current version in since?
There was a problem hiding this comment.
the since refers to the forRemoval
There was a problem hiding this comment.
the since refers to the forRemoval
No,
String since
Returns the version in which the annotated element became deprecated.
And
boolean forRemoval
Indicates whether the annotated element is subject to removal in a future version.
There is no direct connection between both properties.
There was a problem hiding this comment.
@jukzi once more I scanned the constants in question, and found that those have been deprecated some time in the range 2005 - 2011. So proper marking would require to look up the exact JDT release for each of those constants to provide a proper since value. Setting forRemoval is a separate strategic decision altogether.
In this PR I only want to make the existing deprecations (javadoc) visible during a test (using the annotation). Can't we think of this as orthogonal to the issue of improving the details of deprecation? This PR should serve only one purpose: avoid ambiguous IDs of "active" problems, ignoring those that are no longer raised.
There was a problem hiding this comment.
My desire is to actually have unused code to be removed in the future. That should only be done 2 years after announcing the removal. For that purpose it does not matter when exactly deprecation happened, but it should have the forRemoval flag 2 years in front. If you do not feel confident about adding a rough date it is fine if you just ignore my request to add a date (or the forRemoval at all) and leave that burden for someone else in the future. In doubt we just live with the unused constants forever. Wont be of any problem.
Still the easiest and widely accepted solution (yes, slightly dirty, but most helpful) would be to temporarily just use the current's release version/date and delete it after 2 years.
What it does
Whenever we add a new constant to
IProblemit's getting harder by the day, to find a definitely unused ID (since new IDs are allocated in different ranges, and constants are not strictly sorted).So rather than breaking clients by renumbering, I simply wrote a reflective test that ensures that any new constant doesn't share the ID with an existing one.
This was first requested in https://bugs.eclipse.org/bugs/show_bug.cgi?id=509643.
Initially, my test did detect some existing duplicates, where it turned out, that one in each pair was
@deprecated(in javadoc). In order to make this visible via reflection, I'm also adding the@Deprecatedannotation (which was not available in JDT/Core at the time of deprecating pre-1.5 problems).How to test
Add a duplicate to see the test fail :)