Skip to content
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

Lint/AmbiguousBlockAssociation cites unambiguous lambda param #4189

Closed
cainlevy opened this issue Mar 27, 2017 · 6 comments
Closed

Lint/AmbiguousBlockAssociation cites unambiguous lambda param #4189

cainlevy opened this issue Mar 27, 2017 · 6 comments

Comments

@cainlevy
Copy link

When passing a lambda declaration as a method parameter, the new Lint/AmbiguousBlockAssociation cop forces unnecessary parentheses.

Example:

# cited
foo ->(a) { a }

# making the cop happy
foo(->(a) { a })

Note that the method does not generally require parentheses:

# good
foo a

Expected behavior

I expect Lint/AmbiguousBlockAssociation to not cite lambdas passed as params like any other object.

Actual behavior

The new Lint/AmbiguousBlockAssociation cop creates a citation for lambdas passed as params like any other object.

Steps to reproduce the problem

Create file:

foo ->(a) { a }

Run rubocop:

rubocop -D

RuboCop version

$ rubocop -V
0.48.0 (using Parser 2.4.0.0, running on ruby 2.3.3 x86_64-darwin16)
Drenmi added a commit to Drenmi/rubocop that referenced this issue Mar 28, 2017
…bda arguments

This cop would register an offense for lambdas passed as an argument to
a method, suggesting an awkward parentheses placement.

This change fixes that.
@smakagon
Copy link
Contributor

@Drenmi thanks for fixing that! I missed that case...

@ivanovaleksey
Copy link
Contributor

ivanovaleksey commented Apr 3, 2017

I got plain Rails scope like scope :technical, -> { where(technical: true) }
and I have

Parenthesize the param -> { where(technical: true) } to make sure that the block will be associated with the -> method call.

scope(:technical, -> { where(technical: true) }) fixes the problem but is it the way it should be?

scope :technical, (-> { where(technical: true) }) is good too but doesn't it look pretty strange?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 3, 2017

@ivanovaleksey Are you using 0.48.1?

@ivanovaleksey
Copy link
Contributor

@bbatsov, yes

$ bundle exec rubocop -V
0.48.1 (using Parser 2.4.0.0, running on ruby 2.3.3 x86_64-darwin16)

@brandonweiss
Copy link
Contributor

Yeah, I'm seeing the same thing, on 0.48.1 for something like:

scope :vat, -> { where(post_subtotal_type: 'vat') }

@smakagon
Copy link
Contributor

smakagon commented Apr 3, 2017

@brandonweiss @bbatsov yes, there is a problem with lambda check. It's easily fixable. Should I create another PR to this issue?

smakagon added a commit to smakagon/rubocop that referenced this issue Apr 4, 2017
bbatsov pushed a commit that referenced this issue Apr 5, 2017
shimgo added a commit to shimgo/one-progress that referenced this issue May 20, 2017
ラムダをパラメータにしたときにLint/AmbiguousBlockAssociationの違反と
みなされる不具合を回避するため。
rubocop/rubocop#4189

masterにはmergeされているようだが、RubyGemsには来ていないのでそれまでは
個別に回避する。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants