RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages
John Hendrikx
jhendrikx at openjdk.org
Sun Jan 1 15:06:55 UTC 2023
On Wed, 28 Dec 2022 18:57:23 GMT, Nir Lisker <nlisker at openjdk.org> wrote:
>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> modules/javafx.base/src/main/java/com/sun/javafx/binding/BidirectionalContentBinding.java line 81:
>
>> 79: if ((obj1 instanceof ObservableList<?> list1) && (obj2 instanceof ObservableList<?> list2)) {
>> 80: @SuppressWarnings("unchecked")
>> 81: final ListContentBinding<Object> binding = new ListContentBinding<>((ObservableList<Object>) list1, (ObservableList<Object>) list2);
>
> Although the previous code has the same problem, this is sketchy. The two lists can be of different types while `ListContentBinding` requires the same type. This is a result of the `Bindings` public API that takes two `Objects`, so all type information is lost. Is it worth adding a comment about this since suppressing the warning can be understood as "trust me, this is fine".
What would go wrong if they're not the same type? `ListContentBinding` doesn't (and can't) enforce it and doesn't care either way. The whole function fails silently if types don't match. Also `ListContentBinding` is a private class and so I'd expect this code to be aware of how it works and what is/isn't safe to do.
I personally think this entire class is unfinished. It fails miserably in edge cases without so much as a warning to the user. Take this for example:
public static void main(String[] args) {
ObservableList<String> a = FXCollections.observableArrayList();
ObservableList<String> b = FXCollections.observableArrayList();
Bindings.bindContentBidirectional(a, b);
Bindings.bindContentBidirectional(a, b);
a.add("A");
System.out.println(a + " : " + b);
}
Prints:
[A, A, A, A] : [A, A, A]
No mention about this in the API docs at all. It breaks even worse when you make circular bindings `[a,b], [b,c], [c,a]` (stack traces get logged to the console complaining about `IndexOutOfBoundsException`).
I've created a solution that rejects double bindings and circular bindings, but it's a bit out of scope for this. I think however that it is worth adding, and considering that the current behavior is broken when doing any of such things, not a big API break if instead we throw an exception.
-------------
PR: https://git.openjdk.org/jfx/pull/972
More information about the openjfx-dev
mailing list