-
-
Notifications
You must be signed in to change notification settings - Fork 58
Support Prism::Translation::Parser35
for Ruby 3.5 parser
#371
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
Conversation
## Summary `Prism::Translation::Parser35` has been started development for Ruby 3.5 (edge Ruby): ruby/prism#3346 At present, there is no timeline for Ruby 3.5 support in the Parser gem: whitequark/parser#1046 Given this, support for Ruby 3.5 will first be introduced in `Prism::Translation::Parser`. This can also serve as a trigger to facilitate user migration from `ParserEngine: parser_whitequark` to `ParserEngine: parser_prism`. As for Ruby 3.4, `Prism::Translation::Parser` is working on supporting the `it` syntax in ruby/prism#3481, but the Parser gem has not yet supported for it. However, whether to deprecate Ruby 3.4 in the Parser gem will be considered separately from this PR. Since Ruby 3.5 is a development version with minimal impact on users, while Ruby 3.4 is a stable release, they will be evaluated separately. ## Additional Information First, the default `parser_engine` for Ruby 3.5 is set to `parser_prism`. This default aligns with the current state of support, as the Parser gem is not expected to support Ruby 3.5. The more challenging decision is how to handle the default for Ruby 3.4. Ruby 3.4 will likely serve as a transition period between the Parser gem and Prism. While it is possible to set the default `parser_engine` to `parser_prism` for Ruby 3.4 as well, considering the potential unexpected incompatibilities in `Prism::Translation::Parser`, the default remains `parser_whitequark`. In any case, the primary use case is RuboCop, and what matters most is which value RuboCop chooses to pass. In other words, the key decision lies with RuboCop.
ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism, prism_result: :prism_result) | ||
---- | ||
|
||
The Parser gem supports syntax up to Ruby 3.3, but it does not support syntax in Ruby 3.4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good to add IMPORTANT:
here.
I don't think this is the best solution, the user should not see such a warning. You have given me an idea:
It will then choose prism automatically for Ruby 3.5 only and there is nothing the user must do. Also, in the future rubocop-ast could simply choose to automatically switch to prism for ruby 3.4 as well and it would only be a single change. There would be no breaking change for how it currently does things. What do you think? |
|
||
if ruby_version >= 3.5 | ||
if parser_engine == :parser_whitequark | ||
warn Rainbow(<<~MESSAGE).yellow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubocop-ast doesn't depend on rainbow
raise ArgumentError, 'The keyword argument `parser_engine` accepts `parser_whitequark` ' \ | ||
"or `parser_prism`, but `#{parser_engine}` was passed." | ||
parser_engine = if parser_engine.nil? | ||
default_parser_engine(ruby_version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is left over from sometime before. The nil check is not necessary and this method doesn't exist anymore.
I modified my PR to show what I meant and added you as co-author. |
@Earlopain Yeah, regarding #370, if there are no incompatibilities for existing users, it seems reasonable to collaborate based on that approach. |
Yes, since it is a completely new option there is nothing that can be broken. Since I made it to only choose prism on 3.5, there are also no issues for users that currently don't explicitly pass I guess the dependency on prism can still be delayed to a later date, but it has to happen eventually. Let me check how it would look to keep the dependency optional. |
Nice. It will eventually be necessary due to the dependency on |
I updated it. When you add support for 3.5 in rubocop as well, set the target ruby version to 3.5, and parser engine to automatic, you will get this error:
prism 1.2.0 is the default gem version on Ruby 3.4.2, so you get that particular error message. Other target ruby versions continue to simply use parser. |
I will close this PR because it has been integrated into #370. |
Summary
Prism::Translation::Parser35
has been started development for Ruby 3.5 (edge Ruby): ruby/prism#3346At present, there is no timeline for Ruby 3.5 support in the Parser gem: whitequark/parser#1046
Given this, support for Ruby 3.5 will first be introduced in
Prism::Translation::Parser
. This can also serve as a trigger to facilitate user migration fromParserEngine: parser_whitequark
toParserEngine: parser_prism
.As for Ruby 3.4,
Prism::Translation::Parser
is working on supporting theit
syntax in ruby/prism#3481, but the Parser gem has not yet supported for it. However, whether to deprecate Ruby 3.4 in the Parser gem will be considered separately from this PR.Since Ruby 3.5 is a development version with minimal impact on users, while Ruby 3.4 is a stable release, they will be evaluated separately.
Additional Information
First, the default
parser_engine
for Ruby 3.5 is set toparser_prism
. This default aligns with the current state of support, as the Parser gem is not expected to support Ruby 3.5.The more challenging decision is how to handle the default for Ruby 3.4. Ruby 3.4 will likely serve as a transition period between the Parser gem and Prism. While it is possible to set the default
parser_engine
toparser_prism
for Ruby 3.4 as well, considering the potential unexpected incompatibilities inPrism::Translation::Parser
, the default remainsparser_whitequark
.In any case, the primary use case is RuboCop, and what matters most is which value RuboCop chooses to pass. In other words, the key decision lies with RuboCop.