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.

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

Update RuboCop and use children matching #804

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
* Fix `RSpec/DescribedClass`'s error when a `described_class` is part of the namespace. ([@pirj][])
* Fix `RSpec/ExampleWording` autocorrect of multi-line docstrings. ([@pirj][])
* Add `RSpec/ContextMethod` cop, to detect method names in `context`. ([@geniou][])
* Update RuboCop dependency to 0.68.1 with support for children matching node pattern syntax. ([@pirj][])

## 1.35.0 (2019-08-02)

Expand Down
17 changes: 7 additions & 10 deletions lib/rubocop/cop/rspec/describe_class.rb
Expand Up @@ -29,28 +29,25 @@ class DescribeClass < Cop
}
PATTERN

def_node_matcher :describe_with_metadata, <<-PATTERN
(send #{RSPEC} :describe
!const
...
(hash $...))
def_node_matcher :describe_with_rails_metadata?, <<-PATTERN
(send #{RSPEC} :describe !const ...
(hash <#rails_metadata? ...>)
)
PATTERN

def_node_matcher :rails_metadata?, <<-PATTERN
(pair
(sym :type)
(sym {:request :feature :system :routing :view}))
(sym {:request :feature :system :routing :view})
)
PATTERN

def_node_matcher :shared_group?, SharedGroups::ALL.block_pattern

def on_top_level_describe(node, args)
return if shared_group?(root_node)
return if valid_describe?(node)

describe_with_metadata(node) do |pairs|
return if pairs.any?(&method(:rails_metadata?))
end
return if describe_with_rails_metadata?(node)

add_offense(args.first)
end
Expand Down
Expand Up @@ -38,7 +38,7 @@ class AttributeDefinedStatically < Cop
def on_block(node)
factory_attributes(node).to_a.flatten.each do |attribute|
next unless offensive_receiver?(attribute.receiver, node)
next if proc?(attribute) || association?(attribute)
next if proc?(attribute) || association?(attribute.first_argument)

add_offense(attribute)
end
Expand Down Expand Up @@ -72,14 +72,7 @@ def proc?(attribute)
value_matcher(attribute).to_a.all?(&:block_pass_type?)
end

def association?(attribute)
argument = attribute.first_argument
argument.hash_type? && factory_key?(argument)
end

def factory_key?(hash_node)
hash_node.keys.any? { |key| key.sym_type? && key.value == :factory }
end
def_node_matcher :association?, '(hash <(pair (sym :factory) _) ...>)'
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️ It’s so concise! I just hope people (we) will find it readable in the future.


def autocorrect_replacing_parens(node)
left_braces, right_braces = braces(node)
Expand Down
44 changes: 19 additions & 25 deletions lib/rubocop/cop/rspec/pending.rb
Expand Up @@ -12,6 +12,12 @@ module RSpec
# end
#
# describe MyClass do
# it "should be true", skip: true do
# expect(1).to eq(2)
# end
# end
#
# describe MyClass do
# it "should be true" do
# pending
# end
Expand All @@ -28,43 +34,31 @@ module RSpec
class Pending < Cop
MSG = 'Pending spec found.'

PENDING_EXAMPLES = Examples::PENDING + Examples::SKIPPED \
+ ExampleGroups::SKIPPED
SKIPPABLE_EXAMPLES = ExampleGroups::GROUPS + Examples::EXAMPLES
SKIPPABLE_SELECTORS = SKIPPABLE_EXAMPLES.node_pattern_union
PENDING = Examples::PENDING + Examples::SKIPPED + ExampleGroups::SKIPPED
SKIPPABLE = ExampleGroups::GROUPS + Examples::EXAMPLES

SKIP_SYMBOL = s(:sym, :skip)
PENDING_SYMBOL = s(:sym, :pending)
def_node_matcher :skippable?, SKIPPABLE.send_pattern

def_node_matcher :metadata, <<-PATTERN
{(send #{RSPEC} #{SKIPPABLE_SELECTORS} ... (hash $...))
(send #{RSPEC} #{SKIPPABLE_SELECTORS} $...)}
def_node_matcher :skipped_in_metadata?, <<-PATTERN
{
(send _ _ <#skip_or_pending? ...>)
(send _ _ ... (hash <(pair #skip_or_pending? true) ...>))
}
PATTERN

def_node_matcher :pending_block?, PENDING_EXAMPLES.send_pattern
def_node_matcher :skip_or_pending?, '{(sym :skip) (sym :pending)}'
def_node_matcher :pending_block?, PENDING.send_pattern

def on_send(node)
return unless pending_block?(node) || skipped_from_metadata?(node)
return unless pending_block?(node) || skipped?(node)

add_offense(node)
end

private

def skipped_from_metadata?(node)
(metadata(node) || []).any? { |n| skip_node?(n) }
end

def skip_node?(node)
if node.respond_to?(:key)
skip_symbol?(node.key) && node.value.truthy_literal?
else
skip_symbol?(node)
end
end

def skip_symbol?(symbol_node)
[SKIP_SYMBOL, PENDING_SYMBOL].include?(symbol_node)
def skipped?(node)
skippable?(node) && skipped_in_metadata?(node)
end
end
end
Expand Down
6 changes: 6 additions & 0 deletions manual/cops_rspec.md
Expand Up @@ -2257,6 +2257,12 @@ describe MyClass do
it "should be true"
end

describe MyClass do
it "should be true", skip: true do
expect(1).to eq(2)
end
end

describe MyClass do
it "should be true" do
pending
Expand Down
2 changes: 1 addition & 1 deletion rubocop-rspec.gemspec
Expand Up @@ -39,7 +39,7 @@ Gem::Specification.new do |spec|
'documentation_uri' => 'https://rubocop-rspec.readthedocs.io/'
}

spec.add_runtime_dependency 'rubocop', '>= 0.60.0'
spec.add_runtime_dependency 'rubocop', '>= 0.68.1'

spec.add_development_dependency 'rack'
spec.add_development_dependency 'rake'
Expand Down
22 changes: 11 additions & 11 deletions spec/rubocop/cop/rspec/pending_spec.rb
Expand Up @@ -124,61 +124,61 @@
RUBY
end

it 'does not flag describe' do
it 'ignores describe' do
expect_no_offenses(<<-RUBY)
describe 'test' do; end
RUBY
end

it 'does not flag example' do
it 'ignores example' do
expect_no_offenses(<<-RUBY)
example 'test' do; end
RUBY
end

it 'does not flag scenario' do
it 'ignores scenario' do
expect_no_offenses(<<-RUBY)
scenario 'test' do; end
RUBY
end

it 'does not flag specify' do
it 'ignores specify' do
expect_no_offenses(<<-RUBY)
specify 'test' do; end
specify do; end
RUBY
end

it 'does not flag feature' do
it 'ignores feature' do
expect_no_offenses(<<-RUBY)
feature 'test' do; end
RUBY
end

it 'does not flag context' do
it 'ignores context' do
expect_no_offenses(<<-RUBY)
context 'test' do; end
RUBY
end

it 'does not flag it' do
it 'ignores it' do
expect_no_offenses(<<-RUBY)
it 'test' do; end
RUBY
end

it 'does not flag it with skip: false metadata' do
it 'ignores it with skip: false metadata' do
expect_no_offenses(<<-RUBY)
it 'test', skip: false do; end
RUBY
end

it 'does not flag example_group' do
it 'ignores example_group' do
expect_no_offenses(<<-RUBY)
example_group 'test' do; end
RUBY
end

it 'does not flag method called pending' do
it 'ignores method called pending' do
expect_no_offenses(<<-RUBY)
subject { Project.pending }
RUBY
Expand Down