RFR: JDK-8323706 Move SimpleSelector and CompoundSelector to internal packages [v2]

[John Hendrikx]

On Fri, 19 Jan 2024 10:21:05 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> Moves `SimpleSelector` and `CompoundSelector` to internal packages.
>> 
>> This can be done with only a minor API break, as `SimpleSelector` and `CompoundSelector` were public before.  However, these classes could not be constructed by 3rd parties.  The only way to access them was by doing a cast (generally they're accessed via `Selector` not by their sub types).  The reason they were public at all was because the CSS engine needs to be able to access them from internal packages.
>> 
>> This change fixes a mistake (or possibly something that couldn't be modelled at the time) when the CSS API was first made public. The intention was always to have a `Selector` interface/abstract class, with private implementations (`SimpleSelector` and `CompoundSelector`).
>> 
>> This PR as said has a small API break.  The other changes are (AFAICS) source and binary compatible:
>> 
>> - Made `Selector` `sealed` only permitting `SimpleSelector` and `CompoundSelector` -- as `Selector` had a package private constructor, there are no concerns with pre-existing subclasses
>> - `Selector` has a few more methods that are now `protected` -- given that the class is now sealed, these modified methods are not accessible (they may still require rudimentary documentation I suppose)
>> - `Selector` now has a `public` default constructor -- as the class is sealed, it is inaccessible
>> - `SimpleSelector` and `CompoundSelector` have a few more `public` methods, but they're internal now, so it is irrelevant
>> - `createMatch` was implemented directly in `Selector` to avoid having to expose package private fields in `Match` for use by `CompoundSelector`
>> - No need anymore for the `SimpleSelectorShim`
>
> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add since tags

> Hmm. This suggests taking a step back. I think there are only two choices that make sense (there are a couple others, but I don't think they are useful).
> 
> 1. Abandon the notion of making an incompatible change to `SimpleSelector` and `CompoundSelector`. This means that PR [JDK-8322964 Optimize performance of CSS selector matching #1316](https://github.com/openjdk/jfx/pull/1316) will need to preserve the existing `SimpleSelector` method `List<String> getStyleClasses()` and `Set<StyleClass> getStyleClassSet()`, deprecating them with simple deprecation (not for removal).
> 2. Make an incompatible change, meaning that at some point, the existing SceneBuilder binaries will break. We could make the transition a little less painful by providing replacement API in a release that still has the existing API (in fact, I think if we go with this option, then we _must_ do that), but the fact remains that there will then be no way to have a single binary that runs on JavaFX 21 and JavaFX 23 short of a multi-release jar file, which really isn't a good solution for this sort of problem.
> 
> Preserving the existing `SimpleSelector` methods is fairly easy and already done. So, is there any real drawback to doing this? I can't think of any.

Does the comment from José Pereda here https://github.com/openjdk/jfx/pull/1340#issuecomment-1900843857 make any difference in that regard?  If scene builder is always tied to a specific FX release, then I would think we could still go ahead.

The drawback might be that more code may start relying on these classes, or that it will be harder to evolve the CSS API if more than just Simple/CompoundSelector are needed at some point.  The API still seems to have been designed with the idea that `Selector` is the only public class.

I think regardless adding a public API to get the style classes is a good idea, it seems to round out the API a bit, and it would make Scene Builder less dependent on what we consider internals in the future.

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

PR Comment: https://git.openjdk.org/jfx/pull/1333#issuecomment-1900949541