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

John Hendrikx jhendrikx at openjdk.org
Fri May 19 00:05:01 UTC 2023


On Thu, 18 May 2023 23:58:28 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> The class `PseudoClassState` is private API, but was exposed erroneously in the CSS API. Instead, `Set<PseudoClass>` should have been used. This PR corrects this.
>
> John Hendrikx has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into feature/remove-private-api-in-css
>  - Override hashCode with a comment explaining why
>  - Fix style issues
>  - Restore removed public method
>  - Fix PseudoClassTest
>  - Add missing null check
>  - Fix tests, don't call addAll with null
>  - Remove unused methods
>  - Fix build by pulling in partial fix for BitSet class
>  - Ensure Set PseudoClass and StyleClass are immutable in CSS API
>    
>    Updated documentation, fixed warnings and removed some unnecessary code
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/de7bf3da...80ff9992

I will still fix it so it isn't worse than it was before, but I just found another reason why you shouldn't use `getClass() == other.getClass()` when dealing with sets:

    @Test
    void twoEmptyBitSetsShouldBeEqual() {

        /*
         * Per Set contract, the empty set is equal to any other empty set.
         */

        assertEquals(new StyleClassSet(), new PseudoClassState());
        assertEquals(new PseudoClassState(), new StyleClassSet());
        assertEquals(Set.of(), new PseudoClassState());
        assertEquals(new PseudoClassState(), Set.of());
        assertEquals(Set.of(), new StyleClassSet());
        assertEquals(new StyleClassSet(), Set.of());
    }

I will leave that for #1076 (where this test is coming from).  I'm only adding a test to verify the fix I'm adding here, and was seeing what I can use before writing new ones.  The above test passes though because it will back to `super.equals(obj)` in that case.

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

PR Comment: https://git.openjdk.org/jfx/pull/1070#issuecomment-1553807939


More information about the openjfx-dev mailing list