<div dir="ltr">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:<div><br></div><div>* 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<>).</div><div><br></div><div>* 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.</div><div><br></div><div>* 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.</div><div><br></div><div>* Aside from fixing potential bugs, the result is often a more readable code since some fixes also remove clutter.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Dec 5, 2022 at 5:50 AM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank">john.hendrikx@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
On 04/12/2022 20:53, John Neffenger wrote:<br>
> On 12/4/22 8:22 AM, John Hendrikx wrote:<br>
>> - Avoiding indirect access to static methods and fields<br>
>><br>
>>         class A { static final int ONE = 1; }<br>
>>         class B { }<br>
>>         class C extends B {<br>
>>              int one = ONE;  // unqualified access to A.ONE<br>
>>         }<br>
><br>
> Did you mean the following?<br>
><br>
>     class B extends A { }<br>
<br>
Yes, my mistake.<br>
<br>
><br>
> There's a related trap in this category: inaccessible members can be <br>
> unintentionally made public by an intermediate subclass. For example:<br>
><br>
>     package pkg1;<br>
>     interface A { static final int ONE = 1; } // ONE has package access<br>
><br>
>     package pkg1;<br>
>     public class B implements A { } // ONE is now public in B<br>
><br>
>     package pkg2;<br>
>     import pkg1.B;<br>
>     class C extends B {<br>
>         int one = ONE; // ONE is accessible in pkg2.C<br>
>     }<br>
><br>
> Through this chain, members with package access (not just constants) <br>
> can inadvertently end up in a public API. There are 423 occurrences of <br>
> this in the Java API. There's a fix in JDK 20 that should let us find <br>
> them in the JavaFX API, too. I don't think there's much we can do <br>
> about it except be aware of them.<br>
><br>
> I was really surprised by this, but see the classes in the 'pkg1' and <br>
> 'pkg2' directories below for an example that you can compile and run:<br>
><br>
> <a href="https://github.com/openjdk/jdk/tree/master/test/langtools/jdk/javadoc/doclet/testIndexInherited" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/tree/master/test/langtools/jdk/javadoc/doclet/testIndexInherited</a> <br>
><br>
<br>
It is a bit unexpected indeed, I hadn't really thought about it that <br>
much. For methods it makes sense enough, as everything in an interface <br>
is always public and they are inherited and implemented in the <br>
implementation as public members. Those methods can be accessed simply <br>
because they're public, it doesn't matter that they came from an <br>
interface.  The static field however is not inherited or implemented in <br>
the sub type; perhaps it was not such a good idea to allow indirect <br>
access to static members in the first place.<br>
<br>
Hopefully IDE's will get a warning for this soon as well.<br>
<br>
--John<br>
</blockquote></div>