Add object factories to ISelection interface

[laeubi]

Currently creating a IStructuredSelection requires to reference a concrete implementing class, also handling null is explicitly required and only List or Array are supported.

This adds some new object factory methods similar to what users are used to with the Optional.of(...) / List.of(...) and similar and adds some usages of the new methods.

[github-actions]

Test Results

 1 812 files  + 2   1 812 suites  +2   1h 30m 25s ⏱️ -33s
 7 613 tests ± 0   7 384 ✅ ± 0  228 💤 ±0  1 ❌ ±0 
23 991 runs  +18  23 241 ✅ +18  749 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 56df319. ± Comparison against base commit 28f0a63.

♻️ This comment has been updated with latest results.

[mickaelistria]

I don't see much added-value here, and while I understand this is an intent to make things simpler, I'm afraid it can introduce some confusion about how to handle null or array objects or whether other types are permitted, while the current API makes it very explicit what to expect.
This also gives the impression to end-users that the IStructuredSelection is the only entry point to build a structured selection, while looking at the class hierarchy shows multiple implementations existing (eg TreeSelection), and encourages anyone who needs it to build a new implementation.
So it cannot be a 1-1 replacement and it's usually not more efficient to use it over the existing constructs which give more transparency about what is going to happen and what is possible.

[laeubi]

I'm afraid it can introduce some confusion about how to handle null or array objects or whether other types are permitted, while the current API makes it very explicit what to expect

Why should it be confusing? We can improve the javadoc if there is any uncertainty.

This also gives the impression to end-users that the IStructuredSelection is the only entry point to build a structured selection while looking at the class hierarchy shows multiple implementations existing (eg TreeSelection), and encourages anyone who needs it to build a new implementation.

I don't think a user is encouraged to build an own one and effectively TreeSelection is the only one in whole platform, not even in a testcase there is a single custom implementation so it seems there is rarely a need to implement an own, so I really don't se a problem here, e.g. there are numerous Map/List implementations in the JDK still Map.of(...) are of use for the common case and the same is here with the IStructuredSelection.

So it cannot be a 1-1 replacement and it's usually not more efficient to use it over the existing constructs which give more transparency about what is going to happen and what is possible.

It is a 1:1 replacement for the 90% case (2156 references to StructuredSelection versus 229 to TreeSelection versus 0 other implementations), still users are not limited to anything.

[mickaelistria]

Referencing concrete implementation is not always a bad thing. Or are you aware of a particular case where it is an issue here?
I think this change introduce no functional nor technical benefits over the current usage of the StructuredSelection constructors, and brings some (minor) concerns I mentioned earlier. Overall I don't see it as a positive and profitable addition in Platform. I may be missing some rationale to support it.

[laeubi]

The benefit is that I (as a user) compared to using the (only) implementation

• I never care about the underlying implementation, but need to remember it (well its rather trivial here but ...) and it easily let me depend on the impl (e.g. if assigned to a local variable JDT always suggest the concrete type never the interface).
• It misses some convenient ways (e.g. it only allow List as the only collection Type) to construct such object, it can not handle null (so one always has to add checks if in doubt) and that's something that can't change without breaking the existing contracts (and getting error because null is ambiguous)
• On many places factories on interfaces are common and programmers get used to it (List.of() / Path.of() / and so on..)
• On the other hand this neither limits nor breaks anything if even we one time invent a "better" StructuredSelection2 Impl I not need to change anything in my code (if I use the factory).

[laeubi]

This also makes some trivial performance / memory optimizations possible, e.g. for empty arrays/collections this always return StructuredSelection.EMPTY instead of creating a new object + a copy of the array.

[mickaelistria]

For the reason mentioned above, more importantly the one that tells that ISelection is a publicly extensible type and not just a facade and that IMO factories on interfaces don't fit in that case as those lead users to false impressions of a single constrained entry point, I cannot give a positive review here.
I suggest we ask project leads to take a decision here.