Support multi-versioning for Prism::Translation::Parser
#2419
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
koic
force-pushed
the
support_multi_versioning_for_prism_translation_parser
branch
from
f2709e4
to
dc19d0c
Compare
koic
changed the title
Prism::Translation::ParserPrism::Translation::Parser
koic
force-pushed
the
support_multi_versioning_for_prism_translation_parser
branch
from
dc19d0c
to
5fc2c2a
Compare
koic
force-pushed
the
support_multi_versioning_for_prism_translation_parser
branch
from
5fc2c2a
to
bfe24ef
Compare
## Summary Fixes ruby#2356. I'm working on integrating Prism into RuboCop. This PR introduces `Prism::Translation::Parser33` and `Prism::Translation::Parser34`, named in accordance with the following comments. rubocop/rubocop#12600 (comment) Currently, `Prism::Translation::Parser` always operates in Ruby 3.4 mode. This means it will not parse as Ruby 3.3 even if `TargetRubyVersion: 80_82_73_83_77.33` is specified. Therefore, the `it` introduced in Ruby 3.4 is parsed incompatibly with Ruby 3.3. In Ruby 3.3, the expected name for an `lvar` is `:it`, not `:"0it"`. ### Expected AST The following is an expected AST when parsing Ruby 3.3 code: ```console $ bundle exec ruby -rprism -rprism/translation/parser33 -ve "p Prism::Translation::Parser33.parse('items.map { it.do_something }')" ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] s(:block, s(:send, s(:send, nil, :items), :map), s(:args), s(:send, s(:send, nil, :it), :do_something)) ``` ### Actual AST The following is an actual AST when parsing Ruby 3.3 code: ```console $ ruby -rprism -ve "p Prism::Translation::Parser.parse('items.map { it.do_something }')" ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-darwin22] s(:block, s(:send, s(:send, nil, :items), :map), s(:args), s(:send, s(:lvar, :"0it"), :do_something)) ``` `Prism::Translation::Parser33` and `Prism::Translation::Parser34` aim to correspond to Ruby 3.3 and Ruby 3.4, respectively. And, The hack of specifying `TargetRubyVersion: 80_82_73_83_77.33` is expected to become unnecessary in the future, but the behavior will be maintained until RuboCop's support is finalized: rubocop/rubocop#12600 (comment) ## Additional Information A private method named `convert_for_prism` is prepared to convert the `version` from Parser to the `version` expected by Prism. For example, a Parser-compatible value is `3.3`, whereas Prism expects `"3.3.0"`. `Parser#version` is not used in RuboCop, but it's unclear how it is utilized in other libraries that rely on the Parser gem. Therefore, logic to maintain compatibility between Parser and Prism is implemented.
koic
force-pushed
the
support_multi_versioning_for_prism_translation_parser
branch
from
bfe24ef
to
62d3991
Compare
koic
deleted the
support_multi_versioning_for_prism_translation_parser
branch
koic
added a commit
to koic/rubocop-ast
that referenced
this pull request
Feb 24, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
koic
added a commit
to koic/rubocop-ast
that referenced
this pull request
Feb 24, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
koic
added a commit
to koic/rubocop-ast
that referenced
this pull request
Feb 24, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
koic
added a commit
to koic/rubocop-ast
that referenced
this pull request
Feb 25, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser_whitequark`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_whitequark) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
koic
added a commit
to koic/rubocop-ast
that referenced
this pull request
Feb 25, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser_whitequark`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_whitequark) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
bbatsov
pushed a commit
to rubocop/rubocop-ast
that referenced
this pull request
Feb 25, 2024
This PR introduces the `parser_engine` option to `ProcessedSource` to support Prism, as part of the RuboCop AST side effort towards addressing rubocop/rubocop#12600. ## Configuration By default, analysis is performed using the Parser gem, so the default value for the newly added `parser_engine` is `parser_whitequark`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_whitequark) ``` This code maintains compatibility, meaning the traditional behavior is preserved: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file) ``` To perform analysis using Prism, specify `parser_engine: :parser_prism`: ```ruby ProcessedSource.new(@options[:stdin], ruby_version, file, parser_engine: :parser_prism) ``` The parameter name `parser_prism` reflects the original parser_prism which was the basis for `Prism::Translation::Parser` (now integrated into Prism): https://github.com/kddnewton/parser-prism This is an experimental introduction, and some incompatibilities still remain. > [!NOTE] > As initially mentioned in rubocop/rubocop#12600 (comment), > the plan was to set `parser_engine: prism`. > > However, the parser engine used in this PR is `Prism::Translation::Parser`, not `Prism`: > ruby/prism#2419 > > `Prism::Translation::Parser` and `Prism` have different ASTs, so their migration will definitely cause incompatibility. > So, considering the possibility of further replacing `Prism::Translation::Parser` with `Prism` in the future, > it has been decided that it might be better not to use `ParserEngine: prism` for the time being. > `ParserEngine: prism` is reserved for `Prism`, not `Prism::Translation::Parser`. > > Therefore, the parameter value has been set to `parser_engine: parser_prism` specifically for > `Prism::Translation::Parser`. > > This means that the planned way to specify Prism in .rubocop.yml file will be `ParserEngine: parser_prism`, > not `ParserEngine: prism`. ## Compatibility The compatibility issues between Prism and the Parser gem have not been resolved. The failing tests will be skipped with `broken_on: :prism`: - ruby/prism#2454 has been resolved but not yet released. - ruby/prism#2467 is still unresolved. Issues that will be resolved in several upcoming releases of Prism are being skipped with `broken_on: :prism`. Anyway, RuboCop AST can be released independently of the resolution and release of Prism. > [!NOTE] > The hack in `Prism::Translation::Parser` for `ProcessedSource` needs to be fixed: > https://github.com/ruby/prism/blob/v0.24.0/lib/prism/translation/parser/rubocop.rb > > If the above interface is accepted, a fix will be proposed on the Prism side. ## Test Tests for RuboCop AST with Prism as the backend can be run as follows: ```console bundle exec rake prism_spec ``` The above is the shortcut alias for: ```console PARSER_ENGINE=parser_prism TARGET_RUBY_VERSION=3.3 rake spec ``` RuboCop AST works on Ruby versions 2.6+, but since Prism only targets analysis for Ruby 3.3+, `internal_investigation` Rake task will not be executed. This task is only run with the Parser gem, which can analyze Ruby versions 2.0+.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2356
I'm working on integrating Prism into RuboCop.
This PR introduces
Prism::Translation::Parser33andPrism::Translation::Parser34, named in accordance with the following comments.rubocop/rubocop#12600 (comment)
Currently,
Prism::Translation::Parseralways operates in Ruby 3.4 mode. This means it will not parse as Ruby 3.3 even ifTargetRubyVersion: 80_82_73_83_77.33is specified.Therefore, the
itintroduced in Ruby 3.4 is parsed incompatibly with Ruby 3.3. In Ruby 3.3, the expected name for anlvaris:it, not:"0it".Expected AST
The following is an expected AST when parsing Ruby 3.3 code:
Actual AST
The following is an actual AST when parsing Ruby 3.3 code:
Prism::Translation::Parser33andPrism::Translation::Parser34aim to correspond to Ruby 3.3 and Ruby 3.4, respectively.And, The hack of specifying
TargetRubyVersion: 80_82_73_83_77.33is expected to become unnecessary in the future, but the behavior will be maintained until RuboCop's support is finalized: rubocop/rubocop#12600 (comment)Additional Information
A private method named
convert_for_prismis prepared to convert theversionfrom Parser to theversionexpected by Prism. For example, a Parser-compatible value is3.3, whereas Prism expects"3.3.0".Parser#versionis not used in RuboCop, but it's unclear how it is utilized in other libraries that rely on the Parser gem.Therefore, logic to maintain compatibility between Parser and Prism is implemented.