Java: Add support for Annotation types stub generation #8695
Conversation
There was a problem hiding this comment.
Thanks a lot for considering the feedback!
There might be a few remaining issues.
Regarding annotation default values: There might not be any additional work needed because they seem to be included when an annotation is used (at least the last time I checked this), see also #6275.
atorralba
force-pushed
the
atorralba/stub-generator-annotation-types
branch
from
1f1e494 to
fdf6510
Compare
There was a problem hiding this comment.
Should there be a test for this? There's a similar query in C#, we added a not so human readable test in https://github.com/github/codeql/tree/main/csharp/ql/test/query-tests/Stubs/Minimal. It might be worth considering for Java too.
atorralba
force-pushed
the
atorralba/stub-generator-annotation-types
branch
from
fdf6510 to
7687c8c
Compare
Good point @tamasvajk, thanks! I had to create a See 7687c8c for details, let me know if something doesn't add up. |
| } | ||
|
|
||
| private string stubAccessibilityModifier() { | ||
| if this.isPublic() then result = "public " else result = "" | ||
| } | ||
|
|
||
| private string stubAnnotations() { | ||
| result = concat(stubAnnotation(this.(AnnotationType).getAnAnnotation()) + "\n") |
There was a problem hiding this comment.
Do we need a stable order for the annotations? I see that we don't order the members in stubMembers either, but we do order by all concats in C#. Just wondering.
There was a problem hiding this comment.
This would seem wise, ordering by their fully-qualified names for example
| result = this.(AnnotationType).getAnAnnotation().getAValue().getType() or | ||
| result = this.(AnnotationType).getAnAnnotation().getAValue().(ArrayInit).getAnInit().getType() |
There was a problem hiding this comment.
Note that getAValue was recently deprecated because it's usually not useful -- use getAnArrayValue(_) instead, which unpacks array members
| @@ -391,6 +398,47 @@ private string stubMember(Member m) { | |||
| ) | |||
| } | |||
|
|
|||
| language[monotonicAggregates] | |||
| private string stubAnnotation(Annotation a) { | |||
| if exists(a.getAValue()) | |||
atorralba
force-pushed
the
atorralba/stub-generator-annotation-types
branch
from
7687c8c to
f860ae8
Compare
|
Thanks for the review @tamasvajk @smowton and sorry for the delay. Comments addressed in f860ae8. |
| result = concat(stubAnnotation(this.(AnnotationType).getAnAnnotation()) + "\n") | ||
| result = | ||
| concat(Annotation an | | ||
| this.(AnnotationType).getAnAnnotation() = an | ||
| | | ||
| stubAnnotation(an), "\n" order by an.getType().getQualifiedName() |
There was a problem hiding this comment.
This is actually a behavior change: Previously it would add a \n after each annotation. Now it only adds it between annotations but the last one does not have a trailing \n (see also test output changes).
When the last annotation has values (and therefore ends with )) that still seems to produce valid Java code, but I assume when an annotation has no values this produces malformed Java code:
@MyAnnotationpublic void m() {
}There was a problem hiding this comment.
Thanks @Marcono1234, good catch. I addressed this in #10662.
Add line break after all stubbed annotations to avoid malformed code See https://github.com/github/codeql/pull/8695\#discussion_r985674245
Add line break after all stubbed annotations to avoid malformed code See https://github.com/github/codeql/pull/8695\#discussion_r985674245
Adds support for Annotation types in our Java stub generator.
Due to default values for Annotation methods not being currently supported, the stubs generated may need manual correction if the original types have default values, e.g. https://github.com/spring-projects/spring-framework/blob/main/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java#L85.