RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

[Andy Goryachev]

On Mon, 28 Nov 2022 21:11:33 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Note: I ran into a `javac` compiler bug while replacing types with diamond operators (ecj has no issues).  I had two options, add a `SuppressWarnings("unused")` or to use a lambda instead of a method reference to make `javac` happy.  I choose the later and added a comment so it can be fixed once the bug is fixed.  I've reported the issue here: https://bugs.openjdk.org/browse/JDK-8297428
>> 
>> - Remove unsupported/unnecessary SuppressWarning annotations
>> - Remove reduntant type specifications (use diamond operator)
>> - Remove unused or duplicate imports
>> - Remove unnecessary casts (type is already correct type or can be autoboxed)
>> - Remove unnecessary semi-colons (at end of class definitions, or just repeated ones)
>> - Remove redundant super interfaces (interface that is already inherited)
>> - Remove unused type parameters
>> - Remove declared checked exceptions that are never thrown
>> - Add missing `@Override` annotations
>
> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Replace StubToolkit casts with asserts (2)

I think this looks good.
My only two suggestions would be
1. add util.loadStubToolkit() and 
2. collapse multiple lines where we can

but these are not show stoppers, I'll leave it to your discretion.

And finally, once the cleanup PRs are in, I'd like you to publish the **changes** to the Eclipse warnings configuration, i.e. let us know which warnings should be enabled.

Thank you.

modules/javafx.controls/src/main/java/javafx/scene/chart/LineChart.java line 86:

> 84:     private FadeTransition fadeSymbolTransition = null;
> 85:     private Map<Data<X,Y>, Double> XYValueMap =
> 86:                                 new HashMap<>();

can fit on one line now

modules/javafx.controls/src/main/java/javafx/scene/chart/ValueAxis.java line 526:

> 524:     private static class StyleableProperties  {
> 525:         private  static final CssMetaData<ValueAxis<? extends Number>,Number> MINOR_TICK_LENGTH =
> 526:             new CssMetaData<>("-fx-minor-tick-length",

perhaps declarations in this file can fit on two lines now?

modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 799:

> 797:     private static class StyleableProperties {
> 798:         private static final CssMetaData<Control,String> SKIN =
> 799:             new CssMetaData<>("-fx-skin",

one line?

modules/javafx.controls/src/test/java/test/javafx/scene/control/SliderTest.java line 60:

> 58:         tk = Toolkit.getToolkit();
> 59: 
> 60:         assertTrue(tk instanceof StubToolkit);  // Ensure it's StubToolkit

I still think that we should create 

public static Toolkit Util.loadStubToolkit()?

with a javadoc that explains why it is needed.

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

Marked as reviewed by angorya (Author).

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