Create static variant of ImageDescriptor.imageDescriptorFromURI()

[ptziegler]

This method is intended to be used as a wrapper for createFromURL() to avoid having to deal with the checked MalformedURLException. However, this method is effectively unusable, as it requires the user to already have an instance of the ImageDescriptor they want to created.

Instead, create a new, static createFromURI() method, mark the old method as deprecated and internally delegate to the new method.

The old method was created as a response to Bug 559656 [1], in order to match the signature of IResourceUtilities. But because the latter is accessed via an OSGi service, it doesn't need to be static. Which is something that doesn't really work for image descriptors.

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=559656

[ptziegler]

@vogella You contributed the original method. wdyt?

[github-actions]

Test Results

 1 821 files  ±0   1 821 suites  ±0   1h 53m 16s ⏱️ + 7m 51s
 7 724 tests ±0   7 496 ✅ +1  228 💤 ±0  0 ❌  - 1 
24 333 runs  ±0  23 586 ✅ +1  747 💤 ±0  0 ❌  - 1 

Results for commit 7307874. ± Comparison against base commit 136afb4.

♻️ This comment has been updated with latest results.

[vogella]

LGTM, thanks @ptziegler

[laeubi]

As we are currently changing the method, catching NPE is really really BAD!
We should check for null like in createFromURL

Suggested change

	} catch (MalformedURLException | NullPointerException e) {
	} catch (MalformedURLException e) {

[ptziegler]

In e.g. createFromURL(), we have an explicit null check where getMissingImageDescriptor() is returned. I think it makes more sense to do the same here, rather than forwarding the exception.

[ptziegler]

I've also noticed that new URL(String) will be deprecated in Java 20, so I switched to the equivalent URI.toURL().

[merks]

Be very careful. It looks so innocent and obvious, bu there are cases where URI creation will thrown an exception where URL creation does not. E.g., if there is a space in the string

[vogella]

@laeubi do you still have open concerns here?

[laeubi]

Looks fine for me, using this method somewhere in the code (even in a testcase) might help to demonstrate its usefulness.

Maybe we should mention in the javadoc that this only works for URIs that can be converted to URLs?

[ptziegler]

Maybe we should mention in the javadoc that this only works for URIs that can be converted to URLs?

Adding a disclaimer sounds like a good idea.

[HannesWell]

Be very careful. It looks so innocent and obvious, bu there are cases where URI creation will thrown an exception where URL creation does not. E.g., if there is a space in the string

That's right.
But here an URI is converted into a URL and as far as I know URIs are strict regarding their content and therefore the conversion URI -> URL should be (relatively) save? But maybe I'm wrong. It's definitively not true for the opposite: URL -> URL can have many problems since URLs are not strictly validating their content.