On Mon, 21 Nov 2022 16:57:30 GMT, Andy Goryachev <ango...@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

Reply via email to