RuboCop default_scope format #1895
Comments
|
To be honest I didn't factor Mongoid at all in this check. All the Rails cops that are model-related assume you're using ActiveRecord. |
|
I'm using ActiveRecord and was coming here to ask if the cop is working correctly, I'll add this here instead of opening a new issue. While working through our TODO's I've activated this cop on a app with several According to the docs (http://api.rubyonrails.org/classes/ActiveRecord/Scoping/Default/ClassMethods.html#method-i-default_scope):
So I wonder if this is a cop that's targeting an older version of Rails? |
|
@DMA57361 I think it's just a bit more concise to use a block. RC is a style checker. Naturally, code which fails its style checks may still "work". |
|
@alexdowad I think it's less that a style warning is working code that's a problem (it clearly isn't), but more that the cop states "default_scope expects a block as its sole argument", which is inaccurate - especially in the case originally raised, where that does not even work. If I have a |
|
Guess this was the case when I wrote the cop. It might not be relevant anymore. I don't do much Rails programming these days. |
|
@DMA57361 Thanks for further explanation. Can you suggest how this cop should be adjusted? Or do you think it should simply be removed? |
|
I can only comment in terms of ActiveRecord, when used in Rails 4.1 and 4.2 (2 most recent big releases) I'd make Rubocop suggest the use of lambdas as the recommended usage for both (I've roughly said this already, but to keep it together:) This is to avoid unexpected behaviour of the block versions being evaluated at load-time instead of call-time. For example, a scope of |
|
Maybe those cops should simply be dropped. In general I regret introducing the Rails cops, as they are pretty unreliable and susceptible to changes in Rails. Ideally they should be spun into a separate gem, but I'd be fine with simply deleting them... |
|
I'd be sad if the Rails cops were dropped altogether, as we use them in our Hiving them off to a separate gem might make it easier to keep up to date
|
|
The Rails cops are definitely going into another gem. However, the question still remains of what to do with It's been a long time since I developed Rails code. So I had to refresh my memory on how There are 3 forms: default_scope where(a: 1)
default_scope { where(a: 1) }
default_scope -> { where(a: 1) }I don't see any reason to enforce the use of any one of these forms. Rails programmers on a team can pick whichever style seems more appropriate at each call site. (As @DMA57361 pointed out, whether or not they want to re-evaluate the arguments every time will affect their choice.) I recommend that this cop can be dropped, but the other Rails cops be retained until we are ready to split them off. |
|
I'm fine with killing it. |
|
P.S. For me the usage of |
|
Pushing a fix. |
|
Btw, it seems this cop was introduced because of a change from Rails 3 to Rails 4 http://stackoverflow.com/questions/18506038/rails-4-default-scope Unfortunately my memory is a bit fuzzy. |
Active Record's `default_scope` method can be used in 3 ways:
default_scope where(a: 1)
default_scope { where(a: 1) }
default_scope -> { where(a: 1) }
The `DefaultScope` cop enforced the use of the 2nd form. This is unnecessary,
and causes problems, since in some situations only the 3rd form will do.
(To be specific, in cases where the arguments must be re-evaluated every time
records are retrieved using the default scope, only the 3rd form will work.)
Therefore, remove this cop.
The DefaultScope cop was removed: rubocop/rubocop#1895 The TrailingComma cop was split into two: rubocop/rubocop@a0719f9
Hi I'm working with MongoId but I'm having some issues with the recommended format. how can I
fixed, or is MongoId's problem?, thanks
here is my stack trace
The text was updated successfully, but these errors were encountered: