Bugzilla – Full Text Bug Listing
| Summary: | [compiler] Unnecessary implementation of interface in class declaration | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Vijay Aravamudhan <avijayr> | ||||||||
| Component: | Core | Assignee: | Kent Johnson <kent_johnson> | ||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | akiezun, david_audel, ggregory, markus.kell.r, philippe_mulet | ||||||||
| Version: | 3.1 | ||||||||||
| Target Milestone: | 3.4 M4 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows XP | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Vijay Aravamudhan
Are you talking about having redundant references to an interface in a type
hierarchy ?
e.g.
class X implements I {}
class Y extends X implements I {}
YEs - that is exactly what I was referring to. Indeed, this feels like a useful notification. *** Bug 84663 has been marked as a duplicate of this bug. *** *** Bug 150229 has been marked as a duplicate of this bug. *** Exploratory work makes progress. The definite code will have to be fine-tuned depending on the results of bug 192620. Created attachment 71591 [details]
Suggested implementation + test cases
This is my first attempt to implement the feature, Kent please comment.
The following are known limitations/potential further improvements, that we may want to tackle or not depending on the cost/benefit analysis of each of them:
- I have chosen not to introduce a new warning token for this (neither for
SuppressWarning, nor for the -warn option of the batch compiler); I found none
that, IMO, matches this warning; accordingly, it will be tuned from the UI
or the options file only, which I believe is acceptable;
- we may want to try to provide the user with a more detailed message, aka add
one of the super-types that already implement the redundant interface; did the
simplest thing so far;
- did not attempt to implement a similar check against interfaces themselves;
would need to climb up the extends chain; may be far less desirable a warning
anyway.
The ISV doc will need to get modified once we all agree on names etc.
A separate bug will need to be opened against UI to surface the option in the preferences.
I think we should detect redundant interfaces in the MethodVerifier as we walk up the hierarchy to find superclasses/superinterfaces. I also think we need to find redundant superinterfaces in interfaces & not just classes. (In reply to comment #8) > I think we should detect redundant interfaces in the MethodVerifier as we walk > up the hierarchy to find superclasses/superinterfaces. I cannot see the relationship to methods. Could you elaborate please? It has nothing to do with methods, but the verifier walks the hierarchy looking for inherited methods & can easily handle another warning about redundant interfaces. Sure. Yet segregating type-related checks into methods like ClassScope#checkParameterizedSuperTypeCollisions and others seems to be frequent in the current code base. Also, unless we make the default level for this to be warning instead of ignore, a separate call would likely optimize the 'default warnings' case (one single test to exclude the call - interleaving the checks into the method verifier code may call for more?) Take a look at MethodVerifier15.findSuperinterfaceCollisions() Should be able to add a warning in much the same way. Is there any progress on integrating the supplied patch to the codebase? Philippe, is this to be reassigned to Kent? Any updates on this? No not yet - hopefully in the next few weeks, I'll take another crack at it. Created attachment 82360 [details]
Proposed patch
This seems to do the trick and gives us the possibility to cache the unique superinterfaces in the verifier
Released into HEAD for 3.4M4 Added tests to GenericTypeTest and SuperTypeTest Created attachment 82461 [details]
Supplement to patch to make it suppressable
With this extra change, the new warning should be suppressable using
@SuppressWarnings("unused")
Released additional change in CompilerOptions and added AnnotationTest 239 Verified for 3.4M4 using build I20071210-1800. Closing *** Bug 75951 has been marked as a duplicate of this bug. *** |