Documentation for Lint/AmbiguousBlockAssociation cop is confusing

[skalee]

Actual behaviour

Given following file test.rb:

# frozen_string_literal: true

some_method a { |val| puts val }

When I execute command:

rubocop --force-default-config test.rb

I see following:

Inspecting 1 file
W

Offenses:

test.rb:3:1: W: Lint/AmbiguousBlockAssociation: Parenthesize the param a { |val| puts val } to make sure that the block will be associated with the a method call.
some_method a { |val| puts val }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

What documentation says

Examples:

# bad
some_method a { |val| puts val }

# good
# With parentheses, there's no ambiguity.
some_method(a) { |val| puts val }

# good
# Operator methods require no disambiguation
foo == bar { |b| b.baz }

# good
# Lambda arguments require no disambiguation
foo = ->(bar) { bar.baz }

(https://rubocop.readthedocs.io/en/latest/cops_lint/#lintambiguousblockassociation)

What is the problem

The documentation describes a different case than the cop. Given aforementioned code sample:

some_method a { |val| puts val }

The cop message says that this line should be fixed in a following way:

some_method(a { |val| puts val })

Whereas documentation suggests that it should rather be fixed in following way:

some_method(a) { |val| puts val }

Which is syntactically and semantically different.

Obviously, both suggested resolutions are correct in some cases and incorrect in others. Applying any of them results with 1 file inspected, no offenses detected. Nevertheless, this discrepancy is somewhat confusing.

Suggestion 1

IMHO cop documentation should illustrate both "good" resolutions, like in following:

# good
# With parentheses, there's no ambiguity.
some_method(a) { |val| puts val }
# or
some_method(a { |val| puts val })

I have no suggestion how to improve the cop message. It should be succinct, so perhaps the current one is good enough.

Suggestion 2

Given the fact that this cop often reports a false positive for a popular RSpec idiom:

expect { something }.not_to change { some_variable }

Documentation could propose to disable this cop for spec files (see #4222).

RuboCop version

0.76.0 (using Parser 2.6.5.0, running on ruby 2.6.3 x86_64-darwin17)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

[marcandre]

Agreed. Actually some_method(a { |val| puts val }) should come first, as it is the equivalent to some_method a { |val| puts val }