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

Recognize annotations from org.jspecify.annotations or whatever new package we might pick #20

Closed
cpovirk opened this issue Apr 25, 2023 · 3 comments

Comments

@cpovirk
Copy link
Collaborator

cpovirk commented Apr 25, 2023

That is, if we change the package name in jspecify/jspecify#260, then we need to update the annotation class names that we hardcode in the checker.

The main changes should be simple, perhaps two lines...

https://github.com/jspecify/nullness-checker-for-checker-framework/blob/52cef8b0c169e613d7a42d85b4b80d94dd533088/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java#L116

https://github.com/jspecify/nullness-checker-for-checker-framework/blob/52cef8b0c169e613d7a42d85b4b80d94dd533088/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java#L1947

...and the updates to the samples should be easily automated.

Ideally, we'd continue recognizing the old locations for a bit after we add the new ones. However, might not be entirely trivial for @NullMarked, which the Checker Framework wants on its classpath so that it can use a Class object for it. (I think I once got the impression we couldn't trivially switch from Class to String, but I'm not sure.) To keep it on the classpath, maybe we should arrange for JSpecify 0.3 to contain a deprecated version of the old one (yuck)? Or maybe we can just put the old @NullMarked in the checker jar itself: We already include the checker on the compile classpath (i.e., not just the processorpath) because that's how the Checker Framework likes to have at least its annotations. (Googlers can find some discussion in internal bug 187113128.) But wait, did I ever try using getDeclAnnotations (plural) instead of `getDeclAnnotation (singular)? If that works, that would be simpler.

@cpovirk
Copy link
Collaborator Author

cpovirk commented Apr 25, 2023

I did the easy part in 55d6d4d, but I forgot to mention this issue there.

As the post above says, the interesting question is @NullMarked.

@cpovirk
Copy link
Collaborator Author

cpovirk commented Apr 25, 2023

It might be worth thinking about whether the eisop enhancements to "defaulting" annotations could help us here if we did jspecify/checker-framework#29. It's been a while since I thought about what reason(s) we have for our custom defaulting code.

@cpovirk
Copy link
Collaborator Author

cpovirk commented Apr 25, 2023

Fixed by f2a6d9c.

@cpovirk cpovirk closed this as completed Apr 25, 2023
cpovirk pushed a commit to cpovirk/jspecify-reference-checker that referenced this issue Jun 26, 2024
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

1 participant