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

Handle EnsuresCalledMethods.List when checking field ownership #6180

Merged
merged 5 commits into from
Sep 18, 2023

Conversation

Calvin-L
Copy link
Contributor

@Calvin-L Calvin-L commented Sep 15, 2023

Fixes #5911.

Please let me know if there is a shorter or more canonical way to write getEnsuresCalledMethodsAnnotations; this seems like something a lot of checkers have to do, but I couldn't find an appropriate helper method.

@kelloggm
Copy link
Contributor

Please let me know if there is a shorter or more canonical way to write getEnsuresCalledMethodsAnnotations; this seems like something a lot of checkers have to do, but I couldn't find an appropriate helper method.

I'm not aware of an appropriate helper method, and the existence of CreatesMustCallForToJavaExpression#getCreatesMustCallForAnnos, which your implementation is patterned on, suggests that I wasn't aware of one when I wrote it, either.

I think such a helper method could be useful. I would put it in one of the subclasses of AnnotatedTypeFactory or maybe in AnnotatedTypeFactory itself; its signature would look something like:

AnnotationMirrorSet getRepeatableDeclAnnotation(AnnotatedTypeFactory this, ExecutableElement target, Class<?> annoClazz, Class<?> annoListClazz, ExecutableElement listValueElt);

The implementation could be patterned on the code in this PR for @EnsuresCalledMethods and the code in getCreatesMustCallForAnnos for @CreatesMustCallFor.

@smillst does this helper method sound reasonable to you? Is there an existing alternative that I'm unaware of?

I copy-pasted this from a similar method, and sure enough I forgot to
rename the local variables to match their new use.
Copy link
Member

@smillst smillst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a helper method that looked for a specific repeatable annotation. AnnotatedTypeFactory#getDeclAnnotationWithMetaAnnotation looks for repeated annotations with a specific meta annotation. We could copy the functionality for repeated annotations into a more general helper method like @kelloggm suggests. But implementing that helper method doesn't need to hold up this PR.

@kelloggm kelloggm enabled auto-merge (squash) September 18, 2023 19:27
@kelloggm kelloggm merged commit 0c4d952 into typetools:master Sep 18, 2023
29 checks passed
@Calvin-L Calvin-L deleted the multiple-owned-resources branch September 19, 2023 15:53
wmdietl pushed a commit to eisop/checker-framework that referenced this pull request Oct 18, 2023
…ools#6180)

Co-authored-by: Martin Kellogg <martin.kellogg@njit.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants