RFR: JDK-8297332: Remove easy warnings in base

John Hendrikx jhendrikx at openjdk.org
Mon Nov 21 20:38:24 UTC 2022


On Mon, 21 Nov 2022 16:57:30 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> One suggestion I'd like to make is to publish the Compiler Errors/Warnings configuration for Eclipse. I have a set of screenshots that produce a 0 err/warn count with the current master. Eclipse also seem to provide a way to export/import these configs, but for some reason export/import stopped working for me around a decade ago.
> 
> I also noticed two things in general:
> 
> 1. the default err/warn configuration enables meaningless warnings and disables those that point to the real problems (see [JDK-8289379](https://bugs.openjdk.org/browse/JDK-8289379))

The default configuration of Eclipse is a very conservative starting point, and shouldn't really be used for anything serious about code quality.

Also, many of the warnings are not unique to Eclipse (it's just that Eclipse users notice them far easier thanks to a global problem view).  The raw type warnings, unchecked casts and many others are part of `javac` as well.

> 2. in large code bases with multiple contributors, it makes little sense to enable warnings like usage of raw type or unused imports since they a) don't contribute to code quality and b) are getting re-introduced all the time by people who don't use Eclipse

I disagree, raw types warnings are of great value.  Raw types were used in the pre Java 1.5 days, and `ClassCastException`s were a big problem then. After Java got generics, many of these can now be checked by the compiler and will result in compile errors.

        List list = new ArrayList();
        list.add(2);
        String s = (String)list.get(0);

The above code will have 0 warnings apart from raw type usage.  It is also obviously wrong and will result in a runtime exception.  This wouldn't compile if the list was typed.

Unused imports are less of a problem I agree, but some hygiene there can help.  For example, importing things just because Javadoc refers to it can point to problems in documentation where higher level concepts are referred to from a lower level abstraction.  In projects I usually work on, we tend to have a fixed import order to avoid diffs on imports.  Any diffs on imports then more clearly show you're adding/removing new dependencies -- depending on the code involved this can already be insightful to see if the code isn't doing too much or doing things it shouldn't be doing (ie. importing `java.sql.Date` instead of `java.util.Date` -- something you won't notice when only looking at the code).

However, the best way to handle keeping a code base clean with many contributors is to make it part of the build process; the first step to achieve this is to clean the code base. Once the code is nearly warning free, the build can enforce this by compiling with linting on and failing the build if there are warnings (like for Javadoc).  I think the Skara tooling might be able to reject PR's with warnings, if not, Gradle can fail builds with warnings (but that's a bit more heavy handed).

I've attached the Eclipse preferences that I've been using (please remove the txt extension that I only added to allow pasting it in this comment).
[javafx.epf.txt](https://github.com/openjdk/jfx/files/10060251/javafx.epf.txt)

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

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


More information about the openjfx-dev mailing list