Warnings

[Nir Lisker]

I have been working with John on the effort to remove warnings, so
obviously I agree with these. I'd like to add a few points:

* There is flexibility in how to tell the comp[iler to mark these possible
problems. Some warnings can be turned into errors for stricter standards,
and I think that this should be considered in parallel to the cleanup
effort. Personally, I treat "missing override annotations" as errors since
the damage potential when ignoring them is high and there's never a reason
not to treat them. Raw types is another I would consider upgrading since
for its damage potential, and the proper use cases of raw types are
extremely few (I couldn't find a single one in my codebase O(100) classes).
They are very common in JavaFX, unfortunately, but we will work on
minimizing them, which should be simple for the most part (change List to
List<>).

* There are more warnings that can be fixed, but we are doing an iterative
approach by fixing groups of warnings. With every iteration, the next step
can be seen more clearly through the dense warnings forest.

* Many warnings point at a deeper issue in the code. The simple fix is to
address the warning directly, but inspecting the reason sometimes reveals
badly written code. When I reviewed these PRs, there were several places
where I asked myself "why was it written this way to begin with?". For
example, looking at some "raw types" warnings that actually require the use
of raw types will show that the approach taken there was flawed to begin
with. While non-trivial refactoring should not be introduced in these PRs,
and there is little gain in changing some small piece of code if it works,
it's worth noting for the future.

* Aside from fixing potential bugs, the result is often a more readable
code since some fixes also remove clutter.

On Mon, Dec 5, 2022 at 5:50 AM John Hendrikx <john.hendrikx at gmail.com>
wrote:

>
> On 04/12/2022 20:53, John Neffenger wrote:
> > On 12/4/22 8:22 AM, John Hendrikx wrote:
> >> - Avoiding indirect access to static methods and fields
> >>
> >>         class A { static final int ONE = 1; }
> >>         class B { }
> >>         class C extends B {
> >>              int one = ONE;  // unqualified access to A.ONE
> >>         }
> >
> > Did you mean the following?
> >
> >     class B extends A { }
>
> Yes, my mistake.
>
> >
> > There's a related trap in this category: inaccessible members can be
> > unintentionally made public by an intermediate subclass. For example:
> >
> >     package pkg1;
> >     interface A { static final int ONE = 1; } // ONE has package access
> >
> >     package pkg1;
> >     public class B implements A { } // ONE is now public in B
> >
> >     package pkg2;
> >     import pkg1.B;
> >     class C extends B {
> >         int one = ONE; // ONE is accessible in pkg2.C
> >     }
> >
> > Through this chain, members with package access (not just constants)
> > can inadvertently end up in a public API. There are 423 occurrences of
> > this in the Java API. There's a fix in JDK 20 that should let us find
> > them in the JavaFX API, too. I don't think there's much we can do
> > about it except be aware of them.
> >
> > I was really surprised by this, but see the classes in the 'pkg1' and
> > 'pkg2' directories below for an example that you can compile and run:
> >
> >
> https://github.com/openjdk/jdk/tree/master/test/langtools/jdk/javadoc/doclet/testIndexInherited
> >
>
> It is a bit unexpected indeed, I hadn't really thought about it that
> much. For methods it makes sense enough, as everything in an interface
> is always public and they are inherited and implemented in the
> implementation as public members. Those methods can be accessed simply
> because they're public, it doesn't matter that they came from an
> interface.  The static field however is not inherited or implemented in
> the sub type; perhaps it was not such a good idea to allow indirect
> access to static members in the first place.
>
> Hopefully IDE's will get a warning for this soon as well.
>
> --John
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20221205/ef35b256/attachment.htm>