On Wed, 28 Dec 2022 18:57:23 GMT, Nir Lisker <nlis...@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