Rubocop: Suggestion: Ban AndOr almost everywhere

[nathany]

Not only conditionals, but in assignments... that's where I've ran into the most trouble.

a = b() and c() # => a is the result of calling b(), c() doesn't impact the result even when called

But it is nice for "do this, then that" in the typical Rails case:

redirect_to login_path and return

[yujinakayama]

AndOr already checks for and and or everywhere by default.

[nathany]

I don't want it to complain about `and return though.

[mikegee]

I think you are saying and is always bad unless the only thing after and is return. Is that right?

[mikegee]

The rails pattern of render/redirect and return is pet peeve of mine. I kinda hate it. Why not put the second statement on its own line like every other pair of statements in your entire application?

[nathany]

@mikegee Yes, and we certainly could do that. Though and/or exist for such control flow:
http://devblog.avdi.org/2010/08/02/using-and-and-or-in-ruby/

I suppose we may just leave this cop off, or perhaps opt for the conditionals one and see how it goes.

[mikegee]

I wasn't clear. I love using and/or for control flow. The rails redirect/render and return pattern is not that. You are not conditionally returning. You are depending on the return value of redirect/render to be truthy so that return is called.

[nathany]

Now that you say it, it does sound a bit odd. :grimacing:

[jonas054]

I doubt if we're ever going to make any progress here. If we could find a way to differentiate between proper control flow usage and other less desirable usage of and/or, that would be great, but I don't see any way to do that (except that some level is achieved by the two enforced styles we support today). So I suggest we close the issue.

[nathany]

That's fine.

[bbuchalter]

I know this is an old, forgotten issue, but perhaps a pragmatic solution is to not blindly suggest that && can replace and in all usages? I think with the current message, we're likely to introduce bugs in folks applications. Any interest in changing the message?

[nathany]

@bbuchalter Instead of commenting on a closed issue, it's best to open a new issue and link to the closed issue. That way your comment doesn't get lost.

[webzorg]

The other way of doing this redirect_to root_path and return if current_user.id == params[:id].to_i
is to use an if block which would be 4 lines. This is a legitimate use case of using and, where && is just a syntax error...

[jonas054]

That expression is allowed with the configuration

Style/AndOr:
  EnforcedStyle: conditionals

But note that your specific example was criticized above my @mikegee as it depends on a truthy return value from redirect_to, which it happens to give, but it's not documented behavior.