Rubocop: Change AndOr cop to allow `and return` in controllers

Created on 6 Feb 2018  路  4Comments  路  Source: rubocop-hq/rubocop

I find myself disabling the Style/AndOr cop from running against rails controllers because render and return does render and stop execution, whereas render && return performs a Boolean comparison instead.

Maybe this should happen only when Rails: Enabled: true?

Expected behavior

I would expect Rubocop to raise an alert when using if something and something_else, but not when using render and return (or redirect_to some_url and return).

Actual behavior

Instead, Rubocop raises an alert in all situations, so I add the following to my .rubocop.yml:

Style/AndOr:
  Exclude:
    - 'app/controllers/**/*'

Steps to reproduce the problem

class MyController < ApplicationController
    def new
        do_something if params[:guard] and params[:something_else] # This should cause an alert
        render and return unless params[:guard] # This should *not* cause an alert
        # Some code that should only run when :guard is true
    end
end

RuboCop version

0.52.1 (using Parser 2.4.0.2, running on ruby 2.3.3 x86_64-darwin16)

duplicate

Most helpful comment

Expected behavior
I would expect Rubocop to raise an alert when using if something and something_else, but not when using render and return (or redirect_to some_url and return).

You can achieve this by configuring

Style/AndOr:
  EnforcedStyle: conditionals

because render and return does render and stop execution, whereas render && return performs a Boolean comparison

The difference between and and && is only precedence, but you're right that there are situations in which you can't just replace an and with a && for example. The autocorrect method in AndOr is aware of this and adds parentheses where needed, but this knowledge is not reflected in the offense message.

I wonder if the Rails people would be ok with adding something along the lines of render_and_return

I'm not sure if I follow, but if you mean a method that causes the calling method to return right after the method (e.g., render_and_return) returns to its caller, I don't think that's possible.

All 4 comments

Check out https://github.com/bbatsov/rubocop/issues/1288 and https://github.com/bbatsov/rubocop/issues/1083 for some previous discussions on this topic.

Ah, sorry for the duplicate, my search had not surfaced these issues. I wasn't aware that the render and return syntax was kinda hacky, I'll try to use something else.

# this works as expected, is syntactically valid, and pleases Rubocop
unless params[:guard]
    render
    return
end
# this is hacky and offends Rubocop 鈥攂ut it reads much better
render and return unless params[:guard]

Still, I persist in thinking the cop message should be changed because it suggests changes that cause bugs in a pretty common setup, causing much head-scratching for newcomers, all the more so because the Thou shalt use a guard clause cop (rightfully) barges in in many situations where we try to avoid and return.
I'd suggest that the cop message should recommend placing the render and return statements on consecutive lines, which seems to me the simplest way to circumvent this.

I wonder if the Rails people would be ok with adding something along the lines of render_and_return (and render_file_and_return, redirect_to_and_return, and so on), which would clearly convey the author intent while costing exactly the same number of keystrokes, save the syntax misuse.

Expected behavior
I would expect Rubocop to raise an alert when using if something and something_else, but not when using render and return (or redirect_to some_url and return).

You can achieve this by configuring

Style/AndOr:
  EnforcedStyle: conditionals

because render and return does render and stop execution, whereas render && return performs a Boolean comparison

The difference between and and && is only precedence, but you're right that there are situations in which you can't just replace an and with a && for example. The autocorrect method in AndOr is aware of this and adds parentheses where needed, but this knowledge is not reflected in the offense message.

I wonder if the Rails people would be ok with adding something along the lines of render_and_return

I'm not sure if I follow, but if you mean a method that causes the calling method to return right after the method (e.g., render_and_return) returns to its caller, I don't think that's possible.

I'm closing this ticket due to no recent activity. Feel free to re-open it if you ever come back to it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

NobodysNightmare picture NobodysNightmare  路  3Comments

mikegee picture mikegee  路  3Comments

benoittgt picture benoittgt  路  3Comments

tedPen picture tedPen  路  3Comments

millisami picture millisami  路  3Comments