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

Fix tuple parsing bug #627

Merged
merged 2 commits into from
Mar 30, 2021
Merged

Fix tuple parsing bug #627

merged 2 commits into from
Mar 30, 2021

Conversation

jasmith-hs
Copy link
Contributor

In Jinjava, we parse something surrounded with parentheses either as an AstTuple or an AstNested. When there is a single element inside the parentheses, we'll parse it as AstNested, otherwise it will become an AstTuple.

This check was done by seeing if there are any commas before the first ). However, this is inadequate for two reasons:

  1. We can have an expression like: #{(range(0,1))}. Clearly this shouldn't be parsed as a tuple, but this bug caused it to be.
  2. We can have an expression like: #{((1),2)}. Clearly this should be parsed as a tuple, but the bug causes it to be parsed as an AstNested

The fix here simply maintains a counter of depth into parentheses or brackets, and only checks for the existence of a comma symbol when at 0 depth. We don't need to worry about matching ( to ) and [ to ] here because if they are mismatched, the correct parsing exception will get thrown down the line anyway.

@jasmith-hs jasmith-hs merged commit 83ead56 into master Mar 30, 2021
@jasmith-hs jasmith-hs deleted the comma-tuple branch March 30, 2021 20:17
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