[FIX #3931] Implement a cop checking for ambiguous block association …

.gitignore

@@ -45,7 +45,7 @@ TAGS
 #.\#*
 
 # For vim:
-#*.swp
+*.swp
 
 # For redcar:
 #.redcar

.rubocop.yml

@@ -24,3 +24,7 @@ Metrics/BlockLength:
     - 'Rakefile'
     - '**/*.rake'
     - 'spec/**/*.rb'
+
+AmbiguousBlockAssociation:
+  Exclude:
+    - 'spec/**/*.rb'

CHANGELOG.md

@@ -15,6 +15,7 @@
 * [#4019](https://github.com/bbatsov/rubocop/pull/4019): Make configurable `Style/MultilineMemoization` cop. ([@pocke][])
 * [#4018](https://github.com/bbatsov/rubocop/pull/4018): Add autocorrect `Lint/EmptyEnsure` cop. ([@pocke][])
 * [#4028](https://github.com/bbatsov/rubocop/pull/4028): Add new `Style/IndentHeredoc` cop. ([@pocke][])
+* [#3931](https://github.com/bbatsov/rubocop/issues/3931): Add new `Lint/AmbiguousBlockAssociation` cop. ([@smakagon][])
 * Add new `Style/InverseMethods` cop. ([@rrosenblum][])
 * [#4038](https://github.com/bbatsov/rubocop/pull/4038): Allow `default` key in the `Style/PercentLiteralDelimiters` cop config to set all preferred delimiters. ([@kddeisz][])
 
@@ -2659,3 +2660,4 @@
 [@droptheplot]: https://github.com/droptheplot
 [@wkurniawan07]: https://github.com/wkurniawan07
 [@kddeisz]: https://github.com/kddeisz
+[@smakagon]: https://github.com/smakagon

config/enabled.yml

@@ -1059,6 +1059,13 @@ Metrics/PerceivedComplexity:
 #################### Lint ##################################
 ### Warnings
 
+Lint/AmbiguousBlockAssociation:
+  Description: >-
+                 Checks for ambiguous block association with method when param passed without
+                 parentheses.
+  StyleGuide: '#syntax'
+  Enabled: true
+
 Lint/AmbiguousOperator:
   Description: >-
                  Checks for ambiguous operators in the first argument of a

lib/rubocop.rb

@@ -121,6 +121,7 @@
 require 'rubocop/cop/bundler/duplicated_gem'
 require 'rubocop/cop/bundler/ordered_gems'
 
+require 'rubocop/cop/lint/ambiguous_block_association'
 require 'rubocop/cop/lint/ambiguous_operator'
 require 'rubocop/cop/lint/ambiguous_regexp_literal'
 require 'rubocop/cop/lint/assignment_in_condition'

lib/rubocop/cop/lint/ambiguous_block_association.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Lint
+      # This cop checks for ambiguous block association with method
+      # when param passed without parentheses.
+      #
+      # @example
+      #
+      #   # bad
+      #   some_method a { |val| puts val }
+      #
+      # @example
+      #
+      #   # good
+      #   # With parentheses, there's no ambiguity.
+      #   some_method(a) { |val| puts val }
+      class AmbiguousBlockAssociation < Cop
+        MSG = 'Parenthesize the param `%s` to make sure that the block will be'\
+              ' associated with the `%s` method call.'.freeze
+
+        def on_send(node)
+          return if node.parenthesized? || node.assignment? || node.method?(:[])
+
+          return unless method_with_block?(node.first_argument)
+          first_param = node.first_argument.children.first
+          return unless method_as_param?(first_param)
+
+          add_offense(node, :expression, message(first_param, node.method_name))
+        end
+
+        private
+
+        def method_with_block?(param)
+          return false unless param
+
+          param.block_type?
+        end
+
+        def method_as_param?(param)
+          return false unless param
+
+          param.send_type? && !param.arguments?
+        end
+
+        def message(param, method_name)
+          format(MSG, param.children[1], method_name)
+        end
+      end
+    end
+  end
+end

manual/cops.md

@@ -74,6 +74,7 @@ In the following section you find all available cops:
 
 #### Department [Lint](cops_lint.md)
 
+* [Lint/AmbiguousBlockAssociation](cops_lint.md#lintambiguousblockassociation)
 * [Lint/AmbiguousOperator](cops_lint.md#lintambiguousoperator)
 * [Lint/AmbiguousRegexpLiteral](cops_lint.md#lintambiguousregexpliteral)
 * [Lint/AssignmentInCondition](cops_lint.md#lintassignmentincondition)

manual/cops_lint.md

@@ -1,5 +1,33 @@
 # Lint
 
+## Lint/AmbiguousBlockAssociation
+
+Enabled by default | Supports autocorrection
+--- | ---
+Enabled | No
+
+This cop checks for ambiguous block association with method
+when param passed without parentheses.
+
+### Example
+
+```ruby
+# bad
+
+# It's ambiguous because there is no parentheses around `a` param
+some_method a { |val| puts val }
+```
+```ruby
+# good
+
+# With parentheses, there's no ambiguity.
+some_method(a) { |val| puts val }
+```
+
+### References
+
+* [https://github.com/bbatsov/ruby-style-guide#syntax](https://github.com/bbatsov/ruby-style-guide#syntax)
+
 ## Lint/AmbiguousOperator
 
 Enabled by default | Supports autocorrection

spec/rubocop/cop/lint/ambiguous_block_association_spec.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Lint::AmbiguousBlockAssociation do
+  subject(:cop) { described_class.new }
+  subject(:error_message) { described_class::MSG }
+
+  before { inspect_source(cop, source) }
+
+  shared_examples 'accepts' do |code|
+    let(:source) { code }
+
+    it 'does not register an offense' do
+      expect(cop.offenses).to be_empty
+    end
+  end
+
+  it_behaves_like 'accepts', 'some_method(a) { |el| puts el }'
+  it_behaves_like 'accepts', 'some_method(a) do;puts a;end'
+  it_behaves_like 'accepts', 'some_method a do;puts "dev";end'
+  it_behaves_like 'accepts', 'some_method a do |e|;puts e;end'
+  it_behaves_like 'accepts', 'Foo.bar(a) { |el| puts el }'
+  it_behaves_like 'accepts', 'env ENV.fetch("ENV") { "dev" }'
+  it_behaves_like 'accepts', 'env(ENV.fetch("ENV") { "dev" })'
+  it_behaves_like 'accepts', '{ f: "b"}.fetch(:a) do |e|;puts e;end'
+  it_behaves_like 'accepts', 'Hash[some_method(a) { |el| el }]'
+  it_behaves_like 'accepts', 'foo = lambda do |diagnostic|;end'
+  it_behaves_like 'accepts', 'Proc.new { puts "proc" }'
+  it_behaves_like('accepts', 'expect { order.save }.to(change { orders.size })')
+  it_behaves_like(
+    'accepts',
+    'allow(cop).to receive(:on_int) { raise RuntimeError }'
+  )
+  it_behaves_like(
+    'accepts',
+    'allow(cop).to(receive(:on_int) { raise RuntimeError })'
+  )
+
+  context 'without parentheses' do
+    context 'without receiver' do
+      let(:source) { 'some_method a { |el| puts el }' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to(
+          eq(format(error_message, 'a', 'some_method'))
+        )
+      end
+    end
+
+    context 'with receiver' do
+      let(:source) { 'Foo.some_method a { |el| puts el }' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to(
+          eq(format(error_message, 'a', 'some_method'))
+        )
+      end
+    end
+
+    context 'rspec expect {}.to change {}' do
+      let(:source) do
+        'expect { order.expire }.to change { order.events }'
+      end
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to(
+          eq(format(error_message, 'change', 'to'))
+        )
+      end
+    end
+
+    context 'as a hash key' do
+      let(:source) { 'Hash[some_method a { |el| el }]' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to(
+          eq(format(error_message, 'a', 'some_method'))
+        )
+      end
+    end
+
+    context 'with assignment' do
+      let(:source) { 'foo = some_method a { |el| puts el }' }
+
+      it 'registers an offense' do
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to(
+          eq(format(error_message, 'a', 'some_method'))
+        )
+      end
+    end
+  end
+end