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

Adds an autocorrect to RSpec/ExpectActual, swapping the arguments to expect and matcher when possible. #858

Merged
merged 4 commits into from Jan 15, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
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
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
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
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