RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

Andy Goryachev angorya at openjdk.org
Mon Dec 5 21:20:48 UTC 2022


On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> - 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:
> 
>   Fix indentations and merge short lines

1 revert and a follow-up bug is requested.

This is one giant pull request, somewhat difficult on the reviewers, especially since we have to go through it several times.

It would be *much* easier for the reviewers to deal with one fix per PR, especially in the cases where more than 30 files are going to be touched.

For example, changes involving diamond operator and missing @overrides could have been made into two separate PRs, making it a breeze to review and integrate, while also cutting down on the number of files which contained non-trivial changes.

What do you think?

modules/javafx.graphics/src/main/java/com/sun/javafx/application/LauncherImpl.java line 601:

> 599:             }
> 600:             args.add("--" + k + "=" + (v != null ? v : ""));
> 601:             idx++;

I suspect there might be a deviation from original intent that had happened earlier.  The original idea seems to be of capturing IOException and sending some diagnostic output to stderr, then continuing.  I am guessing here, but perhaps at some point in the past `Base64.getDecoder().decode()` replaced whatever was there before - and instead of throwing `IOException` it now throws `IllegalArgumentException` if things cannot be parsed.

So the try-catch block should remain, catching an `IllegalArgumentException` instead.

Perhaps we ought to revert these changes and file a follow-up bug?

What do you think?

modules/javafx.graphics/src/main/java/com/sun/javafx/css/PseudoClassState.java line 172:

> 170: 
> 171:     static final List<PseudoClass> pseudoClasses =
> 172:             new ArrayList<>();

minor: if you make further changes, these two can be on one line.

modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java line 586:

> 584:                 ascent = -(float)hhea.getShort(4);
> 585:                 descent = -(float)hhea.getShort(6);
> 586:                 linegap = hhea.getShort(8);

interesting: why not on the previous 2 lines?  isn't

`-(float)shortValue == (float)(-shortValue)` ?

modules/javafx.graphics/src/main/java/com/sun/prism/d3d/D3DContext.java line 514:

> 512:             case D3DERR_DEVICEREMOVED:
> 513:                 return "D3DERR_DEVICEREMOVED";
> 514:             case D3D_OK:

this change is probably correct, but one can think of a theoretical possibility of collision, i.e.

`hResult = ((1L << 32) + D3DERR_DEVICENOTRESET)` will hit `D3DERR_DEVICENOTRESET` case instead of the default one.

but again, hResult is long perhaps because it's originally unsigned32 or something like that.

modules/javafx.graphics/src/main/java/javafx/css/CssParser.java line 2579:

> 2577:             term = nextLayer(lastTerm);
> 2578:         }
> 2579:         return new ParsedValueImpl<>(layers, CornerRadiiConverter.getInstance());

My eclipse has a problem with this class (might be an Eclipse bug).  It does compile, but any time I modify the class it generates a bunch of errors.  The only code that does not produce the errors is returning a raw value.

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

Changes requested by angorya (Committer).

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


More information about the openjfx-dev mailing list