RFR: JDK-8304959: Public API in javafx.css.Match should not return private API class PseudoClassState [v7]

[John Hendrikx]

On Wed, 17 May 2023 21:59:58 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BitSet.java line 266:
>> 
>>> 264:     @Override
>>> 265:     public boolean addAll(Collection<? extends T> c) {
>>> 266:         if (this.getClass() != c.getClass()) {
>> 
>> same, handling of the null 'c' argument.
>> I wonder if this is intended behavior?
>
> I doubt the `Set` contract was breached here on purpose. More likely, the original implementation was never exposed, or the original developer didn't realize that `Set` has a very specific contract that you must follow, or it won't interact well with other collections.

To be clear, `BitSet` is currently exposed via `Node#getPseudoClassStates`.

I suspect it is almost never used by anyone, because there are a lot of bugs.

For example, equality won't work correctly:

     node.getPseudoClassStates().equals(Set.of( ... ));  // Always false, sets are of different types

Nor will `containsAll`:

     node.getPseudoClassStates().containsAll(Set.of( ... ));  // Always false, because Set is not a BitSet
     node.getPseudoClassStates().containsAll(null);  // Doesn't throw NPE, returns false

All of these are `Set` contract violations, that I suppose were thought to be irrelevant because it is a com.sun.* class.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1070#discussion_r1197116933