-
-
Notifications
You must be signed in to change notification settings - Fork 290
Add RSpec/MemoizedHelperBlockDelimiter cop
#1419
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module RuboCop | ||
| module Cop | ||
| module RSpec | ||
| # Use consistent style block delimiters for memoized helpers. | ||
| # | ||
| # @example EnforcedStyle: do_end (default) | ||
| # # bad | ||
| # let(:foo) { 'bar' } | ||
| # | ||
| # # good | ||
| # let(:foo) do | ||
| # 'bar' | ||
| # end | ||
| # | ||
| # # bad | ||
| # subject(:foo) { 'bar' } | ||
| # | ||
| # # good | ||
| # subject(:foo) do | ||
| # 'bar' | ||
| # end | ||
| # | ||
| # @example EnforcedStyle: braces | ||
| # # bad | ||
| # let(:foo) do | ||
| # 'bar' | ||
| # end | ||
| # | ||
| # # good | ||
| # let(:foo) { 'bar' } | ||
| # | ||
| # # bad | ||
| # subject(:foo) do | ||
| # 'bar' | ||
| # end | ||
| # | ||
| # # good | ||
| # subject(:foo) { 'bar' } | ||
| class MemoizedHelperBlockDelimiter < Base | ||
| extend AutoCorrector | ||
|
|
||
| include ConfigurableEnforcedStyle | ||
| include RangeHelp | ||
| include RuboCop::RSpec::Language | ||
|
|
||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [void] | ||
| def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler | ||
| return unless bad?(node) | ||
|
|
||
| add_offense( | ||
| node.location.begin.with( | ||
| end_pos: node.location.end.end_pos | ||
| ), | ||
| message: "Use #{style} style block delimiters." | ||
| ) do |corrector| | ||
| autocorrect(corrector, node) | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| # @param corrector [RuboCop::Cop::Corrector] | ||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [void] | ||
| def autocorrect(corrector, node) | ||
| case style | ||
| when :braces | ||
| # Not supported because it conflicts with Style/BlockDelimiters. | ||
| when :do_end | ||
| autocorrect_braces(corrector, node) | ||
| end | ||
| end | ||
|
|
||
| # @param corrector [RuboCop::Cop::Corrector] | ||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [void] | ||
| def autocorrect_braces(corrector, node) | ||
| unless whitespace_before?(node.location.begin) | ||
| corrector.insert_before(node.location.begin, ' ') | ||
| end | ||
| corrector.replace(node.location.begin, 'do') | ||
| corrector.replace(node.location.end, 'end') | ||
| wrap_in_newlines(corrector, node) if node.single_line? | ||
| end | ||
|
|
||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [Boolean] | ||
| def bad?(node) | ||
| memoized_helper_method?(node) && | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line doesn't semantically belong to "bad", it detects if it's a match. memoized_helper_method(node) do |memoized_helper|
next if preferred_block_delimiter?(memoized_helper)
add_offense(...) |
||
| !preferred_block_delimiter?(node) | ||
| end | ||
|
|
||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [Boolean] | ||
| def memoized_helper_method?(node) | ||
| let?(node) || subject?(node) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will also match blockpass, should it?
Comment on lines
+98
to
+99
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest turning this into a node pattern: |
||
| end | ||
|
|
||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [Boolean] | ||
| def preferred_block_delimiter?(node) | ||
| case style | ||
| when :braces | ||
| node.braces? | ||
| when :do_end | ||
| !node.braces? | ||
| end | ||
| end | ||
|
|
||
| # @param range [Parser::Source::Range] | ||
| # @return [Boolean] | ||
| def whitespace_before?(range) | ||
| range.source_buffer.source[range.begin_pos - 1].match?(/\s/) | ||
| end | ||
|
|
||
| # @param corrector [RuboCop::Cop::Corrector] | ||
| # @param node [RuboCop::AST::BlockNode] | ||
| # @return [void] | ||
| def wrap_in_newlines(corrector, node) | ||
| corrector.wrap( | ||
| node.location.begin.with( | ||
| begin_pos: node.location.begin.end_pos, | ||
| end_pos: node.location.end.begin_pos | ||
| ), | ||
| "\n", | ||
| "\n" | ||
| ) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree with the choice of the default style.
do_enddoesn't seem to be a frequent choice in the wild, according to a run againstreal-world-rspec:as opposed to
braces:And, I suspect some part of do/end usages (offences to
bracesstyle) are due to the line length limitation:or because of being multi-line e.g.
/Users/pirj/source/real-world-rspec/spree/emails/spec/mailers/spree/order_mailer_spec.rb:25:22::