RFR: 8289394: Fix warnings: Unlikely argument type

Nir Lisker nlisker at openjdk.org
Sun Jul 17 16:37:17 UTC 2022


On Fri, 8 Jul 2022 20:31:31 GMT, Andy Goryachev <duke at openjdk.org> wrote:

> Fixes "Unlikely argument type" warning generated by the latest Eclipse IDE.
> 
> This warning should be reclassified as an error, as it catches bugs missed by javac.  In this case, the following places seem to contain bugs:
> - apps/samples/3DViewer/src/main/java/com/javafx/experiments/jfx3dviewer/TimelineController.java
> - modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/GetEvent.java
> 
> The fixes include:
> - using objects of the right type
> - adding @SuppressWarnings("unlikely-arg-type") in places where intended (e.g. assertFalse)
> 
> There was an earlier discussion about using IDE-specific @SuppressWarnings in the code.  While I might disagree (I think it is ok to use IDE-specific @SuppressWarnings), the tests can be reworked to avoid @SuppressWarnings.  Please let me know.  Thanks!

Changes requested by nlisker (Reviewer).

apps/samples/3DViewer/src/main/java/com/javafx/experiments/jfx3dviewer/TimelineController.java line 79:

> 77:                 loopBtn.setDisable(false);
> 78:                 playBtn.setSelected(t.getCurrentRate() != 0);
> 79:                 loopBtn.setSelected(t.getCycleDuration().equals(Duration.INDEFINITE));

The correct fix is to check `t.getCycleCount() == Timeline.INDEFINITE`.

modules/javafx.base/src/test/java/test/javafx/collections/ObservableSubListTest.java line 161:

> 159:     @Test
> 160:     public void testEqualsOnAnotherType() {
> 161:         assertFalse(sublist.equals(Integer.valueOf(7)));

I'm not sure I see the point of this test. It's testing the implementation of the backing list, not the behavior of the sublist. Same for some other methods in this test, though it's not related to this PR. Let's see what Kevin thinks here.

modules/javafx.graphics/src/test/java/test/javafx/css/Node_cssStyleMap_Test.java line 83:

> 81:     private void checkFoundStyle(Property<?> property, Map<StyleableProperty<?>, List<Style>> map, List<Declaration> decls) {
> 82: 
> 83:         List<Style> styles = map.get((StyleableProperty<?>)property);

If the `Property` must be a `StyleableProperty` then maybe the method signature should be that and the cast should be the responsibility of the callers.

modules/javafx.graphics/src/test/java/test/javafx/css/Node_cssStyleMap_Test.java line 300:

> 298:         return false;
> 299:     }
> 300: 

I don't mind removing this method since it's unused, but probably as part of a "remove unused methods" PR.

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

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


More information about the openjfx-dev mailing list