Skip to content

Various fixes for parsing type use annotations #206

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

theawless
Copy link

@theawless theawless commented Jun 5, 2025

Fix type use annotations before spread parameter

Closes #205

The original fix was here 0d79d9f but there's a typo in that. Java doesn't allow annotations after the ellipsis at all. The original issue #184 as well as the minimal sample found by the maintainer #184 (comment) is for annotations before the ellipsis.

Fix type use annotations in catch types

Closes #208

According to the spec https://docs.oracle.com/javase/specs/jls/se24/html/jls-14.html#jls-CatchType, catch types can be annotated. Note that modifiers and _type can both accept annotations, but I have maintained the existing behaviour - by parsing annotations as part of the modifiers for the first type.

@theawless theawless force-pushed the master branch 2 times, most recently from 1418c63 to 14a8c10 Compare June 5, 2025 21:06
Copy link

@jackschu jackschu left a comment

Choose a reason for hiding this comment

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

Looks like the spec agrees with you :) (im not a maintainer or anything but wanted to take a look)

https://docs.oracle.com/javase/specs/jls/se22/html/jls-8.html#jls-8.4.1

VariableArityParameter: {VariableModifier} UnannType {Annotation} ... Identifier

@jackschu
Copy link

jackschu commented Jun 9, 2025

I'll note that youre upgrading the ABI here, from my POV thats fine, its likely the next release should include ABI 15 anyways (?), but the maintainers know better or feel differently. It might be worth keeping the ABI at 14 to play it safe (tree-sitter generate --abi 14). I had some discussion in the discord about this.

@clason
Copy link

clason commented Jun 9, 2025

Yes, keep commits (and PRs) atomic. Bumping ABI can be a breaking change for consumers stuck on an older version of the library.

@theawless

This comment was marked as outdated.

@theawless theawless requested a review from jackschu June 9, 2025 16:15
@theawless theawless changed the title Parsing fix for type use annotations with spread parameters Parsing fix for annotations before spread parameter Jun 10, 2025
@theawless theawless changed the title Parsing fix for annotations before spread parameter Various fixes for parsing type use annotations Jun 11, 2025
Copy link

@jackschu jackschu left a comment

Choose a reason for hiding this comment

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

Reasoning for the catch type extension seems legit, again though I'm no maintainer (sorry)

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.

Parsing error for annotations on catch parameters Parsing error for annotations before spread parameter
3 participants