Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Fix `RSpec/InstanceVariable` detection inside custom matchers. ([@pirj][])
* Fix `RSpec/ScatteredSetup` to distinguish hooks with different metadata. ([@pirj][])
* Add autocorrect support for `RSpec/ExpectActual` cop. ([@dduugg][], [@pirj][])

## 1.37.1 (2019-12-16)

Expand Down Expand Up @@ -475,3 +476,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
[@mkrawc]: https://github.com/mkrawc
[@jfragoulis]: https://github.com/jfragoulis
[@ybiquitous]: https://github.com/ybiquitous
[@dduugg]: https://github.com/dduugg
29 changes: 27 additions & 2 deletions lib/rubocop/cop/rspec/expect_actual.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,31 @@ class ExpectActual < Cop
regexp
].freeze

def_node_matcher :expect_literal, '(send _ :expect $#literal?)'
SUPPORTED_MATCHERS = %i[eq eql equal be].freeze
Copy link
Member

Choose a reason for hiding this comment

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

it might be nice to have the option to add custom matchers in the configuration. This is another feature requests though


def_node_matcher :expect_literal, <<~PATTERN
(send
(send nil? :expect $#literal?)
#{Runners::ALL.node_pattern_union}
{
(send (send nil? $:be) :== $_)
(send nil? $_ $_ ...)
}
)
PATTERN

def on_send(node)
expect_literal(node) do |argument|
add_offense(argument)
add_offense(node, location: argument.source_range)
end
end

def autocorrect(node)
actual, matcher, expected = expect_literal(node)
lambda do |corrector|
return unless SUPPORTED_MATCHERS.include?(matcher)

swap(corrector, actual, expected)
end
end

Expand All @@ -65,6 +85,11 @@ def complex_literal?(node)
COMPLEX_LITERALS.include?(node.type) &&
node.each_child_node.all?(&method(:literal?))
end

def swap(corrector, actual, expected)
corrector.replace(actual.source_range, expected.source)
corrector.replace(expected.source_range, actual.source)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion manual/cops_rspec.md
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ IgnoredWords | `[]` | Array

Enabled by default | Supports autocorrection
--- | ---
Enabled | No
Enabled | Yes

Checks for `expect(...)` calls containing literal values.

Expand Down
171 changes: 171 additions & 0 deletions spec/rubocop/cop/rspec/expect_actual_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,17 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(123)
expect(bar).to eq(12.3)
expect(bar).to eq(1i)
expect(bar).to eq(1r)
end
end
RUBY
end

it 'flags boolean literal values within expect(...)' do
Expand All @@ -31,6 +42,15 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(true)
expect(bar).to eq(false)
end
end
RUBY
end

it 'flags string and symbol literal values within expect(...)' do
Expand All @@ -44,6 +64,15 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq("foo")
expect(bar).to eq(:foo)
end
end
RUBY
end

it 'flags literal nil value within expect(...)' do
Expand All @@ -55,6 +84,14 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(nil)
end
end
RUBY
end

it 'does not flag dynamic values within expect(...)' do
Expand All @@ -80,6 +117,15 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq([123])
expect(bar).to eq([[123]])
end
end
RUBY
end

it 'flags hashes containing only literal values within expect(...)' do
Expand All @@ -93,6 +139,15 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(foo: 1, bar: 2)
expect(bar).to eq(foo: 1, bar: [{}])
end
end
RUBY
end

it 'flags ranges containing only literal values within expect(...)' do
Expand All @@ -106,6 +161,15 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(1..2)
expect(bar).to eq(1...2)
end
end
RUBY
end

it 'flags regexps containing only literal values within expect(...)' do
Expand All @@ -117,6 +181,14 @@
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eq(/foo|bar/)
end
end
RUBY
end

it 'does not flag complex values with dynamic parts within expect(...)' do
Expand All @@ -135,6 +207,105 @@
RUBY
end

it 'ignores `be` with no argument' do
expect_no_offenses(<<~RUBY)
describe Foo do
it 'uses expect legitimately' do
expect(1).to be
end
end
RUBY
end

it 'flags `be` with an argument' do
expect_offense(<<~RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(true).to be(a)
^^^^ Provide the actual you are testing to `expect(...)`.
end
end
RUBY

expect_correction(<<~RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(a).to be(true)
end
end
RUBY
end

it 'flags `be ==`' do
expect_offense(<<~RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(1).to be == a
^ Provide the actual you are testing to `expect(...)`.
end
end
RUBY

expect_correction(<<~RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(a).to be == 1
end
end
RUBY
end

it 'flags with `eql` matcher' do
expect_offense(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(1).to eql(bar)
^ Provide the actual you are testing to `expect(...)`.
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to eql(1)
end
end
RUBY
end

it 'flags with `equal` matcher' do
expect_offense(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(1).to equal(bar)
^ Provide the actual you are testing to `expect(...)`.
end
end
RUBY

expect_correction(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect(bar).to equal(1)
end
end
RUBY
end

it 'flags but does not autocorrect violations for other matchers' do
expect_offense(<<-RUBY)
describe Foo do
it 'uses expect incorrectly' do
expect([1,2,3]).to include(a)
^^^^^^^ Provide the actual you are testing to `expect(...)`.
end
end
RUBY

expect_no_corrections
end

context 'when inspecting rspec-rails routing specs' do
let(:cop_config) { {} }

Expand Down