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

[John Hendrikx]

On Fri, 7 Apr 2023 21:44:12 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> I'll take a closer look early next week.
>
>> I think that Match is supposed to be immutable, given the non-public constructor. Match itself will never change the set (and nothing else will) so making it observable seems unnecessary.
> 
> Agreed.
> 
>> In other words, simpleSelector.createMatch().getPseudoClasses().clear() would break the Selectors encapsulation.
>> 
>> I think it's best to close that loophole. If you agree, I can document this method that it returns an immutable set, which is also what I assumed would be the case in my other PR where I made many of these immutable.
> 
> Yes, this seems like the best solution to me.
> 
>> Essentially this means that SimpleSelector and CompoundSelector should probably be package private. 
> 
> I agree that these two classes should not have been made public when the javafx.css package was created as public API back in JDK 9.
> 
> The usual process for removing API is to deprecate it for removal in one release and then remove it in a future release, but in this case, since those classes cannot be constructed, and are never returned by any public API, any use of them would be relying on an implementation detail (via an `instanceof` and a cast, to no good purpose).
> 
> Since there is no useful way an application could be using these classes, I recommend option 1, as long as those two classes can be cleanly moved to com.sun.javafx.css. Otherwise, we could go with option 2 along with deprecating those two classes for removal.
> 
> The Specification section of the CSR would simply propose to remove those two classes. You could describe what is happening (moving them to a non-exported package) in the Solution section.

@kevinrushforth It looks like it will have to be option 2; the reason is that both `Selector` and `Match` are abstract classes (with a package private constructor).  The `CompoundSelector` and `SimpleSelector` extend this abstract class (as there is some small overlap). If `Selector` and `Match` had been interfaces, it would have allowed this.  

I couldn't find anything about binary compatibility when changing an abstract class with a package private constructor to an interface, so I tested it and I got an `IncompatibleClassChangeError` when calling `Selector Selector.createSelector(...)` method.  Apparently, the compiled method call does encode the difference between an interface and a class:

    invokestatic  #7                  // Method Selector.createSelector:(I)LSelector;

vs:

    invokestatic  #7                  // InterfaceMethod Selector.createSelector:(I)LSelector;

At the most, I could make `CompoundSelector` and `SimpleSelector` package private, or nested classes in `Selector`; they would at least be hidden then.

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

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