[java] Support for annotations in types#4245
Merged
Merged
Conversation
Missing pieces: - other declarations - some type annot paths
All TypePaths are now handled
…em.type-annotations-support
This is because iteration order is not defined, also in some places we have to merge annotations from a declaration with another set of annots and there may be duplicates.
…em.type-annotations-support
This was an implementation bug, type param annotations are now correctly reflected only on the symbol. They need to be merged with the type annots of the usage when doing inference though - todo for later
Member
|
@oowekyala is this now ready for review? |
Member
Author
|
I still have a couple of local commits, will publish them tonight. I'll mark this as draft until then |
3d98a9e to
f850049
Compare
Member
|
This PR states that there is future work:
I'd add to this list that return types for constructors are currently not including annotations. Given the code (present in this PR): public abstract class ClassWithTypeAnnotationsOnMethods {
static class CtorOwner {
CtorOwner(@A @B int i) { }
@A CtorOwner() { }
CtorOwner(String i) throws @A Exception {}
}
}The following test would fail: JClassType symParent = impl.getDeclaration(ClassWithTypeAnnotationsOnMethods.class);
JClassType sym = symParent.getDeclaredClass("CtorOwner");
assertHasTypeAnnots(sym.getConstructors().get(1).getReturnType(), ANNOT_A); // FAILS!Type params on constructor args and exceptions are properly retrieved (although we are missing those tests). |
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the PR
PSet<SymAnnot>instead ofList<SymAnnot>. We need a set because there is no defined iteration order for annotations that come from the class file, and we also have to merge annotations during inference. PSet is practical because we don't have to make copies to make the list immutable.Related issues
class Foo extends List<@NonNull Integer>)Ready?
./mvnw clean verifypasses (checked automatically by github actions)