diff --git a/CHANGELOG.md b/CHANGELOG.md index 606fedadc..5a75fd429 100644 --- 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 index 2b1e87b16..7a82d7bda 100644 --- 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-rspec.rb b/lib/rubocop-rspec.rb index 840b3a5a6..9aa56c6f2 100644 --- a/lib/rubocop-rspec.rb +++ b/lib/rubocop-rspec.rb @@ -22,6 +22,7 @@ require_relative 'rubocop/rspec/factory_bot' require_relative 'rubocop/rspec/final_end_location' require_relative 'rubocop/rspec/blank_line_separation' +require_relative 'rubocop/rspec/corrector/move_node' RuboCop::RSpec::Inject.defaults! diff --git a/lib/rubocop/cop/rspec/hooks_before_examples.rb b/lib/rubocop/cop/rspec/hooks_before_examples.rb index 0d4b94500..1c924da42 100644 --- a/lib/rubocop/cop/rspec/hooks_before_examples.rb +++ b/lib/rubocop/cop/rspec/hooks_before_examples.rb @@ -24,9 +24,6 @@ module RSpec # end # class HooksBeforeExamples < Cop - include RangeHelp - include RuboCop::RSpec::FinalEndLocation - MSG = 'Move `%s` above the examples in the group.' def_node_matcher :example_or_group?, <<-PATTERN @@ -45,11 +42,9 @@ def on_block(node) def autocorrect(node) lambda do |corrector| first_example = find_first_example(node.parent) - first_example_pos = first_example.loc.expression - indent = "\n" + ' ' * first_example.loc.column - - corrector.insert_before(first_example_pos, source(node) + indent) - corrector.remove(node_range_with_surrounding_space(node)) + RuboCop::RSpec::Corrector::MoveNode.new( + node, corrector, processed_source + ).move_before(first_example) end end @@ -77,19 +72,6 @@ def check_hooks(node) def find_first_example(node) node.children.find { |sibling| example_or_group?(sibling) } end - - def node_range_with_surrounding_space(node) - range = node_range(node) - range_by_whole_lines(range, include_final_newline: true) - end - - def source(node) - node_range(node).source - end - - def node_range(node) - node.loc.expression.with(end_pos: final_end_location(node).end_pos) - end end end end diff --git a/lib/rubocop/cop/rspec/leading_subject.rb b/lib/rubocop/cop/rspec/leading_subject.rb index 513163685..6176f5c58 100644 --- a/lib/rubocop/cop/rspec/leading_subject.rb +++ b/lib/rubocop/cop/rspec/leading_subject.rb @@ -32,8 +32,6 @@ module RSpec # it { expect_something_else } # class LeadingSubject < Cop - include RangeHelp - MSG = 'Declare `subject` above any other `%s` declarations.' def on_block(node) @@ -58,10 +56,9 @@ def check_previous_nodes(node) def autocorrect(node) lambda do |corrector| first_node = find_first_offending_node(node) - first_node_position = first_node.loc.expression - indent = "\n" + ' ' * first_node.loc.column - corrector.insert_before(first_node_position, node.source + indent) - corrector.remove(node_range(node)) + RuboCop::RSpec::Corrector::MoveNode.new( + node, corrector, processed_source + ).move_before(first_node) end end @@ -75,10 +72,6 @@ def find_first_offending_node(node) node.parent.children.find { |sibling| offending?(sibling) } end - def node_range(node) - range_by_whole_lines(node.source_range, include_final_newline: true) - end - def in_spec_block?(node) node.each_ancestor(:block).any? do |ancestor| example?(ancestor) diff --git a/lib/rubocop/cop/rspec/let_before_examples.rb b/lib/rubocop/cop/rspec/let_before_examples.rb index 5efb30d32..c41b60add 100644 --- a/lib/rubocop/cop/rspec/let_before_examples.rb +++ b/lib/rubocop/cop/rspec/let_before_examples.rb @@ -31,9 +31,6 @@ module RSpec # expect(some).to be # end class LetBeforeExamples < Cop - include RangeHelp - include RuboCop::RSpec::FinalEndLocation - MSG = 'Move `let` before the examples in the group.' def_node_matcher :example_or_group?, <<-PATTERN @@ -52,11 +49,9 @@ def on_block(node) def autocorrect(node) lambda do |corrector| first_example = find_first_example(node.parent) - first_example_pos = first_example.loc.expression - indent = "\n" + ' ' * first_example.loc.column - - corrector.insert_before(first_example_pos, source(node) + indent) - corrector.remove(node_range_with_surrounding_space(node)) + RuboCop::RSpec::Corrector::MoveNode.new( + node, corrector, processed_source + ).move_before(first_example) end end @@ -80,19 +75,6 @@ def check_let_declarations(node) def find_first_example(node) node.children.find { |sibling| example_or_group?(sibling) } end - - def node_range_with_surrounding_space(node) - range = node_range(node) - range_by_whole_lines(range, include_final_newline: true) - end - - def source(node) - node_range(node).source - end - - def node_range(node) - node.loc.expression.with(end_pos: final_end_location(node).end_pos) - end end end end diff --git a/lib/rubocop/cop/rspec/scattered_let.rb b/lib/rubocop/cop/rspec/scattered_let.rb index b707e048c..07c88c6e1 100644 --- 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/lib/rubocop/rspec/corrector/move_node.rb b/lib/rubocop/rspec/corrector/move_node.rb new file mode 100644 index 000000000..d85ff6c70 --- /dev/null +++ b/lib/rubocop/rspec/corrector/move_node.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module RuboCop + module RSpec + module Corrector + # Helper methods to move a node + class MoveNode + include RuboCop::Cop::RangeHelp + include RuboCop::RSpec::FinalEndLocation + + attr_reader :original, :corrector, :processed_source + + def initialize(node, corrector, processed_source) + @original = node + @corrector = corrector + @processed_source = processed_source # used by RangeHelp + end + + def move_before(other) # rubocop:disable Metrics/AbcSize + position = other.loc.expression + indent = "\n" + ' ' * other.loc.column + + corrector.insert_before(position, source(original) + indent) + corrector.remove(node_range_with_surrounding_space(original)) + end + + def move_after(other) + position = final_end_location(other) + indent = "\n" + ' ' * other.loc.column + + corrector.insert_after(position, indent + source(original)) + corrector.remove(node_range_with_surrounding_space(original)) + end + + private + + def source(node) + node_range(node).source + end + + def node_range(node) + node.loc.expression.with(end_pos: final_end_location(node).end_pos) + end + + def node_range_with_surrounding_space(node) + range = node_range(node) + range_by_whole_lines(range, include_final_newline: true) + end + end + end + end +end diff --git a/manual/cops_rspec.md b/manual/cops_rspec.md index ca6524bee..ddf21a531 100644 --- 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 index 9044b5bc6..f496b41cb 100644 --- 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