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.

Sign up for GitHub

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

Jump to bottom

Extract some shared logic to BlankLineSeparation #961

Merged
merged 4 commits into from Jul 14, 2020
Merged

Extract some shared logic to BlankLineSeparation #961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8014b6b
Use subject and subject! in message
bquorning Jul 13, 2020
88d2b7c
Use let and let! in message
bquorning Jul 13, 2020
ee6da00
Rename BlankLineSeparation to EmptyLineSeparation
bquorning Jul 13, 2020
51d593a
Extract some shared logic to BlankLineSeparation
bquorning Jul 13, 2020
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: 1 addition & 1 deletion lib/rubocop-rspec.rb
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@
require_relative 'rubocop/rspec/align_let_brace'
require_relative 'rubocop/rspec/factory_bot'
require_relative 'rubocop/rspec/final_end_location'
require_relative 'rubocop/rspec/blank_line_separation'
require_relative 'rubocop/rspec/empty_line_separation'
require_relative 'rubocop/rspec/corrector/move_node'

RuboCop::RSpec::Inject.defaults!
Expand Down
10 changes: 3 additions & 7 deletions lib/rubocop/cop/rspec/empty_line_after_example.rb
View file
Open in desktop
Expand Up @@ -43,20 +43,16 @@ module RSpec
#
class EmptyLineAfterExample < Cop
extend AutoCorrector
include RuboCop::RSpec::BlankLineSeparation
include RuboCop::RSpec::EmptyLineSeparation

MSG = 'Add an empty line after `%<example>s`.'

def on_block(node)
return unless example?(node)
return if last_child?(node)
return if allowed_one_liner?(node)

missing_separating_line(node) do |location|
msg = format(MSG, example: node.method_name)
add_offense(location, message: msg) do |corrector|
corrector.insert_after(location.end, "\n")
end
missing_separating_line_offense(node) do |method|
format(MSG, example: method)
end
end

Expand Down
10 changes: 3 additions & 7 deletions lib/rubocop/cop/rspec/empty_line_after_example_group.rb
View file
Open in desktop
Expand Up @@ -25,19 +25,15 @@ module RSpec
#
class EmptyLineAfterExampleGroup < Cop
extend AutoCorrector
include RuboCop::RSpec::BlankLineSeparation
include RuboCop::RSpec::EmptyLineSeparation

MSG = 'Add an empty line after `%<example_group>s`.'

def on_block(node)
return unless example_group?(node)
Copy link
Member

Choose a reason for hiding this comment

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

Will it work as

example_group?(node) do |example_group|
  msg = format(MSG, example_group: example_group.method_name)
  check_missing_separating_lines(example_group, message: msg)
end

Copy link
Member

Choose a reason for hiding this comment

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

Same above for EmptyLineAfterExample and below for EmptyLineAfterHook.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be neat, but we’d have to add a capture in the right place in RuboCop::RSpec::Language::SelectorSet. Something to explore in a separate PR, maybe?

Copy link
Member

Choose a reason for hiding this comment

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

It still could be

example_group?(node) do
  msg = format(MSG, example_group: node.method_name)
  check_missing_separating_lines(node, message: msg)
end

but then I would remove the ? from the name. Still, something that could be explored in a separate PR

Copy link
Member

Choose a reason for hiding this comment

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

Up to you. Looks good as is.
There's always an indefinite space for improvement.

return if last_child?(node)

missing_separating_line(node) do |location|
msg = format(MSG, example_group: node.method_name)
add_offense(location, message: msg) do |corrector|
corrector.insert_after(location.end, "\n")
end
missing_separating_line_offense(node) do |method|
format(MSG, example_group: method)
end
end
end
Expand Down
11 changes: 4 additions & 7 deletions lib/rubocop/cop/rspec/empty_line_after_final_let.rb
View file
Open in desktop
Expand Up @@ -18,22 +18,19 @@ module RSpec
# it { does_something }
class EmptyLineAfterFinalLet < Cop
extend AutoCorrector
include RuboCop::RSpec::BlankLineSeparation
include RuboCop::RSpec::EmptyLineSeparation

MSG = 'Add an empty line after the last `let` block.'
MSG = 'Add an empty line after the last `%<let>s`.'

def on_block(node)
return unless example_group_with_body?(node)

latest_let = node.body.child_nodes.select { |child| let?(child) }.last
bquorning marked this conversation as resolved.
Show resolved Hide resolved

return if latest_let.nil?
bquorning marked this conversation as resolved.
Show resolved Hide resolved
return if last_child?(latest_let)

missing_separating_line(latest_let) do |location|
add_offense(location) do |corrector|
corrector.insert_after(location.end, "\n")
end
missing_separating_line_offense(latest_let) do |method|
format(MSG, let: method)
end
end
end
Expand Down
10 changes: 3 additions & 7 deletions lib/rubocop/cop/rspec/empty_line_after_hook.rb
View file
Open in desktop
Expand Up @@ -35,19 +35,15 @@ module RSpec
#
class EmptyLineAfterHook < Cop
extend AutoCorrector
include RuboCop::RSpec::BlankLineSeparation
include RuboCop::RSpec::EmptyLineSeparation

MSG = 'Add an empty line after `%<hook>s`.'

pirj marked this conversation as resolved.
Show resolved Hide resolved
def on_block(node)
return unless hook?(node)
return if last_child?(node)

missing_separating_line(node) do |location|
msg = format(MSG, hook: node.method_name)
add_offense(location, message: msg) do |corrector|
corrector.insert_after(location.end, "\n")
end
missing_separating_line_offense(node) do |method|
format(MSG, hook: method)
end
end
end
Expand Down
11 changes: 4 additions & 7 deletions lib/rubocop/cop/rspec/empty_line_after_subject.rb
View file
Open in desktop
Expand Up @@ -16,18 +16,15 @@ module RSpec
# let(:foo) { bar }
class EmptyLineAfterSubject < Cop
extend AutoCorrector
include RuboCop::RSpec::BlankLineSeparation
include RuboCop::RSpec::EmptyLineSeparation

MSG = 'Add empty line after `subject`.'
MSG = 'Add an empty line after `%<subject>s`.'

bquorning marked this conversation as resolved.
Show resolved Hide resolved
def on_block(node)
return unless subject?(node) && !in_spec_block?(node)
return if last_child?(node)

missing_separating_line(node) do |location|
add_offense(location) do |corrector|
corrector.insert_after(location.end, "\n")
end
missing_separating_line_offense(node) do |method|
format(MSG, subject: method)
end
end

Expand Down
15 changes: 13 additions & 2 deletions lib/rubocop/rspec/blank_line_separation.rb → lib/rubocop/rspec/empty_line_separation.rb
View file
Open in desktop
Expand Up @@ -2,12 +2,23 @@

module RuboCop
module RSpec
# Helps determine the offending location if there is not a blank line
# Helps determine the offending location if there is not an empty line
# following the node. Allows comments to follow directly after.
module BlankLineSeparation
module EmptyLineSeparation
include FinalEndLocation
include RuboCop::Cop::RangeHelp

def missing_separating_line_offense(node)
return if last_child?(node)

missing_separating_line(node) do |location|
msg = yield(node.method_name)
add_offense(location, message: msg) do |corrector|
corrector.insert_after(location.end, "\n")
end
end
end

def missing_separating_line(node)
line = final_end_location(node).line

Expand Down
14 changes: 7 additions & 7 deletions spec/rubocop/cop/rspec/empty_line_after_final_let_spec.rb
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@
RSpec.describe User do
let(:a) { a }
let(:b) { b }
^^^^^^^^^^^^^ Add an empty line after the last `let` block.
^^^^^^^^^^^^^ Add an empty line after the last `let`.
it { expect(a).to eq(b) }
end
RUBY
Expand All @@ -30,7 +30,7 @@
let!(:b) do
b
end
^^^ Add an empty line after the last `let` block.
^^^ Add an empty line after the last `let!`.
it { expect(a).to eq(b) }
end
RUBY
Expand All @@ -52,7 +52,7 @@
RSpec.describe User do
let(:a) { a }
let(:user, &args[:build_user])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add an empty line after the last `let` block.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add an empty line after the last `let`.
it { expect(a).to eq(b) }
end
RUBY
Expand Down Expand Up @@ -96,7 +96,7 @@
let(:a) { a }
let(:b) { b }
# end of setup
^^^^^^^^^^^^^^ Add an empty line after the last `let` block.
^^^^^^^^^^^^^^ Add an empty line after the last `let`.
it { expect(a).to eq(b) }
end
RUBY
Expand All @@ -119,7 +119,7 @@
let(:b) { b }
# a multiline comment marking
# the end of setup
^^^^^^^^^^^^^^^^^^ Add an empty line after the last `let` block.
^^^^^^^^^^^^^^^^^^ Add an empty line after the last `let`.
it { expect(a).to eq(b) }
end
RUBY
Expand All @@ -144,7 +144,7 @@
subject { described_class }
let!(:b) { b }
^^^^^^^^^^^^^^ Add an empty line after the last `let` block.
^^^^^^^^^^^^^^ Add an empty line after the last `let!`.
it { expect(a).to eq(b) }
end
RUBY
Expand Down Expand Up @@ -236,7 +236,7 @@
hello
world
BAR
^^^ Add an empty line after the last `let` block.
^^^ Add an empty line after the last `let`.
it 'has tricky syntax' do
expect(foo).to eql(" hello\n world\n")
end
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cop/rspec/empty_line_after_subject_spec.rb
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@
expect_offense(<<-RUBY)
RSpec.describe User do
subject { described_class.new }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add empty line after `subject`.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add an empty line after `subject`.
let(:params) { foo }
end
RUBY
Expand All @@ -25,7 +25,7 @@
expect_offense(<<-RUBY)
RSpec.describe User do
subject! { described_class.new }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add empty line after `subject`.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Add an empty line after `subject!`.
let(:params) { foo }
end
RUBY
Expand Down