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

Read Java annotations. #389

Merged
merged 5 commits into from
Nov 22, 2023
Merged

Read Java annotations. #389

merged 5 commits into from
Nov 22, 2023

Conversation

sjrd
Copy link
Contributor

@sjrd sjrd commented Nov 16, 2023

Bonus: read Java parameter names.

@sjrd sjrd requested a review from bishabosha November 16, 2023 16:06
@sjrd sjrd force-pushed the java-annotations branch 3 times, most recently from e0b4960 to d35c1fb Compare November 21, 2023 11:14

case AnnotationValue.ClassConst(descriptor) =>
val classType = JavaSignatures.parseReturnDescriptor(descriptor.name)
Literal(Constant(classType))(pos)
Copy link
Member

@bishabosha bishabosha Nov 21, 2023

Choose a reason for hiding this comment

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

documentation of Constant should mention that this can be a raw class type (aka unapplied generic class)

Copy link
Member

Choose a reason for hiding this comment

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

maybe add a test for a generic external class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added documentation + test.

@@ -38,10 +38,51 @@ private[classfiles] object JavaSignatures:
superRef :: interfaces.map(classRef).toList
end parseSupers

def parseReturnDescriptor(descriptor: String)(using ReaderContext, InnerClasses, Resolver): Type =
Copy link
Member

Choose a reason for hiding this comment

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

this is also implemented by the local method result inside of parseSignature, so I wonder if it can be reused with refactoring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know. It's not that easy, however, because the more capable parseSignature expects a member: TermOrTypeSymbol that the signature attaches to, which we have no business having here. Factoring this out would require significant refactoring of the whole parsing, so I would rather not do that here (at least not at this point).

@bishabosha bishabosha merged commit 2c65b9e into scalacenter:main Nov 22, 2023
4 checks passed
@sjrd sjrd deleted the java-annotations branch November 22, 2023 15:37
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

Successfully merging this pull request may close these issues.

2 participants