Community
Participate
Working Groups
Build id: I20090217-2200 AbstractClass's x() method is flagged as unused. Is this the expected behaviour? I guess one could argue that it is used, albeit indirectly. ----------------- public class QuestionableUnusedMethod { public void v() { ConcreteClass tab = new ConcreteClass(); tab.x(); } private abstract class AbstractClass { public abstract void x(); } private class ConcreteClass extends AbstractClass { public void x() { } } }
I'm guessing this was introduced by bug 201912...but I'm sure you all figured that out already.
Srikanth - please take a look at this one.
(In reply to comment #2) > Srikanth - please take a look at this one. Will do. This case was correctly handled in the originally proposed patch, but seems to have fallen through the cracks subsequently.
Created attachment 126517 [details] Proposed patch & test
(In reply to comment #4) > Created an attachment (id=126517) [details] > Proposed patch & test Kent, appreciate your review and help with commit, the patch has a final line that says : \ No newline at end of file Is this OK ? FYI, this line was not touched by me at all.
Do you think we should complain about the abstract method in this case ? class QuestionableUnusedMethod { private abstract class AbstractClass { public abstract void x(); } abstract class ConcreteClass extends AbstractClass {} } Its never a bad idea to add the testcases that are close to the original one.
(In reply to comment #6) > Do you think we should complain about the abstract method in this case ? Are you talking to me, Kent? If yes, then I guess it depends if that's all that's on the classpath and no other classes in that package is calling x(). I don't know what the compiler can do so I don't know if that kind of detection is possible. :)
(In reply to comment #6) > Do you think we should complain about the abstract method in this case ? No, I don't think we should complain about unused abstract methods at all, period. Either the abstract class gets implemented in a concrete incarnation in which case it is the compiler's business to ensure that all abstract methods get implemented or if there is no concerete incarnation of it (i.e, the class), we should warn about the whole abstract class being unused (not the individual abstract methods). The class unused warning for local or private classes only of course (isOrEnclosedByPrivateType()). If I comment out the line // abstract class ConcreteClass extends AbstractClass {} we issue a warning about AbstractClass being unused locally and if I change ConcreteClass from having default visibility to being private, - we DON'T complain about AbstractClass being unused locally anymore - we DO instead complain about ConcreteClass being unused locally. I think this is the right behavior. (Ignoring for the moment the fact that the package level visibility of ConcreteClass doesn't amount to much since you cannot further subclass it outside this source file at least as things stand now due to the requirement for the enclosing instance)
Sounds good. Fix and tests released for 3.5M6
Verified for 3.5M6 using I20090310-0100.
