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 it lexing #3325

Closed
wants to merge 1 commit into from
Closed

Fix it lexing #3325

wants to merge 1 commit into from

Conversation

tenderlove
Copy link
Member

@tenderlove tenderlove commented Dec 19, 2024

Given the following code:

i=2; 42.tap { it /1/i }

Before this commit, the tokenizer would treat it /1/i as a method call with a regular expression parameter. it is to be treated as a local variable, so this needs to be tokenized as division rather than a regular expression. In other words it should be tokenized as it / 1 / i

This commit fixes tokenization.

[Bug #20970]

Bug is here: https://bugs.ruby-lang.org/issues/20970#change-111104

This makes the AST correct, but I haven't tested it with prism_compile.c

Given the following code:

```ruby
i=2; 42.tap { it /1/i }
```

Before this commit, the tokenizer would treat `it /1/i` as a method call
with a regular expression parameter.  `it` is to be treated as a local
variable, so this needs to be tokenized as division rather than a
regular expression.  In other words it should be tokenized as
`it / 1 / i`

This commit fixes tokenization.

[Bug #20970]
@tenderlove
Copy link
Member Author

I think we're treating it as a local variable. Even after this change, it won't appear in the local variable lists in Prism, though I believe ruby/ruby#12398 takes care of the case. I think we should probably be fixing Prism to include it in it's tables, and then we can probably remove ruby/ruby#12398 ?

I'm not sure.

@tenderlove
Copy link
Member Author

Seems like we don't need this anymore.

@tenderlove tenderlove closed this Dec 23, 2024
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.

1 participant