guava
guava copied to clipboard
google
Add SafeVarargs annotations
Original issue created by electrum on 2012-07-19 at 12:27 AM
Please add @ SafeVarargs to things like ImmutableSet.of().
Original comment posted by [email protected] on 2012-07-19 at 05:49 AM
You can be sure we've been eager to do so, but AFAIK we can't until we are ready to require JDK 1.7. Given all the trouble that it's caused when we recently started requiring 1.6, I expect this will be at least Guava 19.0 or so.
Status: Accepted
Labels: Package-General, Type-Enhancement
Original comment posted by electrum on 2012-07-19 at 06:47 AM
Wouldn't this only require 1.7 for compiling? The annotation should be ignored if the annotation definition is not available.
Original comment posted by [email protected] on 2012-07-19 at 06:50 AM
Unfortunately (or fortunately depending on how you look at it), the annotation is itself annotated with @Retention(value=RUNTIME)
So it will be required at run time; you will get a ClassNotFound exception.
Original comment posted by wasserman.louis on 2012-07-19 at 09:58 AM
Ew.
That said, I wonder if the funky bootstrap technique we used to make Java 5/6 compatible might be applicable here.
Original comment posted by [email protected] on 2012-07-19 at 03:15 PM
That was just a testing technique.
And re #2, if you can come up with some hackery that allows building with 1.7 yet being deployable on 1.6, while having those same .class files actually work for @SafeVarargsness, and also properly fails if any other 1.7isms get used.... let us know, but this is seeming farfetched to me.
The Java folks could have decided to support safe-varargs with something like @SuppressWarnings("SafeVarargs"), an existing annotation, but they did not, so here we are.
We did start adding this for some types (after an earlier abortive attempt documented in internal bug 10396752), and to the best of my knowledge, we haven't heard reports of issues. That's probably because we've require a Java 7 JRE (or Android, which is especially tolerant of missing classes, as I understand it). Also, I've heard that missing annotations don't cause problems at runtime nowadays -- except that's not entirely true of type-use annotations, but fortunately that exception doesn't apply here.
(OK, another problem was Proguard. But we already require users to provide a Proguard config (https://github.com/google/guava/issues/2117). And again, we've already started using @SafeVarargs. So using @SafeVarargs more should make things at most marginally worse.)
I'm assuming the answer is "all of them", but are all guava generic varargs methods intended to be treated as safe?
If not, and assuming we still can't add the safevarargs annotation, can there be alternative documentation? It would be nice if there was some indicator in the javadocs for the methods that are safe so people calling these could feel good about suppressing the varargs warning the compiler will give. Alternatively a guava specific annotation could used.
If the answer is indeed "all of them," is there a permanent FAQ item or documentation laying that out? Googling safevarargs and guava doesn't bring anything obvious up in the results.
"All of them" is extremely likely, though I can't say I've looked.
Based on https://github.com/google/guava/issues/1073#issuecomment-822655396, I suspect that we could add @SafeVarargs wherever we want nowadays. If you're in a position to sign the CLA and submit a PR, I should be able to merge it. (If you do, you can skip the android directory entirely, changing only the mainline guava directory: We'll automatically merge the changes into android.)