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

org.junitpioneer.jupiter.params.RangeClass should be package-private #564

Closed
Marcono1234 opened this issue Dec 25, 2021 · 4 comments · Fixed by #598
Closed

org.junitpioneer.jupiter.params.RangeClass should be package-private #564

Marcono1234 opened this issue Dec 25, 2021 · 4 comments · Fixed by #598

Comments

@Marcono1234
Copy link
Contributor

It appears org.junitpioneer.jupiter.params.RangeClass is only used internally as meta-annotation, however currently it is public.

This leads to the inconvenient situation where RangeClass refers to the internal Range, which might be confusing for users, see:
https://javadoc.io/doc/org.junit-pioneer/junit-pioneer/latest/org/junitpioneer/jupiter/params/RangeClass.html
Javadoc screenshot

Therefore the annotation type should probably be made package-private and its @Documented annotation should be removed.

A similar issue exists for org.junitpioneer.jupiter.params.RangeSourceArgumentsProvider which is visible through the documented annotation ArgumentsSource on the NumericTypeRangeSource classes (e.g. ShortRangeSource). However, there it cannot be easily solved without making RangeSourceArgumentsProvider public, which might not be desired, so maybe there this broken reference is acceptable.

@Michael1993
Copy link
Member

If I remember correctly, it's public so people can make their own range sources. Does the documentation have anything on that? 🤔

@Marcono1234
Copy link
Contributor Author

The documentation does not seem to mention that, and in its current form users cannot create their own range sources because neither RangeSourceArgumentsProvider nor Range or its subclasses are public.

@Michael1993
Copy link
Member

I recently did a review of our classes and it seems some of them were unnecessarily public instead of package-private... This is probably the case with RangeClass as well. It has some Javadoc and that kind of threw me off. 😓

nipafx pushed a commit that referenced this issue Feb 22, 2022
The `RangeClass` annotation is only used internally, as a
meta-annotation. It references the internal class `Range`.

Closes: #564
PR: #598
@nipafx
Copy link
Member

nipafx commented Feb 22, 2022

Thank for pointing this out, @Marcono1234. This was indeed an implementation-only class.

Re NumericTypeRangeSource ~> ArgumentsSource ~> RangeSourceArgumentsProvider: It will usually be the case that an annotation is public but the extension class implementing its behavior (linked via public annotations like @ExtendWith, @ArgumentsSource, or something like @RangeClass) is not.

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

Successfully merging a pull request may close this issue.

3 participants