-
-
Notifications
You must be signed in to change notification settings - Fork 277
Commit
Closes #94
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module RSpec | ||
# Detect unreferenced `let!` calls being used for test setup | ||
# | ||
# @example | ||
# # Bad | ||
# let!(:my_widget) { create(:widget) } | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
backus
Author
Collaborator
|
||
# | ||
# it 'counts widgets' do | ||
# expect(Widget.count).to eq(1) | ||
# end | ||
# | ||
# # Good | ||
# it 'counts widgets' do | ||
# create(:widget) | ||
# expect(Widget.count).to eq(1) | ||
# end | ||
# | ||
# # Good | ||
# before { create(:widget) } | ||
# | ||
# it 'counts widegts' do | ||
# expect(Widget.count).to eq(1) | ||
# end | ||
class LetSetup < Cop | ||
include RuboCop::RSpec::TopLevelDescribe, RuboCop::RSpec::Language | ||
|
||
MSG = 'Do not use `let!` for test setup.'.freeze | ||
|
||
def_node_search :let_bang, '(block $(send nil :let! (sym $_)) args ...)' | ||
|
||
def_node_search :method_called?, '(send nil %)' | ||
|
||
def_node_matcher :example_group?, <<-PATTERN | ||
(block (send _ {#{ExampleGroups::ALL.to_node_pattern}} ...) ...) | ||
PATTERN | ||
|
||
def on_block(node) | ||
return unless example_group?(node) | ||
|
||
unused_let_bang(node) do |let| | ||
add_offense(let, :expression) | ||
end | ||
end | ||
|
||
private | ||
|
||
def unused_let_bang(node) | ||
let_bang(node) do |method_send, method_name| | ||
yield(method_send) unless method_called?(node, method_name) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
# frozen_string_literal: true | ||
|
||
describe RuboCop::Cop::RSpec::LetSetup do | ||
subject(:cop) { described_class.new } | ||
|
||
it 'complains when let! is used and not referenced' do | ||
expect_violation(<<-RUBY) | ||
describe Foo do | ||
let!(:foo) { bar } | ||
^^^^^^^^^^ Do not use `let!` for test setup. | ||
it 'does not use foo' do | ||
expect(baz).to eq(qux) | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'ignores let! when used in `before`' do | ||
expect_no_violations(<<-RUBY) | ||
describe Foo do | ||
let!(:foo) { bar } | ||
before do | ||
foo | ||
end | ||
it 'does not use foo' do | ||
expect(baz).to eq(qux) | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'ignores let! when used in example' do | ||
expect_no_violations(<<-RUBY) | ||
describe Foo do | ||
let!(:foo) { bar } | ||
it 'uses foo' do | ||
foo | ||
expect(baz).to eq(qux) | ||
end | ||
end | ||
RUBY | ||
end | ||
|
||
it 'complains when let! is used and not referenced within nested group' do | ||
expect_violation(<<-RUBY) | ||
describe Foo do | ||
context 'when something special happens' do | ||
let!(:foo) { bar } | ||
^^^^^^^^^^ Do not use `let!` for test setup. | ||
it 'does not use foo' do | ||
expect(baz).to eq(qux) | ||
end | ||
end | ||
it 'references some other foo' do | ||
foo | ||
end | ||
end | ||
RUBY | ||
end | ||
end |
3 comments
on commit b85c9ad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@backus would it make sense to have this cover let
as well as let!
, so as to detect any unused let
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andyw8 no I think that is better for dynamic analysis. See http://github.com/dgollahon/rspectre
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@backus oh, interesting!
Why is this bad? Can you provide some justification? Is this just personal preference?