Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-public types should have @API(Internal) #685

Closed
2 tasks
smoyer64 opened this issue Feb 24, 2017 · 5 comments
Closed
2 tasks

Non-public types should have @API(Internal) #685

smoyer64 opened this issue Feb 24, 2017 · 5 comments

Comments

@smoyer64
Copy link
Contributor

Overview

  • (X) Consistency.

The javadocs for @API states:

Annotates public types within JUnit to indicate their level of stability and how they are intended to be used. If the annotation is present on a type, it is considered to hold for all public members of the type as well. A member is allowed to declare a different API.Usage of lower stability.

It occurs to me that a public class with only non-public constructors is effectively internal whether or not the API annotation says so. Classes in this state could however have public static members with API annotations that are not internal-only.

Clarifying the precise rules for the API annotations use is important for the completion of #144 (which I intend to return to when I'm done with #343). One additional question is whether @API(Internal) types should show up in the "developer javadocs" (#676).

There are currently classes with package access-levels that are marked as @API(Experimental) - I've mostly noticed them in the org.junit.jupiter.engine.discovery package:

  • JavaElementsResolver
  • MethodFinder
  • TestContainerResolver
  • TestFactoryMethodResolver
  • TestMethodResolver

Deliverables

  • Clarify @API usage.
  • Mark non-public classes as @API(Internal)
@marcphilipp
Copy link
Member

marcphilipp commented Feb 24, 2017

Our original intent was to mark classes that are visible to third-party code (i.e. classes declared as public) with @API. All other classes are invisible and therefore do not need to be marked as @API(Internal). Why should we mark them?

@smoyer64
Copy link
Contributor Author

That sounds like a great clarification to the API usage and can certainly be codified into the API tools. If you take a look at JavaElementsResolver, it's not public and has no public members, yet it's marked as @API(Experimental). If I understand what you're saying correctly, that class should either have the @API(Internal) annotation or it should have the API annotation removed altogether.

If it's marked as experimental because it's not finished yet, that's more of a status than a usage. In any case, I should have some tools ready to try out on the API annotations by mid-March. Thanks for helping me better understand what the API tools should be reporting!

@marcphilipp
Copy link
Member

marcphilipp commented Feb 24, 2017

I think it's marked as experimental by mistake. Maybe it was public during development and the annotation was simply forgotten when the public keyword was removed. Having an API tool check for such illegal usage patterns would be great... 😉

Edit: I replaced "default" with "mistake" in the first sentence.

@mkobit
Copy link
Contributor

mkobit commented Sep 12, 2017

This seems done with the 5.0.0/1.0.0 releases?

@marcphilipp
Copy link
Member

Yes, I've opened apiguardian-team/apiguardian#2 to create a validation check for non-public types/methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants