RFR: 8313651: Add 'final' keyword to public property methods in controls [v5]

[Kevin Rushforth]

On Tue, 22 Aug 2023 15:34:17 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> In the Control hierarchy, all property accessor methods must be declared `final`.
>> 
>> Added a test to check for missing `final` keyword and added the said keyword where required.
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review comments

The fix looks good. So does the test with a couple comments. I also noted a couple potential follow-up items.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlPropertiesTest.java line 95:

> 93:  *
> 94:  * Currently uses a list of classes, so any new Controls must be added manually.
> 95:  * Perhaps the test should scan classpath and find all the Controls automagically.

Minor: "classpath" --> "modulepath"

More interesting question: Should we do this in other modules (e.g., javafx.graphics), too? If so, that would be for a follow-up issue.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlPropertiesTest.java line 103:

> 101:     // list all descendants of Control class.
> 102:     // or perhaps collect all classes in a package as described here:
> 103:     // https://stackoverflow.com/questions/28678026/how-can-i-get-all-class-files-in-a-specific-package-in-java

Please remove the reference to stackoverflow.

modules/javafx.controls/src/test/java/test/javafx/scene/control/ControlPropertiesTest.java line 212:

> 210:     private void checkModifiers(Method m) {
> 211:         int mod = m.getModifiers();
> 212:         if (Modifier.isPublic(mod) && !Modifier.isFinal(mod)) {

Should we also check `protected` methods? If so, that would be something for a follow-up issue.

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

PR Review: https://git.openjdk.org/jfx/pull/1213#pullrequestreview-1616319317
PR Review Comment: https://git.openjdk.org/jfx/pull/1213#discussion_r1319172554
PR Review Comment: https://git.openjdk.org/jfx/pull/1213#discussion_r1319177546
PR Review Comment: https://git.openjdk.org/jfx/pull/1213#discussion_r1319181683