On Tue, 22 Nov 2022 16:44:01 GMT, John Hendrikx <jhendr...@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:
>
> Revert "Fix warnings in fxml"
>
> This reverts commit b148aa3cc8a4676167a9eb8a023cddce3de116e7.
Changes requested by angorya (Author).
General comment: there are many places where the test seem to require the
StubToolkit:
tk = (StubToolkit)Toolkit.getToolkit();//This step is not needed (Just to make
sure StubToolkit is loaded into VM)
The question is - do we really need the StubToolkit there, and if so, we
probably should create a method in Util, something like Util.loadStubToolkit().
If not, then the proposed changes are ok.
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/FXVKSkin.java
line 750:
> 748: protected void sendKeyEvents() {
> 749: Node target = fxvk.getAttachedNode();
> 750: if (chars != null) {
not functionally equivalent:
(instanceof EventTarget) here is equivalent to
(!= null)
modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/skin/FXVKSkin.java
line 858:
> 856: protected void sendKeyEvents() {
> 857: Node target = fxvk.getAttachedNode();
> 858:
same comment - check for null?
modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java line
2114:
> 2112:
> this.tableView.itemsProperty().addListener(itemsPropertyListener);
> 2113:
> 2114: selectedCellsMap = new SelectedCellsMap<>(c ->
> fireCustomSelectedCellsListChangeEvent(c)) { // Note: use of method
> reference causes javac compilation error (see JDK-8297428)
+1
modules/javafx.controls/src/test/java/test/javafx/scene/control/AccordionTest.java
line 57:
> 55:
> 56: @Before public void setup() {
> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
not equivalent.
I wonder if we need an assertTrue here
modules/javafx.controls/src/test/java/test/javafx/scene/control/ButtonTest.java
line 79:
> 77: @Before public void setup() {
> 78: btn = new Button();
> 79: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
assertTrue?
modules/javafx.controls/src/test/java/test/javafx/scene/control/CheckMenuItemTest.java
line 53:
> 51:
> 52: @Before public void setup() {
> 53: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
assertTrue?
modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxTest.java
line 80:
> 78:
> 79: //This step is not needed (Just to make sure StubToolkit is
> loaded into VM)
> 80: tk = Toolkit.getToolkit();
assertTrue?
modules/javafx.controls/src/test/java/test/javafx/scene/control/ColorPickerTest.java
line 53:
> 51:
> 52: @Before public void setup() {
> 53: tk = Toolkit.getToolkit();
assertTrue?
modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboSpecialKeyTest.java
line 163:
> 161: // This step is not needed (Just to make sure StubToolkit is
> 162: // loaded into VM)
> 163: tk = Toolkit.getToolkit();
assertTrue?
modules/javafx.controls/src/test/java/test/javafx/scene/control/CustomMenuItemTest.java
line 55:
> 53:
> 54: @Before public void setup() {
> 55: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
or maybe create a Util.loadStubToolkit()
modules/javafx.controls/src/test/java/test/javafx/scene/control/DefaultCancelButtonTestBase.java
line 316:
> 314: //This step is not needed (Just to make sure StubToolkit is
> loaded into VM)
> 315: @SuppressWarnings("unused")
> 316: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/MenuBarTest.java
line 75:
> 73: setUncaughtExceptionHandler();
> 74:
> 75: tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/PaginationTest.java
line 69:
> 67: @Before public void setup() {
> 68: pagination = new Pagination();
> 69: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/RadioMenuItemTest.java
line 59:
> 57:
> 58: @Before public void setup() {
> 59: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/ScrollBarTest.java
line 55:
> 53:
> 54: @Before public void setup() {
> 55: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/ScrollPaneTest.java
line 57:
> 55:
> 56: @Before public void setup() {
> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/SeparatorMenuItemTest.java
line 52:
> 50:
> 51: @Before public void setup() {
> 52: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/SliderTest.java
line 57:
> 55:
> 56: @Before public void setup() {
> 57: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/SpinnerTest.java
line 1384:
> 1382: boolean escapeCancelPass = false;
> 1383: @Test public void testEnterEscapeKeysWithDefaultCancelButtons() {
> 1384: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/SpinnerTest.java
line 1462:
> 1460: // Test for JDK-8185937
> 1461: @Test public void testIncDecKeys() {
> 1462: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/SplitPaneTest.java
line 77:
> 75:
> 76: @Before public void setup() {
> 77: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TabPaneTest.java
line 102:
> 100:
> 101: @Before public void setup() {
> 102: tk = Toolkit.getToolkit();//This step is not needed (Just to
> make sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TabTest.java
line 64:
> 62:
> 63: @Before public void setup() {
> 64: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
line 393:
> 391: // Test for JDK-8178417
> 392: @Test public void caretPositionUndo() {
> 393: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
line 1942:
> 1940: // Test for JDK-8178418
> 1941: @Test public void UndoRedoSpaceSequence() {
> 1942: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
line 1981:
> 1979: // Test for JDK-8178418
> 1980: @Test public void UndoRedoReverseSpaceSequence() {
> 1981: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
line 2028:
> 2026: // Test for JDK-8178418
> 2027: @Test public void UndoRedoWords() {
> 2028: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java
line 2077:
> 2075: // Test for JDK-8178418
> 2076: @Test public void UndoRedoTimestampBased() {
> 2077: Toolkit tk = Toolkit.getToolkit();
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TitledPaneTest.java
line 77:
> 75: @Before public void setup() {
> 76: node = new Rectangle();
> 77: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/ToggleButtonTest.java
line 58:
> 56:
> 57: @Before public void setup() {
> 58: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/ToolbarTest.java
line 76:
> 74:
> 75: @Before public void setup() {
> 76: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/TooltipTest.java
line 62:
> 60:
> 61: @Before public void setup() {
> 62: tk = Toolkit.getToolkit();//This step is not needed (Just to make
> sure StubToolkit is loaded into VM)
toolkit
modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/ProgressBarSkinTest.java
line 67:
> 65: private void initStage() {
> 66: //This step is not needed (Just to make sure StubToolkit is
> loaded into VM)
> 67: Toolkit tk = Toolkit.getToolkit();
toolkit
-------------
PR: https://git.openjdk.org/jfx/pull/959
