Permit Weirich-style block syntax

* 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`.

As there are some methods that are functional or procedural but cannot
be categorised as such from their usage alone, add configurable lists to
permit methods such as `RSpec::Core::ExampleGroup.let` which is
functional but appears procedural and `tap` which is procedural but
appears functional. For methods which can be both functional and
procedural (such as `lambda`) and cannot be categorised by usage, add a
configurable list of ignored methods.

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

CHANGELOG.md

@@ -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][])
@@ -1313,3 +1314,5 @@
 [@vassilevsky]: https://github.com/vassilevsky
 [@gerry3]: https://github.com/gerry3
 [@ypresto]: https://github.com/ypresto
+[@clowder]: https://github.com/clowder
+[@mudge]: https://github.com/mudge

config/default.yml

@@ -148,6 +148,70 @@ Style/BarePercentLiterals:
     - percent_q
     - bare_percent
 
+Style/Blocks:
+  EnforcedStyle: multiline
+  SupportedStyles:
+    # The `multiline` style enforces braces around single line blocks and
+    # do..end around multi-line blocks.
+    - multiline
+    # The `weirich` style enforces braces around functional blocks, where the
+    # primary purpose of the block is to return a value and do..end for
+    # procedural blocks, where the primary purpose of the block is its
+    # side-effects.
+    #
+    # This looks at the usage of a block's method to determine its type (e.g. is
+    # the result of a `map` assigned to a variable or passed to another
+    # variable) but exceptions are permitted in the `ProceduralMethods`,
+    # `FunctionalMethods` and `IgnoredMethods` sections below.
+    - weirich
+  ProceduralMethods:
+    # Methods that are known to be procedural in nature but look functional from
+    # their usage, e.g.
+    #
+    #   time = Benchmark.realtime do
+    #     foo.bar
+    #   end
+    #
+    # Here, the return value of the block is discarded but the return value of
+    # `Benchmark.realtime` is used.
+    - benchmark
+    - bm
+    - bmbm
+    - create
+    - each_with_object
+    - measure
+    - new
+    - realtime
+    - tap
+  FunctionalMethods:
+    # Methods that are known to be functional in nature but look procedural frm
+    # their usage, e.g.
+    #
+    #   let(:foo) { Foo.new }
+    #
+    # Here, the return value of `Foo.new` is used to define a `foo` helper but
+    # doesn't appear to be used from the return value of `let`.
+    - let
+    - let!
+    - subject
+  IgnoredMethods:
+    # Methods that can be either procedural or functional and cannot be
+    # categorised from their usage alone, e.g.
+    #
+    #   foo = lambda do |x|
+    #     puts "Hello, #{x}"
+    #   end
+    #
+    #   foo = lambda do |x|
+    #     x * 100
+    #   end
+    #
+    # Here, it is impossible to tell from the return value of `lambda` whether
+    # the inner block's return value is significant.
+    - lambda
+    - proc
+    - it
+
 Style/BracesAroundHashParameters:
   EnforcedStyle: no_braces
   SupportedStyles:

lib/rubocop/cop/style/blocks.rb

@@ -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?
@@ -27,18 +25,35 @@ def on_send(node)
         def on_block(node)
           return if ignored_node?(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)
+          if proper_block_style?(node)
+            correct_style_detected
+          else
+            add_offense(node, :begin) { opposite_style_detected }
           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 correction(node)
           lambda do |corrector|
             b, e = node.loc.begin, node.loc.end
@@ -74,6 +89,97 @@ def parentheses?(send_node)
         def operator?(method_name)
           method_name =~ /^\W/
         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)
+          method_name = extract_method_name_from_block(node)
+          return true if ignored_method?(method_name)
+
+          block_begin = node.loc.begin.source
+
+          if block_begin == '{'
+            functional_method?(method_name) || functional_block?(node)
+          else
+            procedural_method?(method_name) || !return_value_used?(node)
+          end
+        end
+
+        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 extract_method_name_from_block(block)
+          node, _args, _body = *block
+          _receiver, method_name, *_args = *node
+
+          method_name
+        end
+
+        def ignored_method?(method_name)
+          ignored_methods.include?(method_name)
+        end
+
+        def functional_method?(method_name)
+          functional_methods.include?(method_name)
+        end
+
+        def functional_block?(node)
+          return_value_used?(node) || return_value_of_scope?(node)
+        end
+
+        def procedural_method?(method_name)
+          procedural_methods.include?(method_name)
+        end
+
+        def return_value_used?(node)
+          return unless node.parent
+
+          # If there are parentheses around the block, check if that
+          # is being used.
+          if node.parent.begin_type?
+            return_value_used?(node.parent)
+          else
+            Util::ASGN_NODES.include?(node.parent.type) ||
+              node.parent.send_type?
+          end
+        end
+
+        def return_value_of_scope?(node)
+          return unless node.parent
+
+          conditional?(node.parent) || node.parent.children.last == node
+        end
+
+        def procedural_methods
+          Array(cop_config['ProceduralMethods']).map(&:to_sym)
+        end
+
+        def functional_methods
+          Array(cop_config['FunctionalMethods']).map(&:to_sym)
+        end
+
+        def ignored_methods
+          Array(cop_config['IgnoredMethods']).map(&:to_sym)
+        end
+
+        def conditional?(node)
+          node.if_type? || node.or_type? || node.and_type?
+        end
       end
     end
   end