RFR: 8289384: Fix warnings: method does not override the inherited method since it is private to a different package

Andy Goryachev duke at openjdk.org
Wed Jul 13 15:31:17 UTC 2022


On Wed, 13 Jul 2022 15:15:10 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> The fix includes:
>> - renaming of offending methods to avoid confusion
>> - explicitly declaring the offending methods as private
>
>> > I turned on errors on missing `@Override` annotations. I got over 1k errors. I think it's worth fixing it in batches.
>> 
>> missing @OverRide is a different warning. people do tend not to use this annotation, so any legacy code base usually contains lots of those. same with synthetic accessors and unused imports.
> 
> You perhaps mean that old code bases may have many places where this is not done, but new code written by professional developers without `@Override` seems highly suspect as it is a very valuable annotation, especially during refactors and method renames -- the IDE will warn you if for some reason a method is declared overridden but doesn't override anything, very handy when an overridden method was renamed and you forgot one of its subclasses where it was overridden.
> 
> Unused imports, just reorganize them one time project wide (same for `@Override`, it's a safe change).
> 
> Synthetic accessors, aside from making call stacks a little uglier, they don't really need attention at all (they're not a performance issue in modern JVM's).
> 
>> I am ok with fixing all warnings, but people who use other IDEs or the IDEs with these warnings suppressed are bound to introduce them again and again.
> 
> Missing overrides should be signaled in code reviews and/or automated. All IDE's support this and so they should just be configured properly.  In my experience, I haven't seen code with missing `@Override`s unless the code base is incredibly old -- I'm a warning fixer myself, and I prefer to keep the warning list empty, so I would certainly have noticed this.  Making JavaFX warning free would be a very good step, especially since some of the warnings can actually point to real issues.
> 
> Eclipse makes such missing annotations (or other dubious constructs) painfully obvious with its clear and direct list of errors and warnings project wide.  IntelliJ makes this a lot harder and is often (in my experience) a "source" of new warnings because people are not made aware of them explicitly enough.  That is however no excuse for allowing such warnings to accumulate in the code base.

@hjohn : 
I wholeheartedly agree.  Created JDK-8290244 to fix @override's at a later date, on a per-module basis.

-------------

PR: https://git.openjdk.org/jfx/pull/824


More information about the openjfx-dev mailing list