Skip to content
Closed
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][])

## 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
16 changes: 16 additions & 0 deletions lib/rubocop/cop/rspec/expect_actual.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ def on_send(node)
end
end

def autocorrect(node)
lambda do |corrector|
expectation = node.parent.parent
rhs = expectation.children.last
return unless rhs.is_a?(RuboCop::AST::MethodDispatchNode)
return if rhs.method_name != :eq
Copy link
Member

Choose a reason for hiding this comment

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

Maybe also :==, :eql?, :equal?, and :be?
A culprit with :be is that it might not have an argument:

expect(3).to be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @pirj, thanks for the suggestions. Could I trouble you to provide examples using the first three methods? I interpreted your suggestion as applying like so:

expect(123).to ==(bar)
expect(123).to eql?(bar)
expect(123).to equal?(bar)

…but that doesn't appear to be valid:

     Failure/Error: expect(123).to equal?(123)

     ArgumentError:
       The expect syntax does not support operator matchers, so you must pass a matcher to `#to`.

Copy link
Member

@pirj pirj Jan 8, 2020

Choose a reason for hiding this comment

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

My bad. Should of course be:

expect(123).to be
expect(123).to be == 123
expect(123).to eql(123)
expect(123).to equal(123)

Thanks for the extra care.


swap_order(corrector, node, rhs.children.last)
end
end

private

# This is not implement using a NodePattern because it seems
Expand All @@ -65,6 +76,11 @@ def complex_literal?(node)
COMPLEX_LITERALS.include?(node.type) &&
node.each_child_node.all?(&method(:literal?))
end

def swap_order(corrector, lhs_arg, rhs_arg)
corrector.replace(lhs_arg.source_range, rhs_arg.source)
corrector.replace(rhs_arg.source_range, lhs_arg.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
85 changes: 85 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,19 @@
RUBY
end

it 'flags but does not autocorrect violations without eq' 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