Make DefaultPluginDescriptor#addDependency usable

[rreich]

Do not use Collections.emptyList() to create empty lists that should be writable,
otherwise an UnsupportedOperationException is thrown when you try to add something to that list.

This is especially annoying as it is not easy to fix in a subclass because of the asynchronous setter and getter of dependencies (Is that really a good idea?).

[rreich]

First of all, thanks for your great library!
I just stumbled across this little bug and thought I create a quick PR - I didn't create an extra issue though. Hope that's ok.
Cheers,
Rainer

[decebals]

I use method Collections.emptyList() intentionally and I know that the returned list is immutable.
Do you have an use case when you need a non immutable list?

[rreich]

Yes I have a usecase. We extended the framework a bit and introduced something we called "add-ons", which are plugins that extend other plugins. To keep changes minimal, I add dependencies of add-ons programatically to the extended plugins to make use of the built-in plugin resolution / ordering, which works nicely.

But besides my usecase, you already provide the method addDependency(), which does not work when the dependencies list is empty. If you do not want to allow adding dependencies, this method should be removed (which of course is not my intent).

[decebals]

But besides my usecase, you already provide the method addDependency(), which does not work when the dependencies list is empty. If you do not want to allow adding dependencies, this method should be removed (which of course is not my intent).

@rreich @josiahhaswell Can we reduce the visibility of DefaultPluginDescriptor#addDependency() from public in protected?
For the moment I think that I will merge this PR, to not break the API, but in the future I think that I prefer to make DefaultPluginDescriptor immutable. What do you think?

[rreich]

It wouldn't be a problem for me, if addDependency() was protected, as I have my own subclass anyway and can extend visibility there.

If the descriptors were immutable, I could still subclass it and allow adding dependencies, storing the additional dependencies in the subclass, so it shouldn't be a big problem for me - at least if you don't make them final.

Normally the descriptors should only represent the information from the plugin's metadata, so I think it would make sense to make them immutable. Extending the dependencies of a plugin is probably a very special use-case.

I noticed one more thing, which I probably don't need after this PR is merged but I think would still be nice is, if a PluginDependency was able to convert itself back into its String representation.