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