How to implement a @noimplement interface correctly?

[laeubi]

PDE has an implementation of IModelChangedEvent that is implemented as ModelChangedEvent ... as the interface is marked with @noimplement API tools complain.

So my question is how one should ever "legally" implement such an interface, as of course there must be an implementation to make it useful. Or is the only way to have suppress that warning?

[merks]

Does declaring a friend help?

[laeubi]

Both classes are even in the same package!

[merks]

I see what you mean. Surely that looks more like a bug. Surely something in the same package is not a "client" to which the rule applies...

[mickaelistria]

So my question is how one should ever "legally" implement such an interface, as of course there must be an implementation to make it useful. Or is the only way to have suppress that warning?

IIRC (I think I faced similar questioning a dozen years ago) if the interface is introduced in the same version as the implementation and both are in same bundles, then PDE won't complain.
But in general, @noimplement is an external API contract, there is always an internal implementation for an interface. It would make sense that API Tools ignore this @noimplement in general for references in the same bundle.

[gireeshpunathil]

thanks @mickaelistria for the explanation, I had this question too!

so it looks like this contract is used to limit the scope of the subject interface within the bundle? but why would such a limitation exist in the first place? Java interface contracts are defined in such a way that any class that implements the interface get the benefit of the contract? or is it that we don't want the underlying capability to go outside of the bundle?

[merks]

Here's a proper functional example:

https://github.com/eclipse-platform/eclipse.platform/blob/ca9b95a3d7fc16149fc3466913b9a07c65f08f32/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/resources/IFile.java#L41-L44

There is no warning here:

https://github.com/eclipse-platform/eclipse.platform/blob/ca9b95a3d7fc16149fc3466913b9a07c65f08f32/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/File.java#L54

But there is a warning here:

https://github.com/eclipse-platform/eclipse.platform.ui/blob/c1d22a3cb5983dc5ec091af3239cd5aaad8fc85f/tools/bundles/org.eclipse.e4.tools.emf.ui/src/org/eclipse/e4/tools/emf/ui/internal/common/component/dialogs/ContributionDataFile.java#L56

I can even add a new class implementing IFile without a warning:

So as I said, I think it's a bug that PDE sees such a warning.

[akurtakov]

I think instead of relying on such "hacks" like noimplement - the interface should become "sealed" one thus explicitly allowing only the implementors the project allows.

[iloveeclipse]

the interface should become "sealed" one thus explicitly allowing only the implementors the project allows.

The problem with "sealed" is:

1. In sealed scope only allowed classes visible on classpath
2. Always breaks binary compatibility on changing sealed scope, because compiler writes switches based on "previously visible" classes in class files.

So I would not recommend use "sealed" for any externally visible API

[akurtakov]

What's the goal of @noimplement if not to allow only a small predefined list of implementors?

[laeubi]

What's the goal of @noimplement if not to allow only a small predefined list of implementors?

From my POV this is all about Eclipse API policy, e.g. there is no real way to "evolve" the API as we do not use semantic versioning on package exports, actually @noimplement is what the @Provider role is in OSGi/Semver that tells we never expect that consumer of that interface to implement it but there might be providers that are then tightly coupled to the actual version that is used.

So a provider interface is not suitable for a plugin that wants to be compatible for a large range of eclipse versions.

[merks]

The goal is to strongly suggest/recommend to the consumer that it's a bad idea to implement the interface because it's extremely likely (almost for sure) that binary incompatible changes will be made to that interface. It does not prevent the consumer from ignoring said advice. If someone wants to use sealed things for future new things, that is something to consider. If there is a suggestion here to change existing things to become sealed (decades) after the fact, I think that an extremely poor idea. It's like when you hear, "It's for your safety and comfort", yet so rarely is it actually conformable. You can already see from the IResource example that this would break in the SDK.

[szarnekow]

What's the goal of @noimplement if not to allow only a small predefined list of implementors?

To me, this is mostly a formalized documentation of the fact that the inventor of the interface did not anticipate custom implementations, yet it is technically allowed to implement these. Something like a more structured way to say, "Implement at your own risk, but think twice before doing so."

Making it technically impossible to implement these interfaces might completely get in the way of use cases that the original author didn't foresee.

[merks]

I think the bug is here:

https://github.com/eclipse-pde/eclipse.pde/blob/master/apitools/org.eclipse.pde.api.tools/src/org/eclipse/pde/api/tools/internal/builder/LeakImplementsProblemDetector.java#L55-L59

Specifically the comment says:

  // check if no implement interface is implemented and thereby leaking api
  // types from noimplement interface

So the check is testing if some API thing implements the restricted interface and is leaking that to clients. But this thing is marked @noextend so it is not leaking a @noimplement interface to API clients. So I think this needs to change:

Does that seem like the correct fix?