Skip to content

Commit

Permalink
Fix a false positive for RSpec/PendingWithoutReason when not inside…
Browse files Browse the repository at this point in the history
… example and pending/skip with block

Fix: rubocop#1565
  • Loading branch information
ydah committed Jan 26, 2023
1 parent 4db43fb commit 84cd968
Show file tree
Hide file tree
Showing 3 changed files with 235 additions and 81 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Add `RSpec/RedundantAround` cop. ([@r7kamura])
- Add `RSpec/Rails/TravelAround` cop. ([@r7kamura])
- Add `RSpec/ContainExactly` and `RSpec/MatchArray` cops. ([@faucct])
- Fix a false positive for `RSpec/PendingWithoutReason` when not inside example and pending/skip with block. ([@ydah])

## 2.18.1 (2023-01-19)

Expand Down
99 changes: 65 additions & 34 deletions lib/rubocop/cop/rspec/pending_without_reason.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,28 @@ module RSpec
# it 'does something', skip: 'reason' do
# end
class PendingWithoutReason < Base
include InsideExampleGroup
MSG = 'Give the reason for pending or skip.'

# @!method pending_by_example_method?(node)
def_node_matcher :pending_by_example_method?, block_pattern(<<~PATTERN)
#Examples.pending
PATTERN

# @!method pending_by_metadata_without_reason?(node)
def_node_matcher :pending_by_metadata_without_reason?, <<~PATTERN
(send #rspec? {#ExampleGroups.all #Examples.all} ... {<(sym :pending) ...> (hash <(pair (sym :pending) true) ...>)})
# @!method skipped_by_example_method?(node)
def_node_matcher :skipped_by_example_method?, <<~PATTERN
(block
(send nil?
$#Examples.skipped
(...)
) args...
)
PATTERN

# @!method skipped_by_example_method?(node)
def_node_matcher :skipped_by_example_method?, block_pattern(<<~PATTERN)
#Examples.skipped
# @!method metadata_without_reason?(node)
def_node_matcher :metadata_without_reason?, <<~PATTERN
(send #rspec?
{#ExampleGroups.all #Examples.all} ...
{
<(sym ${:pending :skip}) ...>
(hash <(pair (sym ${:pending :skip}) true) ...>)
}
)
PATTERN

# @!method skipped_by_example_group_method?(node)
Expand All @@ -82,38 +89,62 @@ class PendingWithoutReason < Base
PATTERN
)

# @!method skipped_by_metadata_without_reason?(node)
def_node_matcher :skipped_by_metadata_without_reason?, <<~PATTERN
(send #rspec? {#ExampleGroups.all #Examples.all} ... {<(sym :skip) ...> (hash <(pair (sym :skip) true) ...>)})
PATTERN

# @!method without_reason?(node)
def_node_matcher :without_reason?, <<~PATTERN
(send nil? ${:pending :skip})
# @!method pending_step_without_reason?(node)
def_node_matcher :pending_step_without_reason?, <<~PATTERN
(send nil? {:skip :pending})
PATTERN

def on_send(node)
if pending_without_reason?(node)
add_offense(node, message: 'Give the reason for pending.')
elsif skipped_without_reason?(node)
add_offense(node, message: 'Give the reason for skip.')
elsif without_reason?(node) && example?(node.parent)
add_offense(node,
message: "Give the reason for #{node.method_name}.")
end
return unless inside_example_group?(node)

on_pending_by_metadata(node)
on_skipped_by_example_method(node)
on_skipped_by_example_group_method(node)
end

def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
return unless inside_example_group?(node)

on_pending_step(node)
end

private

def pending_without_reason?(node)
pending_by_example_method?(node.block_node) ||
pending_by_metadata_without_reason?(node)
def on_pending_by_metadata(node)
metadata_without_reason?(node) do |pending|
add_offense(node, message: "Give the reason for #{pending}.")
end
end

def skipped_without_reason?(node)
skipped_by_example_group_method?(node.block_node) ||
skipped_by_example_method?(node.block_node) ||
skipped_by_metadata_without_reason?(node)
def on_skipped_by_example_method(node)
skipped_by_example_method?(node.block_node) do |pending|
add_offense(node, message: "Give the reason for #{pending}.")
end
end

def on_skipped_by_example_group_method(node)
skipped_by_example_group_method?(node.block_node) do
add_offense(node, message: 'Give the reason for skip.')
end
end

def on_pending_step(node)
block_node_bodys(node).each do |body|
if pending_step_without_reason?(body)
add_offense(body,
message: "Give the reason for #{body.method_name}.")
end
end
end

def block_node_bodys(node)
return [] unless (body = node.body)

if body.begin_type?
body.child_nodes
else
[body]
end
end
end
end
Expand Down
Loading

0 comments on commit 84cd968

Please sign in to comment.