-
Notifications
You must be signed in to change notification settings - Fork 355
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
Warn when using the top type in a pre- or post-condition annotation #6412
base: master
Are you sure you want to change the base?
Conversation
Thanks! I'm in favour of merging this over my work in #6409 |
List<ExecutableElement> methods = ElementFilter.methodsIn(anno.getEnclosedElements()); | ||
if (isPreOrPostConditionAnnotation(annoName)) { | ||
AnnotationMirror anno = TreeUtils.annotationFromAnnotationTree(tree); | ||
System.out.printf("anno = %s for annoName = %s for tree = %s%n", anno, annoName, tree); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be removed or commented.
if (annoName.contentEquals(DefaultQualifier.class.getName()) | ||
|| annoName.contentEquals(SuppressWarnings.class.getName())) { | ||
// Skip these two annotations, as we don't care about the arguments to them. | ||
return null; | ||
} | ||
|
||
List<ExecutableElement> methods = ElementFilter.methodsIn(anno.getEnclosedElements()); | ||
if (isPreOrPostConditionAnnotation(annoName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this logic in BaseTypeVisitor#checkContractsAtMethodDeclaration
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially implemented this check in BaseTypeVisitor#checkContractsAtMethodDeclaration
(because I also thought it was useful that the contracts are already there in the method), but that method appears to check implicit annotations, as well.
if (isPreOrPostConditionAnnotation(annoName)) { | ||
AnnotationMirror anno = TreeUtils.annotationFromAnnotationTree(tree); | ||
System.out.printf("anno = %s for annoName = %s for tree = %s%n", anno, annoName, tree); | ||
// TODO: `getQualifierEnforcedByContractAnnotation` does not work for @PreconditionAnnotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smillst What is your view on this design question? I lean toward the latter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to answer this question because I don't know what you mean by "does not work". How/where are those annotations handled else where?
Possible replacement for #6409.
This pull request is a bit simpler, but more importantly it puts the warning messages at the right location rather than on the method itself.