JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web

[hjohn]

• 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

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

• JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web

Reviewers

• Andy Goryachev (@andy-goryachev-oracle - Committer)
• Nir Lisker (@nlisker - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/958/head:pull/958
$ git checkout pull/958

Update a local copy of the PR:
$ git checkout pull/958
$ git pull https://git.openjdk.org/jfx pull/958/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 958

View PR using the GUI difftool:
$ git pr show -t 958

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/958.diff

[bridgekeeper]

👋 Welcome back jhendrikx! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[mlbridge]

Webrevs

• 04: Full - Incremental (91e01c26)
• 03: Full - Incremental (a5bbff82)
• 02: Full - Incremental (da0439c6)
• 01: Full - Incremental (1c5e0fdd)
• 00: Full (f5046dab)

[kevinrushforth]

I think a single approving reviewer should be sufficient for this PR. Please allow enough time for anyone else who might be interested to comment on this.

[kevinrushforth]

I just noticed that this touches files under javafx.web/src/main/native. This will cause extra work for us, since we keep all of the files under javafx.web/src/main/native in sync across release families (and we will definitely not backport this entire fix). I need some time to think about this, but it might be better to do these Java files in a separate PR.

[nlisker]

Looks good. Warning numbers changes:

web: 1988 -> 646
swt: 58 -> 32
swing: 120 -> 101
media: 317 -> 180

I think you can add fxml to this PR as well since it's small enough.

[hjohn]

I just noticed that this touches files under javafx.web/src/main/native. This will cause extra work for us, since we keep all of the files under javafx.web/src/main/native in sync across release families (and we will definitely not backport this entire fix). I need some time to think about this, but it might be better to do these Java files in a separate PR.

I wasn't aware of special treatment of those files, I've reverted them so they can be done in another PR.

[kevinrushforth]

Given that some changes will require a bit more review, I'd like two reviewers.

/reviewers 2

[openjdk]

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[andy-goryachev-oracle]

looks good now

[nlisker]

An abstract comment about the "warning removals" PRs.

As I go over the code in various places to look at suspicious changes, I find some rather peculiar pieces of code. In this PR it was the non-abstract empty implementations, in the other PRs it's various other things. While I don't suggest to start rewriting code that has been working for years, I find it interesting to see how paradigms have changed since 2014 or before and how there are more expressive ways to write the same code today.

I toyed a bit with some code segments and rewrote them locally just to see what it would look like, and I was happy with the results (didn't save any of the changes since I don't plan to commit them). Just food for thought about how dealing with these warnings can reveal not only immediate bugs, but also sketchy code that makes you raise an eyebrow.

[openjdk]

@hjohn This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web

Reviewed-by: angorya, nlisker

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 7 new commits pushed to the master branch:

• fbad15d: 8295796: ScrollPaneSkin: memory leak when changing skin
• 4ad8582: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest
• d040c1f: 8297680: JavaDoc example for PseudoClass has minor typo
• 3376228: 8294809: ListenerHelper for managing and disconnecting listeners
• 67088be: 8256397: MultipleSelectionModel throws IndexOutOfBoundException
• 4a697f1: 8297130: ComboBox popup doesn't close after selecting value that was added with 'runLater'
• cce8580: 8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@andy-goryachev-oracle, @nlisker) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[nlisker]

/sponsor

[openjdk]

@nlisker The change author (@hjohn) must issue an integrate command before the integration can be sponsored.

[hjohn]

/integrate

[openjdk]

@hjohn
Your change (at version 91e01c2) is now ready to be sponsored by a Committer.

[kevinrushforth]

As I go over the code in various places to look at suspicious changes, I find some rather peculiar pieces of code.
...
Just food for thought about how dealing with these warnings can reveal not only immediate bugs, but also sketchy code that makes you raise an eyebrow.

That's an interesting observation. As you say, "food for thought". If done carefully in an area that we are intended to improve for other (functional) reasons, then it might be worth it in some cases. Probably not for most areas, though.

[nlisker]

/sponsor

[openjdk]

Going to push as commit 7cb408b.
Since your change was applied there have been 7 commits pushed to the master branch:

• fbad15d: 8295796: ScrollPaneSkin: memory leak when changing skin
• 4ad8582: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest
• d040c1f: 8297680: JavaDoc example for PseudoClass has minor typo
• 3376228: 8294809: ListenerHelper for managing and disconnecting listeners
• 67088be: 8256397: MultipleSelectionModel throws IndexOutOfBoundException
• 4a697f1: 8297130: ComboBox popup doesn't close after selecting value that was added with 'runLater'
• cce8580: 8254676: Alert disables Tab selection when TabDragPolicy REORDER is used

Your commit was automatically rebased without conflicts.

[openjdk]

@nlisker @hjohn Pushed as commit 7cb408b.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.