apiguardian
apiguardian copied to clipboard
apiguardian-team
Introduce annotations to further restrict API usage
Overview
For Frameworks and libraries it is quite common to provide interfaces and classes. But not all interfaces are intended to be implemented by clients, or classes may not be intended to be instantiated. If you write plugins for Eclipse you can use the Eclipse API Annotations to express such restrictions (and the IDE supports them by generating violation errors).
It would be nice to have a common-lib/apiguardian that provides these so they may become standard.
Example
Copied directly from Eclipse API Annotations:
| Annotation Type | Description |
|---|---|
| NoExtend | Classes and interfaces tagged with this annotation are declaring they are not to be extended by clients. |
| NoImplement | Interfaces tagged with this annotation are declaring they are not to be implemented by clients. |
| NoInstantiate | Classes tagged with this annotation are declaring they are not to be instantiated by clients. |
| NoOverride | Methods tagged with this annotation are declaring they are not to be overridden by clients. |
| NoReference | Classes, interfaces, annotations, enums, methods and fields tagged with this annotation are declaring they are not to be referenced at all by clients. |
I like this idea. I can imagine some build tool plugins that would validate these restrictions. Not sure though whether it should be the same plugin as the one proposed in here or a new one.
Most of these are somehow similiar to INTERNAL, but I'd say that this is a different perspective. While API.Status only describes the lifecycle of API, these methods describe expected behaviour and some sort of library contract.
Even though you're right and it is similiar, I wouldn't say that this is breaking DRY rule.
@NoReferenceis similar toAPI.Status.INTERNAL, isn't it?
I would say they are literally exactly the same thing.
While API.Status only describes the lifecycle of API, these methods describe expected behaviour and some sort of library contract.
That's not correct.
The @API status also conveys a contract, which may eventually be enforceable by tools in combination with the consumers attribute.
If you feel that the documentation for the status does not adequately explain, then please raise an issue to address that.
Cheers
@sbrannen I wouldn't agree. These are 2 different perspectives - @API describes high-level contract about interface of the artifact, while @NoReference is used in context of a single class.
Also:
@NoReference is similar to API.Status.INTERNAL, isn't it?
I would say they are literally exactly the same thing.
isn't exactly true. If I expose an interface as public API and provide a public APIfor obtaining the implementation (which is internal), that implementation class is not a subject for @NoReference, but it is still internal, so these are NOT exactly the same.
Example: Spring Data repositories; let's forget for a moment that they are built in runtime and assume that they are "real" (not synthethic) classes. These implementations are purely INTERNAL, but they are referenced, so they aren't subjects for @NoReference.
From the JavaDoc of @NoReference:
Classes, interfaces, annotations, enums, methods and fields tagged with this annotation are declaring they are not to be referenced at all by clients. For example a method tagged with this annotation should not be called by clients. If this annotation appears anywhere other than classes, interfaces, annotations, enums, methods and fields it will be ignored.
In the Eclipse API's you will find classes that are public but not all of its methods are intended to be called by client code. E..g constructors or dispose()-methods are invoked by the framework so they must be public, but this doesn't mean clients are allowed to call them, so they a annotated with @NoReference.
Yes. I only argued that INTERNAL isn't exact equivalent of @NoReference. They may overlap, but are not replacable in 1-1 relation.
Example: Spring Data repositories; let's forget for a moment that they are built in runtime and assume that they are "real" (not synthethic) classes. These implementations are purely INTERNAL,
Why would they be internal, if clients need to use them?
/**
* Must not be used by any external code. Might be removed without prior
* notice.
*/
INTERNAL,
If those classes would have constructors, I'd understand to make those INTERNAL, but the class itself most definitely isn't.
Huh... I think that the word "used" is the source of misunderstanding here.
From my perspective such repo implementations (as well as e.g. SOAP service implementations, etc) are purely internal and are not exposed in any way to client, as he'd use interface and wouldn't care what is that implementation. Removing of these classes would be seemless, as their replacement would be provided by API owners. In this interpretation INTERNAL and @NoReference are not equivalent; "used" means "use explicitly by instantiating (in case of concrete types), subclassing (in case of abstract types), accessing (in case of fields) or calling (in case of methods and constructors)".
On the other hand, I understand that this can be interpreted as "class that is only used only in internal works of the library". In this interpretation INTERNAL and @NoReference are definitely equivalent; "used" means "is in any way touched or interacted with by 3rd party". In such case I don't know what would be the @API.Status of API elements mentioned in previous paragraph though.
And to answer your question:
Why would they be internal, if clients need to use them?
Because client is not using them explicitly, but rather via their interface. From his perspective they don't matter, but they are still provided to him.
are not exposed in any way to client, as he'd use interface and wouldn't care what is that implementation
Maybe the class has to be public due to technical reasons, but the API owner wants to make sure that nobody uses the implementation directly.
Must not be used by any external code.
To me this obviously means to refer to something by name.
Let's take the following sample interface and implementation.
public interface Animal {
void move();
}
@API(status = INTERNAL)
public class Cat implements Animal {
// omitted for readability
}
Clients should not reference Cat in any way. There should be no import, no sub classing, no instantiating, not constructor call, no reflection, nothing like that.
But given the following factory method:
public Animal create() {
return new Cat();
}
Clients should be allowed to use that one to obtain an Animal. At runtime they will obviously work with Cat instances and e.g. make them move(). But they are not directly relying on the Cat class.
You can replace @API(status = INTERNAL) with @NoReference and all statements are still true.
I totally agree and I would have provided almost identical example. I agree that Cat can be annotated with INTERNAL status, but by calling create() you'll obtain reference to Cat instance (even though for you it will be Animal), so IMO it should not be annotated with @NoReference.
so IMO it should not be annotated with @NoReference
Why not? Your source code is not referring to it, is it?
The other problem is, we can only statically check source code to obey those rules. I don't believe anybody want's to enforce them at runtime using an agent or something.
I guess it boils down to whether a reference is a reference by name in source code or a reference to an object at runtime.
Classes, interfaces, annotations, enums, methods and fields tagged with this annotation are declaring they are not to be referenced at all by clients.
OK, so this is yet another cause for misunderstanding - does "to reference" used as above mean "reference by name in source code" or "reference in runtime".
OK, now we're on the same page. All this time I was defending this annotation as non-breaking DRY, because I understood "referencing" as "referencing in runtime". Would we understand that as "reference by name", I agree that these two (annotation and API status) are totally equivalent.
I would definitely find usage for this annotation, though and still think that this kind of contract can simplify a lot of things.
Classes, interfaces, annotations, enums, methods and fields tagged with this annotation are declaring they are not to be referenced at all by clients.
It mentions classes, interfaces, annotations, enums, methods and fields all of which are constructs that you refer to by name. At runtime you have references to objects, which are not mentioned. Makes me believe we're talking about referencing by name.
Yeah, I just looked into Eclipse javadocs and I've read:
For example a method tagged with this annotation should not be called by clients
So it probably means referencing by name.
Now, we don't have to exactly copy Eclipse approach. Would you say that such annotation with "reference in runtime" semantics would be useful?
Would you say that such annotation with "reference in runtime" semantics would be useful?
It would be hard to keep track of when implementing something and even hard to enforce with tools.
Does it make sense to add those Annotations (except @NoReference). If yes, should I open a PR?
My take on those:
@NoExtend
This is usually not a problem if you're operating within a single package, since the constructor can just be package private then. See Guava's Immutable* collections. They have specific sub classes for empty, singleton, etc. But users of the library are not able to extend anything since they don't see the constructor. If the constructor has to be public for technical reasons (reflection, serialization, etc.) then it should be perfectly fine to annotate all constructors with @API(status = INTERNAL).
Even though interface technically use the extend keyword when inheriting from another interface, I'd expect this to be covered by the next one.
@NoImplement
I'm assuming the goal is to have an interface exposed but clients should rely on implementations provided by the library/framework. That sounds useful to me.
@NoInstantiate
See @NoExtend, since it can be covered by the same strategy.
@NoOverride
In most cases it should be enough to make the method final. But I could imagine cases where a method has to be non-final, but users are not meant to override it, just other parts of the owning library.
@NoReference
As discussed before, I believe this is identical to @API(status = INTERNAL)
@NoImplement and @NoOverride could be seen as the same problem. Users shouldn't try to use inheritance at all. Maybe those could be combined into a single @Final. Same semantics as the keyword and in addition can also be applied to interface.
@NoExtend: Your argument makes perfect sense for classes, but this annotation can be applied to interfaces too.
@NoOverride: For test cases it is not always possible to make a method final. Not every mocking framework supports stubbing final methods.
@NoExtend: Your argument makes perfect sense for classes, but this annotation can be applied to interfaces too.
Quoting myself:
Even though interface technically use the extend keyword when inheriting from another interface, I'd expect this to be covered by the next one.
Where I stated:
That sounds useful to me.
Not every mocking framework supports stubbing final methods.
The good ones do. Just saying...
Here's my real-world use case: I have a library whose methods return interfaces that the calling code is expected to reference by name, but should never implement in their own class or extend in their own interface. The implementation classes in my library are package protected. (Note that some of my interfaces even include default methods, so users might be even more inclined to misunderstand their purpose.)
@NoImplement as described by @whiskeysierra would be a perfect match for this.
Given that this has not seen much traction, how about splitting this into several issues?
Alternatively, if @ChristianSchwarz is still interested, he could create a PR that includes the first few annotations where there was at least rough consensus. I imagine that having something concrete to look at and review could help with the discussion.