-
Notifications
You must be signed in to change notification settings - Fork 260
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
Replace obsolete @NullableDecl #704
Conversation
Also, remove dependency on checker-compat-qual since it is no longer needed. From downstream fix in Debian.
Thanks. Some of these changes produce different meanings for the annotations, as laid out in https://checkerframework.org/manual/#faq-array-syntax-meaning and https://checkerframework.org/manual/#declaration-annotations-moved. I've started tweaking a copy of this PR to try to make that work, but I've hit a tool bug that I'm reporting. It's possible that the right solution is for us to give up on nullness annotations entirely (at least in this open-source copy of the library) until we can require our users to use Java 8. (Maybe we could get away with requiring Java 8 even today: Even though some of our users target Java 7, they are likely all testing with Java 8+.) Maybe we'll remove them from our external copy entirely, or maybe we'll use Retrolambda to rewrite Java 8 bytecode to Java 7 bytecode (but solely for the purpose of removing type annotations, without actually touching lambdas). (Or we could migrate to a different system of nullness annotations, one that still uses declaration annotations.) I will figure something out, but it's looking like it may take some time. |
Update: Someone added a workaround for us. So now I've hit... another bug in another tool! And a diamond-dependency conflict! I'll keep plugging away.... |
We do so for a couple reasons: - We've gotten [requests to avoid it](google/truth#704) (and checker-compat-qual in general) before. - The Checker Framework defines it to have a different meaning than other nullness declaration annotations (as discussed in [this documentation's exception for the `org.checkerframework` package](https://checkerframework.org/manual/#faq-declaration-annotations-moved)), and I would guess that no other tools handle it accordingly. We mostly aren't using `@NullableDecl` nowadays, since we've migrated to jsr305 and checker-qual annotations. But some annotations remain, mostly in tests and testlib. RELNOTES=n/a PiperOrigin-RevId: 399231620
We do so for a couple reasons: - We've gotten [requests to avoid it](google/truth#704) (and checker-compat-qual in general) before. - The Checker Framework defines it to have a different meaning than other nullness declaration annotations (as discussed in [this documentation's exception for the `org.checkerframework` package](https://checkerframework.org/manual/#faq-declaration-annotations-moved)), and I would guess that no other tools handle it accordingly. We mostly aren't using `@NullableDecl` nowadays, since we've migrated to jsr305 and checker-qual annotations. But some annotations remain, mostly in tests and testlib. RELNOTES=n/a PiperOrigin-RevId: 399471446
Also, remove dependency on checker-compat-qual since it is no longer needed.
From downstream fix in Debian.
Fix for #497