Streamline E4 CSS Core and E4 CSS SWT and remove URI re-implementation

[HannesWell]

Replace the copy of org.eclipse.emf.common.util.URI the default Java java.net.URI.
A noticable main difference between both, except for some different APIs, is that the EMF URI does not require the URI spec to be encoded while java.net.URI has this requirement.

But since for example OSGi bundle names also only support a limited set of characters (see https://docs.osgi.org/specification/osgi.core/8.0.0/framework.module.html#framework.module.bsn), I assume we can expect that encoding is never necessary.

@merks what do you think?

[vogella]

Nice cleanup, deleting ~3000 lines of vendored EMF URI is a clear win. Two small things worth tightening on the java.net.URI swap:

1. CSSPropertyTabRendererSWTHandler.getPathSegments: for an empty path split("/") returns [""] (length 1), which then falls into the bundle.loadClass("") branch. Old EMF segmentCount() was 0 here. Worth a path == null || path.isEmpty() short-circuit.
2. CSSPropertyThemeElementDefinitionHandler.getBundle: uri.getPath().split("/") will NPE on an opaque URI where getPath() is null. A cheap String path = uri.getPath(); if (path == null) return null; would harden it.

Maybe wait for the next release which such a change? I also have huge CSS branch for cleanup (IIRC removes 20 000 - 30 000 lines of code) for the next release pilled up.

[merks]

Wow, a copy of URI. What a bad idea. It's a copy before URI was ultra-performance-tuned. And this in the context of e4 where the model uses EMF anyway. Sigh. Of course every time we convert something to use java.net.URI we have "fun" new problems, so probably @vogella's warning is a good one.

[github-actions]

Test Results

   864 files  ±0     864 suites  ±0   52m 55s ⏱️ - 7m 16s
 7 988 tests ±0   7 745 ✅ +1  243 💤 ±0  0 ❌  - 1 
20 418 runs  ±0  19 763 ✅ +1  655 💤 ±0  0 ❌  - 1 

Results for commit 0fe9266. ± Comparison against base commit 50c75ef.

♻️ This comment has been updated with latest results.

[HannesWell]

1. CSSPropertyTabRendererSWTHandler.getPathSegments: for an empty path split("/") returns [""] (length 1), which then falls into the bundle.loadClass("") branch. Old EMF segmentCount() was 0 here. Worth a path == null || path.isEmpty() short-circuit.

As far as I understand the code, an ArrayIndexOutOfBounds exception would be thrown if the path was empty with the old implementation? It was only checked if !(segmentCount() > 1), but that's not the case for one and zero segments.
Therefore I assume an empty path was already an error before. But we could throw a more meaningful exception in that case.

2. CSSPropertyThemeElementDefinitionHandler.getBundle: uri.getPath().split("/") will NPE on an opaque URI where getPath() is null. A cheap String path = uri.getPath(); if (path == null) return null; would harden it.

Yes, that's reasonable.

Maybe wait for the next release which such a change?

Yes, absolutely. That was my intention from the beginning.

Wow, a copy of URI. What a bad idea. It's a copy before URI was ultra-performance-tuned. And this in the context of e4 where the model uses EMF anyway. Sigh. Of course every time we convert something to use java.net.URI we have "fun" new problems,

Yes. I can only speculate why. Additionally the URI implementation also caches URI instances. Which is good if many URI objects are long living. But here it's only used temporarily and therefore I don't think caching saves more than it costs.
But since the contained values, are bundle or class names, I expect the values to not need encoding anyways.