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

Rails - rubocop doesn't like syntax for model scopes #373

Closed
kaldrenon opened this issue Jul 17, 2013 · 10 comments · Fixed by #378
Closed

Rails - rubocop doesn't like syntax for model scopes #373

kaldrenon opened this issue Jul 17, 2013 · 10 comments · Fixed by #378

Comments

@kaldrenon
Copy link

In my rails app, I have scopes that use the following syntax:

scope :foo, lambda { |f|
  where(condition: "value")
}

And when I rubocop my models, I get C: Avoid using {...} for multi-line blocks.

When I replace the {...} form with a do..end form, my scopes don't work.

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 17, 2013

Ah, yes - {} binds more tightly than do..end and in the second case the code is interpreted as:

scope(:foo, lambda) do |f|
  where(condition: 'value')
end

@jonas054 Will you please take care of this bug?

@yujinakayama
Copy link
Collaborator

As well, also this case:

expect { do_something }.to raise_error(ErrorClass) { |error|
  # ...
}

@jonas054
Copy link
Collaborator

@bbatsov @yujinakayama Yes. But how do we want RuboCop to act in these cases? Should it conclude that changing the braces to do..end and adding parentheses to overcome the difference in binding would give awkward code? My initial reaction is that the inspected code should be changed to

scope :foo, (lambda do |f|
               where(condition: 'value')
             end)

and

expect { do_something }.to(raise_error(ErrorClass) do |error|
                             # ...
                           end)

respectively.

@yujinakayama
Copy link
Collaborator

I was thinking about making RuboCop accept {..} if changing it to do..end leads to a different meaning of the syntax. Anyway I think most users don't accept such awkward codes, since DSL is a beautiful part of Ruby.

@jonas054
Copy link
Collaborator

That makes sense. I'll try to implement it.

@deivid-rodriguez
Copy link
Contributor

@jonas054 I'm still experiencing this in RuboCop 0.27.1. Is this fixed?

@jonas054
Copy link
Collaborator

@deivid-rodriguez Are you really? We're checking a few cases in the spec for the Blocks cop. See https://github.com/bbatsov/rubocop/blob/master/spec/rubocop/cop/style/blocks_spec.rb#L52-L66

Do you have some example code where RuboCop still reports braces that actually can't be changed to do..end?

@deivid-rodriguez
Copy link
Contributor

Not actually, my bad. Don't know why I thought the resolution was allowing the -> syntax in the ambiguous cases. Just reread this issue and I really don't know where I got that idea from... Again, sorry.

@bishisht
Copy link

@jonas054 I am still experiencing this. I ran this through the code for https://github.com/VisitMeet/visitmeet and it showed me the error everyone is having here.

@jonas054
Copy link
Collaborator

@bishisht Please open a new issue with the details of your problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants