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

Always use expect_correction or expect_no_corrections when autocorrect is supported #2052

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
70 changes: 70 additions & 0 deletions spec/rubocop/cop/rspec/described_class_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,22 @@
end
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
controller(ApplicationController) do
bar = MyClass
end

before do
described_class

Foo.custom_block do
MyClass
end
end
end
RUBY
end
end

Expand All @@ -46,6 +62,22 @@
end
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
controller(ApplicationController) do
bar = described_class
end

before do
described_class

Foo.custom_block do
described_class
end
end
end
RUBY
end
end

Expand Down Expand Up @@ -95,6 +127,12 @@ module Foo
^^^^^^^ Use `described_class` instead of `MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyClass, some: :metadata do
subject { described_class }
end
RUBY
end

it 'ignores described class as string' do
Expand Down Expand Up @@ -146,6 +184,16 @@ module MyModule
end
end
RUBY

expect_correction(<<~RUBY)
describe MyClass do
describe MyClass::Foo do
subject { described_class }

let(:foo) { MyClass }
end
end
RUBY
end

it 'ignores non-matching namespace defined on `describe` level' do
Expand Down Expand Up @@ -174,6 +222,12 @@ module MyNamespace
^^^^^^^^^^^^^^^^^^^^ Use `described_class` instead of `MyNamespace::MyClass`.
end
RUBY

expect_correction(<<~RUBY)
describe MyNamespace::MyClass do
subject { described_class }
end
RUBY
end

it 'ignores non-matching namespace in usages' do
Expand Down Expand Up @@ -248,6 +302,22 @@ module D
end
end
RUBY

expect_correction(<<~RUBY)
module A
class B::C
module D
describe E do
subject { described_class }
let(:one) { described_class }
let(:two) { described_class }
let(:six) { described_class }
let(:ten) { described_class }
end
end
end
end
RUBY
end

it 'accepts an empty block' do
Expand Down
10 changes: 10 additions & 0 deletions spec/rubocop/cop/rspec/empty_example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@
end
end
RUBY

expect_correction("\n\n")
end

it 'ignores example group with examples defined in `if` branches' do
Expand Down Expand Up @@ -123,6 +125,8 @@
end
end
RUBY

expect_correction("\n\n")
end

it 'ignores example group with examples defined in `case` branches' do
Expand Down Expand Up @@ -186,6 +190,8 @@
end
end
RUBY

expect_correction("\n\n")
end

it 'ignores example group with examples defined in iterator' do
Expand Down Expand Up @@ -222,6 +228,8 @@
more_surroundings
end
RUBY

expect_correction('')
end

it 'ignores example group with examples defined in `if` branch ' \
Expand Down Expand Up @@ -407,6 +415,8 @@
end
end
RUBY

expect_correction('')
end

context 'when a custom include method is specified' do
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cop/rspec/empty_line_after_example_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,19 @@
it { }
end
RUBY

expect_correction(<<~RUBY)
RSpec.context 'foo' do
it { }
it { }

it 'does this' do
end

it { }
it { }
end
RUBY
end

it 'does not register an offense for a comment followed by an empty line' do
Expand Down Expand Up @@ -258,6 +271,14 @@
it { two }
end
RUBY

expect_correction(<<~RUBY)
RSpec.describe Foo do
it { one }

it { two }
end
RUBY
end
end
end
14 changes: 14 additions & 0 deletions spec/rubocop/cop/rspec/example_wording_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@
DESC
end
RUBY

expect_no_corrections
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this what we want, though? Does it discourage people from implementing autocorrect, or will it work fine as a “placeholder” until autocorrect is implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method only useful for cops that implement autocorrect but not for all cases. If someone doesn't implement autocorrect to begin with, there will also be no offense. There's no cop to check for extend AutoCorrector (;

In this specific case, I think autocorrect could theoretically be implemented but hasn't because heredocs are hard and nobody actually writes tests with heredoc text descriptions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, some cops have autocorrect for most cases, but some cases are left to implement later. Maybe we should have an expect_no_corrections_yet alias for those cases 😆

No, it’s probably fine with the expect_no_corrections. I guess we’ll only use it for exactly those cases, where it’s theoretically possible to implement autocorrection.

end

it 'flags an unclear description' do
Expand All @@ -337,6 +339,8 @@
^^^^^ Your example description is insufficient.
end
RUBY

expect_no_corrections
end

it 'flags an unclear description despite extra spaces' do
Expand All @@ -345,6 +349,8 @@
^^^^^^^^^^^ Your example description is insufficient.
end
RUBY

expect_no_corrections
end

it 'flags an unclear description despite uppercase and lowercase strings' do
Expand All @@ -353,6 +359,8 @@
^^^^^^ Your example description is insufficient.
end
RUBY

expect_no_corrections
end

context 'when `DisallowedExamples: Workz`' do
Expand All @@ -373,6 +381,8 @@
" " do
end
RUBY

expect_no_corrections
end

it 'flags an unclear description' do
Expand All @@ -381,6 +391,8 @@
^^^^^ Your example description is insufficient.
end
RUBY

expect_no_corrections
end

it 'flags an unclear description despite uppercase and lowercase strings' do
Expand All @@ -389,6 +401,8 @@
^^^^^^ Your example description is insufficient.
end
RUBY

expect_no_corrections
end
end
end
12 changes: 12 additions & 0 deletions spec/rubocop/cop/rspec/expect_change_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@
^^^^^^^^^^^^^^^^^^^^^ Prefer `change(User, :count)`.
end
RUBY

expect_correction(<<~RUBY)
it do
expect(run).to change(User, :count).by(1)
end
RUBY
end

it 'ignores the usage that adheres to the enforced style' do
Expand Down Expand Up @@ -256,6 +262,12 @@
^^^^^^^^^^^^^^^^^^^^ Prefer `change { User.count }`.
end
RUBY

expect_correction(<<~RUBY)
it do
expect(run).to change { User.count }.by(1)
end
RUBY
end
end
end
19 changes: 19 additions & 0 deletions spec/rubocop/cop/rspec/predicate_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,13 @@
expect(foo).to have_something
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `has_something?` over `have_something` matcher.
RUBY

expect_correction(<<~RUBY)
expect(foo.empty?).to #{matcher_true}
expect(foo.empty?).to #{matcher_true}, 'fail'
expect(foo.empty?).to #{matcher_false}
expect(foo.has_something?).to #{matcher_true}
RUBY
end

it 'registers an offense for a predicate mather with argument' do
Expand All @@ -266,6 +273,11 @@
expect(foo).to have_key(1)
^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `has_key?` over `have_key` matcher.
RUBY

expect_correction(<<~RUBY)
expect(foo.something?(1, 2)).to #{matcher_true}
expect(foo.has_key?(1)).to #{matcher_true}
RUBY
end

it 'registers an offense for a predicate matcher with a block' do
Expand All @@ -279,6 +291,13 @@
expect(foo).to be_something(x) { |y| y.present? }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `something?` over `be_something` matcher.
RUBY

expect_correction(<<~RUBY)
expect(foo.all?(&:present?)).to #{matcher_true}
expect(foo.all? { |x| x.present? }).to #{matcher_true}
expect(foo.all? { present }).to #{matcher_true}
expect(foo.something?(x) { |y| y.present? }).to #{matcher_true}
RUBY
end

it 'accepts built in matchers' do
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/rspec/return_from_stub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,12 @@ def stub_foo
^^^^^^^^^^ Use block for static values.
end
RUBY

expect_correction(<<~RUBY)
it do
allow(Foo).to receive(:bar) { 42 }
end
RUBY
end

it 'registers an offense for static values returned from chained method' do
Expand All @@ -227,6 +233,12 @@ def stub_foo
^^^^^^^^^^ Use block for static values.
end
RUBY

expect_correction(<<~RUBY)
it do
allow(Foo).to receive(:bar).with(1) { 42 }
end
RUBY
end

it 'registers, but does not try to autocorrect, heredocs' do
Expand Down
10 changes: 10 additions & 0 deletions spec/rubocop/cop/rspec/sort_metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@
end
end
RUBY

expect_correction(<<~RUBY)
RSpec.describan "Algo", :a, :b do
contexto_compartido 'una situación complicada', baz: true, foo: 'bar' do
end

ejemplo "hablando español", baz: true, foo: 'bar' do
end
end
RUBY
end
end
end
Loading