Implement auto-correct for ScatteredLet cop

rubocop · Apr 29, 2020 · 9941338 · 9941338
1 parent 7615d62
commit 9941338
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 * Ignore String constants by `RSpec/Describe`. ([@AlexWayfer][])
 * Drop support for ruby 2.3. ([@bquorning][])
 * Fix multiple cops to detect `let` with proc argument. ([@tejasbubane][])
+* Add autocorrect support for `RSpec/ScatteredLet`. ([@Darhazer][])
 
 ## 1.38.1 (2020-02-15)
 

diff --git a/config/default.yml b/config/default.yml
@@ -416,6 +416,7 @@ RSpec/ScatteredLet:
   Description: Checks for let scattered across the example group.
   Enabled: true
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ScatteredLet
+  VersionChanged: '1.39'
 
 RSpec/ScatteredSetup:
   Description: Checks for setup scattered across multiple hooks in an example group.

diff --git a/lib/rubocop/cop/rspec/scattered_let.rb b/lib/rubocop/cop/rspec/scattered_let.rb
@@ -35,6 +35,15 @@ def on_block(node)
           check_let_declarations(node.body)
         end
 
+        def autocorrect(node)
+          lambda do |corrector|
+            first_let = find_first_let(node.parent)
+            RuboCop::RSpec::Corrector::MoveNode.new(
+              node, corrector, processed_source
+            ).move_after(first_let)
+          end
+        end
+
         private
 
         def check_let_declarations(body)
@@ -47,6 +56,10 @@ def check_let_declarations(body)
             add_offense(node)
           end
         end
+
+        def find_first_let(node)
+          node.children.find { |child| let?(child) }
+        end
       end
     end
   end

diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md
@@ -2723,7 +2723,7 @@ EnforcedStyle | `and_return` | `and_return`, `block`
 
 Enabled by default | Supports autocorrection
 --- | ---
-Enabled | No
+Enabled | Yes
 
 Checks for let scattered across the example group.
 
@@ -2751,6 +2751,12 @@ describe Foo do
 end
 ```
 
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+VersionChanged | `1.39` | String
+
 ### References
 
 * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ScatteredLet](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ScatteredLet)

diff --git a/spec/rubocop/cop/rspec/scattered_let_spec.rb b/spec/rubocop/cop/rspec/scattered_let_spec.rb
@@ -7,16 +7,86 @@
     expect_offense(<<-RUBY)
       RSpec.describe User do
         let(:a) { a }
-        subject { User }
+        it { expect(subject.foo).to eq(a) }
         let(:b) { b }
         ^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
       end
     RUBY
+
+    expect_correction(<<-RUBY)
+      RSpec.describe User do
+        let(:a) { a }
+        let(:b) { b }
+        it { expect(subject.foo).to eq(a) }
+      end
+    RUBY
+  end
+
+  it 'works with heredocs' do
+    expect_offense(<<-RUBY)
+      describe User do
+        let(:a) { <<-BAR }
+          hello
+          world
+        BAR
+        it { expect(subject.foo).to eq(a) }
+        let(:b) { <<-BAZ }
+        ^^^^^^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
+          again
+        BAZ
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      describe User do
+        let(:a) { <<-BAR }
+          hello
+          world
+        BAR
+        let(:b) { <<-BAZ }
+          again
+        BAZ
+        it { expect(subject.foo).to eq(a) }
+      end
+    RUBY
+  end
+
+  it 'flags `let` at different nesting levels' do
+    expect_offense(<<-RUBY)
+      describe User do
+        let(:a) { a }
+        it { expect(subject.foo).to eq(a) }
+
+        describe '#property' do
+          let(:c) { c }
+
+          it { expect(subject.property).to eq c }
+
+          let(:d) { d }
+          ^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
+        end
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      describe User do
+        let(:a) { a }
+        it { expect(subject.foo).to eq(a) }
+
+        describe '#property' do
+          let(:c) { c }
+          let(:d) { d }
+
+          it { expect(subject.property).to eq c }
+
+        end
+      end
+    RUBY
   end
 
   it 'doesnt flag `let!` in the middle of multiple `let`s' do
     expect_no_offenses(<<-RUBY)
-      RSpec.describe User do
+      describe User do
         subject { User }
 
         let(:a) { a }
@@ -26,14 +96,41 @@
     RUBY
   end
 
+  it 'flags scattered `let!`s' do
+    expect_offense(<<-RUBY)
+      describe User do
+        let!(:a) { a }
+        it { expect(subject.foo).to eq(a) }
+        let!(:c) { c }
+        ^^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      describe User do
+        let!(:a) { a }
+        let!(:c) { c }
+        it { expect(subject.foo).to eq(a) }
+      end
+    RUBY
+  end
+
   it 'flags `let` with proc argument' do
     expect_offense(<<-RUBY)
-      RSpec.describe User do
+      describe User do
         let(:a) { a }
-        subject { User }
+        it { expect(subject.foo).to eq(a) }
         let(:user, &args[:build_user])
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
       end
     RUBY
+
+    expect_correction(<<-RUBY)
+      describe User do
+        let(:a) { a }
+        let(:user, &args[:build_user])
+        it { expect(subject.foo).to eq(a) }
+      end
+    RUBY
   end
 end