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

Use @SafeVarargs annotation instad of multiple fixed-arity factory methods that overload varargs method #83

Closed
npryce opened this issue Dec 23, 2014 · 7 comments

Comments

@npryce
Copy link
Contributor

npryce commented Dec 23, 2014

The overloaded methods were written to reduce compile warnings in client code. The @SafeVarargs annotation that was introduced in Java 7 suppresses them altogether.

This will change the ABI, but code will still be source compatible.

@npryce npryce added this to the 7.0 milestone Dec 23, 2014
@npryce npryce changed the title Replace fixed-arity factory methods that overload varargs with @SafeVarargs annotation Use @SafeVarargs annotaiton instad of multiple fixed-arity factory methods that overload varargs method Dec 23, 2014
@npryce npryce changed the title Use @SafeVarargs annotaiton instad of multiple fixed-arity factory methods that overload varargs method Use @SafeVarargs annotation instad of multiple fixed-arity factory methods that overload varargs method Dec 29, 2014
@sf105 sf105 modified the milestones: 2.0.0.0, 2.1.0.0 Dec 30, 2014
@sf105 sf105 closed this as completed Dec 30, 2014
@lukeu
Copy link

lukeu commented Feb 4, 2016

I'm currently hitting warnings with IsIterableContainingInOrder.contains

Should @SafeVarArgs be added to this and other methods also? I.e. should this fix be extended to annotate methods which might not have had overloads to begin with?

(Or might it be that these methods are not actually safe to annotate this way...?)

@kwin
Copy link

kwin commented Oct 5, 2016

Even with 2.0.0.0 I still do not see the annotation on org.hamcrest.Matchers.contains(...) therefore I still get the following warning
Type safety: A generic array of Matcher<? super something> is created for a varargs parameter

@ePaul
Copy link

ePaul commented May 28, 2018

I think this was closed preliminarily (unless it was meant as a "won't fix", but then a comment would be nice).

I propose all of the varargs methods in Matchers should get this annotation, as well as the static methods they are delegating to. (Unless there are some which actually write into the array, but I don't think that is happening here.)

(I don't understand how this changes the ABI ... the method still accepts an array of the upper bound type, right?)

@kwin
Copy link

kwin commented Sep 21, 2018

@sf105 Would you consider reopening this?

@sf105 sf105 reopened this Sep 23, 2018
@sf105
Copy link
Member

sf105 commented Sep 23, 2018

Agreed. This have got too confused.

@arifogel
Copy link

arifogel commented Mar 2, 2019

What exactly will be done with this issue? It looks like the current source has the @SafeVarargs annotation:
https://github.com/hamcrest/JavaHamcrest/blob/master/hamcrest/src/main/java/org/hamcrest/Matchers.java#L886
Will there ever be another release with this change? (Release is out, but under different artifactId)
Isn't this issue technically resolved (but not by the original milestone)?

@tumbarumba
Copy link
Member

Agree, closing this.

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

No branches or pull requests

7 participants