Skip to content

Commit

Permalink
Permit Weirich-style block syntax
Browse files Browse the repository at this point in the history
* rubocop#1600
* rubocop/ruby-style-guide#162
* http://devblog.avdi.org/2011/07/26/the-procedurefunction-block-convention-in-ruby/

Add a configuration option to the Style/Blocks cop to permit two styles:

* multiline (the current default);
* weirich (the semantic rule as described in the links above).

With Weirich style enabled, this allows multi-line blocks with braces if
the block is considered "functional". The current implementation checks
whether the return value of a block is used to classify it as
"functional". It performs the following checks:

1. Is the return value of the block being assigned?
2. Is the return value of the block sent a message?
3. Is the return value of the block the last thing in its scope?

This should cover the following Weirich-style use cases:

    # 1
    foo = map { |x|
      x * 2
    }

    # 2
    map { |x|
      x * 2
    }.inspect

    # 3
    block do
      foo

      map { |x|
        x * 2
      }
    end

    # 3
    puts map { |x|
      x * 2
    }

Add offenses if the return value of a block is used but do...end is used
instead of the intention-revealing {...}. Conversely, if the return
value of a block is not used, add an offense if {...} is used instead of
do...end.

Add a configurable whitelist of methods that use the return value of
their block without being obvious to the caller. This permits use cases
such as let or subject in RSpec.

As DSLs often use do...end (e.g. RSpec.describe), do not add an offense
if a block uses do...end even though it could potentially be the return
value of its outer scope, e.g.

    RSpec.describe Foo do
      it 'blah' do
        # ...
      end
    end
  • Loading branch information
Chris Lowder & Paul Mucur authored and mudge committed Mar 21, 2015
1 parent 1ecb718 commit 1569d21
Show file tree
Hide file tree
Showing 4 changed files with 305 additions and 94 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features

* [#1600](https://github.com/bbatsov/rubocop/issues/1600): Add `multiline` and `weirich` styles to the `Blocks` cop. ([@clowder][], [@mudge][])
* [#1712](https://github.com/bbatsov/rubocop/pull/1712): Set `Offense#corrected?` to `true`, `false`, or `nil` when it was, wasn't, or can't be auto-corrected, respectively. ([@vassilevsky][])
* [#1669](https://github.com/bbatsov/rubocop/pull/1669): Add command-line switch `--display-style-guide`. ([@marxarelli][])
* [#1405](https://github.com/bbatsov/rubocop/issues/1405): Add Rails TimeZone and Date cops. ([@palkan][])
Expand Down Expand Up @@ -1309,3 +1310,5 @@
[@zvkemp]: https://github.com/zvkemp
[@vassilevsky]: https://github.com/vassilevsky
[@gerry3]: https://github.com/gerry3
[@clowder]: https://github.com/clowder
[@mudge]: https://github.com/mudge
10 changes: 10 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ Style/BarePercentLiterals:
- percent_q
- bare_percent

Style/Blocks:
EnforcedStyle: multiline
SupportedStyles:
- multiline
- weirich
Whitelist:
- let
- let!
- subject

Style/BracesAroundHashParameters:
EnforcedStyle: no_braces
SupportedStyles:
Expand Down
93 changes: 85 additions & 8 deletions lib/rubocop/cop/style/blocks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ module Style
# Check for uses of braces or do/end around single line or
# multi-line blocks.
class Blocks < Cop
include ConfigurableEnforcedStyle
include AutocorrectUnlessChangingAST

MULTI_LINE_MSG = 'Avoid using {...} for multi-line blocks.'
SINGLE_LINE_MSG = 'Prefer {...} over do...end for single-line blocks.'

def on_send(node)
_receiver, method_name, *args = *node
return unless args.any?
Expand All @@ -27,17 +25,67 @@ def on_send(node)
def on_block(node)
return if ignored_node?(node)

if proper_block_style?(node)
correct_style_detected
else
add_offense(node, :begin)
end
end

private

def message(node)
block_begin = node.loc.begin.source
block_length = Util.block_length(node)

case style
when :weirich
if block_begin == '{'
'Prefer do...end over {...} for procedural blocks.'
else
'Prefer {...} over do...end for functional blocks.'
end
when :multiline
if block_length > 0
'Avoid using {...} for multi-line blocks.'
else
'Prefer {...} over do...end for single-line blocks.'
end
end
end

def proper_block_style?(node)
case style
when :weirich
weirich_block_style?(node)
when :multiline
multiline_block_style?(node)
end
end

def weirich_block_style?(node)
block_length = Util.block_length(node)
block_begin = node.loc.begin.source

if block_length > 0 && block_begin == '{'
add_offense(node, :begin, MULTI_LINE_MSG)
elsif block_length == 0 && block_begin != '{'
add_offense(node, :begin, SINGLE_LINE_MSG)
return true if block_length == 0

if block_begin == '{'
functional_block?(node)
else
!return_value_used?(node)
end
end

private
def multiline_block_style?(node)
block_length = Util.block_length(node)
block_begin = node.loc.begin.source

if block_length > 0
block_begin != '{'
else
block_begin == '{'
end
end

def correction(node)
lambda do |corrector|
Expand Down Expand Up @@ -74,6 +122,35 @@ def parentheses?(send_node)
def operator?(method_name)
method_name =~ /^\W/
end

def functional_block?(node)
whitelisted_method?(node) || return_value_used?(node) ||
return_value_of_scope?(node)
end

def whitelisted_method?(node)
receiver, _args, _body = *node
_, method_name = *receiver

whitelist.include?(method_name)
end

def return_value_used?(node)
return unless node.parent

Util::ASGN_NODES.include?(node.parent.type) ||
node.parent.send_type?
end

def return_value_of_scope?(node)
return unless node.parent

node.parent.children.last == node
end

def whitelist
Array(cop_config['Whitelist']).map(&:to_sym)
end
end
end
end
Expand Down
Loading

0 comments on commit 1569d21

Please sign in to comment.