Skip to content

Conversation

@Strum355
Copy link
Contributor

@Strum355 Strum355 commented Apr 8, 2021

Previously, symbols that had annotations did not render those in the formatter/pretty-printer.

This PR introduces 2 new semanticdb spec changes that need to be upstreamed:

  1. A new AnnotationTree type as part of the Tree collection.
    • This replaces the Annotation type used in SymbolInformation (and AnnotatedType) while having the same index 1 field and an additional repeated Tree field for parameters
  2. A new AssignTree type as part of the Tree collection, for assignment expressions.

In combination, these add support to semanticdb for modeling the full JLS annotation spec, which we utilize to pretty-print.


Follow-up PRs:

  1. Add helper functions for building many of the semanticdb constructs add pretty-printing of annotations on symbols #148 (comment) ✔️
  2. Move the semanticdbXXX methods in SemanticdbVisitor to a separate class add pretty-printing of annotations on symbols #148 (comment) ✔️

Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Phenomenal work getting annotations working! SemanticDB has never properly supported annotations because they're too complicated to model in Scala. It's a fantastic breakthrough that we can properly support Java annotations.

A few comments, the most important ones relate to changes in the SemanticDB spec.

@Strum355 Strum355 requested a review from olafurpg April 9, 2021 19:17
@Strum355
Copy link
Contributor Author

Strum355 commented Apr 9, 2021

Ive updated the PR description with the changes that arose from the review 🙂 @olafurpg

@Strum355 Strum355 force-pushed the nsc/symbol-annotation-format branch from a8e5910 to e5f14a2 Compare April 9, 2021 20:24
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

Outstanding work 👏 This is ready to merge!

It would be good to followup soon with the upstream spec changes.

@Strum355 Strum355 force-pushed the nsc/symbol-annotation-format branch 2 times, most recently from e419be0 to 2d36a96 Compare April 12, 2021 22:17
@Strum355
Copy link
Contributor Author

Merging ahead of scalameta/scalameta#2281. We dont anticipate the PR to result in any byte-level breaking changes to what is being merged here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants