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.

Sign up for GitHub

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

Jump to bottom

Fix: Fix false negative for RSpec/Dialect when specified Capybara-specific methods #1952

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: Fix false negative for RSpec/Dialect when specified Capybara-specific methods #1952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

- Fix false-negative and error for `RSpec/MetadataStyle` when non-literal args are used in metadata in `EnforceStyle: hash`. ([@cbliard])
- Fix false negative for `RSpec/Dialect` when specified Capybara-specific methods. ([@sanfrecce-osaka])

## 3.0.4 (2024-08-05)

Expand Down Expand Up @@ -1004,6 +1005,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@rspeicher]: https://github.com/rspeicher
[@rst-j]: https://github.com/RST-J
[@samrjenkins]: https://github.com/samrjenkins
[@sanfrecce-osaka]: https://github.com/sanfrecce-osaka
[@schmijos]: https://github.com/schmijos
[@seanpdoyle]: https://github.com/seanpdoyle
[@sl4vr]: https://github.com/sl4vr
Expand Down
6 changes: 2 additions & 4 deletions lib/rubocop/cop/rspec/dialect.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,12 @@ module RSpec
class Dialect < Base
extend AutoCorrector
include MethodPreference
include InsideExampleGroup

MSG = 'Prefer `%<prefer>s` over `%<current>s`.'

# @!method rspec_method?(node)
def_node_matcher :rspec_method?, '(send #rspec? #ALL.all ...)'

def on_send(node)
return unless rspec_method?(node)
return unless inside_example_group?(node)
return unless preferred_methods[node.method_name]

msg = format(MSG, prefer: preferred_method(node.method_name),
Expand Down
217 changes: 173 additions & 44 deletions spec/rubocop/cop/rspec/dialect_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,59 +1,186 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::Dialect do
let(:cop_config) do
{
'PreferredMethods' => {
'context' => 'describe'
}
}
end
context 'with preferred methods' do
context 'when `describe` is preferred to `context`' do
let(:cop_config) do
{
'PreferredMethods' => {
'context' => 'describe'
}
}
end

it 'allows describe blocks' do
expect_no_offenses(<<~RUBY)
describe 'display name presence' do
it 'allows describe blocks' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'context' do
describe 'display name presence' do
end
end
RUBY
end
RUBY
end

it 'allows calling methods named context in examples' do
expect_no_offenses(<<~RUBY)
it 'tests common context invocations' do
expect(request.context).to be_empty?
it 'registers an offense for context blocks' do
expect_offense(<<~RUBY)
RSpec.describe 'context' do
context 'display name presence' do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `describe` over `context`.
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'context' do
describe 'display name presence' do
end
end
RUBY
end
RUBY
end
end

it 'registers an offense for context blocks' do
expect_offense(<<~RUBY)
context 'display name presence' do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `describe` over `context`.
context 'when `describe` is preferred to `feature`' do
let(:cop_config) do
{
'PreferredMethods' => {
'feature' => 'describe'
}
}
end
RUBY

expect_correction(<<~RUBY)
describe 'display name presence' do
it 'allows describe blocks' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'context' do
describe 'display name presence' do
end
end
RUBY
end
RUBY
end

it 'registers an offense for RSpec.context blocks' do
expect_offense(<<~RUBY)
RSpec.context 'context' do
^^^^^^^^^^^^^^^^^^^^^^^ Prefer `describe` over `context`.
it 'tests common context invocations' do
expect(request.context).to be_empty?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those ones were here to ensure that context with an explicit receiver is not detected. Can you keep it in some way?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply 🙏

Those ones were here to ensure that context with an explicit receiver is not detected. Can you keep it in some way?

In fa5da43 the check using rspec_method? has been removed, so explicit receivers are also detected. To avoid detection, I can't think of any ideas other than to do the following.

          def on_send(node)
            return unless inside_example_group?(node)
+           return unless rspec?(node.children.first)
            return unless preferred_methods[node.method_name]

Do you have any good ideas?

end
it 'registers an offense for feature blocks' do
expect_offense(<<~RUBY)
RSpec.describe 'context' do
feature 'display name presence' do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `describe` over `feature`.
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'context' do
describe 'display name presence' do
end
end
RUBY
end
RUBY
end

expect_correction(<<~RUBY)
RSpec.describe 'context' do
it 'tests common context invocations' do
expect(request.context).to be_empty?
end
context 'when `let` is preferred to `given`' do
let(:cop_config) do
{
'PreferredMethods' => {
'given' => 'let'
}
}
end

it 'allows let blocks' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'context' do
let do
end
end
RUBY
end
RUBY

it 'registers an offense for given blocks' do
expect_offense(<<~RUBY)
RSpec.describe 'context' do
given do
^^^^^ Prefer `let` over `given`.
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'context' do
let do
end
end
RUBY
end
end

context 'when `let!` is preferred to `given!`' do
let(:cop_config) do
{
'PreferredMethods' => {
'given!' => 'let!'
}
}
end

it 'allows let! blocks' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'context' do
let! do
end
end
RUBY
end

it 'registers an offense for given! blocks' do
expect_offense(<<~RUBY)
RSpec.describe 'context' do
given! do
^^^^^^ Prefer `let!` over `given!`.
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'context' do
let! do
end
end
RUBY
end
end

context 'when `before` is preferred to `background`' do
let(:cop_config) do
{
'PreferredMethods' => {
'background' => 'before'
}
}
end

it 'allows before blocks' do
expect_no_offenses(<<~RUBY)
RSpec.describe 'context' do
before do
end
end
RUBY
end

it 'registers an offense for background blocks' do
expect_offense(<<~RUBY)
RSpec.describe 'context' do
background do
^^^^^^^^^^ Prefer `before` over `background`.
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe 'context' do
before do
end
end
RUBY
end
end
end

context 'without preferred methods' do
Expand All @@ -65,9 +192,11 @@

it 'allows all methods blocks' do
expect_no_offenses(<<~RUBY)
context 'is important' do
specify 'for someone to work' do
everyone.should have_some_leeway
RSpec.describe 'context' do
context 'is important' do
specify 'for someone to work' do
everyone.should have_some_leeway
end
end
end
RUBY
Expand Down