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