Support itblock
#2077
Support itblock
#2077
Conversation
| return unless example_group_with_body?(node) | ||
|
|
||
| check_hooks(node.body) if multiline_block?(node.body) | ||
| end | ||
|
|
||
| alias on_numblock on_block |
There was a problem hiding this comment.
This had no effect, so I removed it. It relies on a previous block and then simply uses next_sibling together with any_block (which includes itblock). I only added tests to show it already works.
| @@ -38,7 +38,6 @@ def on_block(node) | |||
| on_metadata_arguments(metadata_arguments) | |||
| end | |||
| end | |||
| alias on_numblock on_block | |||
There was a problem hiding this comment.
This module is included in 4 cops and none have tests for numblocks. So I simply removed this one.
I don't think handling these blocks makes sense in context anyways and it was only added here because of the internal affairs cop
| @@ -39,5 +39,5 @@ Gem::Specification.new do |spec| | |||
| } | |||
|
|
|||
| spec.add_dependency 'lint_roller', '~> 1.1' | |||
| spec.add_dependency 'rubocop', '~> 1.72', '>= 1.72.1' | |||
| spec.add_dependency 'rubocop', '~> 1.75' | |||
There was a problem hiding this comment.
1.75 transitively depends on rubocop-ast >= 1.43.0 which is good enough
|
I guess I should clarify: |
| @@ -44,6 +44,11 @@ module SpecHelper | |||
|
|
|||
| config.include_context 'with default RSpec/Language config', :config | |||
| config.include_context 'smoke test', type: :cop_spec | |||
|
|
|||
| # whitequark/parser is syntax compatible with 2.0 to 3.3. | |||
| if ENV['PARSER_ENGINE'] != 'parser_prism' | |||
There was a problem hiding this comment.
The parser gem doesn't support 3.4-specific syntax. RuboCop now always uses prism for these versions, so some tests that use newer syntax may fail with parser.
There was a problem hiding this comment.
If we remove this, what fails?
The ultimate goal is to be running those new specs at least in one CI job. What job would it be?
We can still check against 3.4 syntax and use prism even if we run ‘rubocop’ in 3.1, right?
How do we set the “Ruby version of the code being investigated”? Same as the runtime Ruby?
There was a problem hiding this comment.
If we remove this, what fails?
I was a bit overeager with adding the excludes. There are only two failures for two cops, so I removed it from the others. The others pass by chance because itblock is simply block with zero block arguments from the perspective of parser.
The ultimate goal is to be running those new specs at least in one CI job. What job would it be?
It will run only in the prism job from workflows/main.yml since it will fail on the parser one. It sets PARSER_ENGINE=parser_prism which is used by the specs.
We can still check against 3.4 syntax and use prism even if we run ‘rubocop’ in 3.1, right?
How do we set the “Ruby version of the code being investigated”? Same as the runtime Ruby?
These two points I don't understand what you are asking, can you elaborate?
|
Hm, coverage is reduced because when using the |
|
|
||
| context 'when Ruby 3.4', :ruby34, unsupported_on: :parser do | ||
| it 'does not flag :example for hooks' do | ||
| expect_no_offenses(<<~RUBY) |
There was a problem hiding this comment.
Can you please change this to ‘it.run’ not to offend the other cop?
There was a problem hiding this comment.
I copy-pasted the current numblock specs, will change these as well.
| expect_offense(<<~RUBY) | ||
| RSpec.describe User do | ||
| it { is_expected.to be_after_around_hook } | ||
| around { it } |
There was a problem hiding this comment.
I wonder if this would work anyway without making a special case for an it-block? Looks like a regular no-args block call with an ‘it’ call inside. No surprise, this may or may not be an it-block, hard to tell.
Side note, how does Ruby knows ‘describe { it { foo } }’ is not an it-block?
There was a problem hiding this comment.
By what I propose here to keep the spec, and attempt to remove special handling in the cop. Chances are it will work anyway.
There was a problem hiding this comment.
Yup, that's what I did: #2077 (comment)
Side note, how does Ruby knows ‘describe { it { foo } }’ is not an it-block?
it with block had special considerations from ruby itself, it only points to the block argument when it doesn't take a block itself. https://bugs.ruby-lang.org/issues/18980
The following cases have been discussed:
- it method, most famously in RSpec: You almost always pass a positional and/or block argument to RSpec's it, so the conflict is avoided with my proposal. You virtually never use a completely naked it
All cops that handle numblock now also handle itblock. `itblock` is relatively new and the required `rubocop-ast` version is required via RuboCop 1.75
Earlopain
force-pushed
the
itblock-support
branch
from
e4a4818 to
3279266
Compare
All cops that handle numblock now also handle itblock.
itblockis relatively new and the requiredrubocop-astversion is required via RuboCop 1.75Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).