Rubocop: AndOr sees `render and return` as a violation

[itavero]

How can I configure RuboCop to no longer see the code below as a violation to the AndOr check?

render nothing: true, status: :bad_request and return

Currently it tells me to replace and with &&, which is not correct as far as I know.

[bbatsov]

It's not possible at the moment. Either disable the cop or add parens to render.

[mikegee]

I prefer to put the return on the next line. I'm not sure why and return became ok after render & redirect_to in the Rails community. It looks like a hack to me.

Also, I think render nothing: true, status: :bad_request is the same as head :bad_request.

[bbatsov]

Seems that we should solve this in a similar manner to https://github.com/bbatsov/rubocop/pull/1047. @jonas054 How does this sound?

[yujinakayama]

@bbatsov I think this is not an auto-correction issue. Also I remember the auto-correction already handles it (https://github.com/bbatsov/rubocop/pull/415).

I agree with @mikegee. If replacing an and with a && changes the semantics, the and is ambiguous and might be a cause of bug. So I think the current behavior is right since that's what we want to detect, though there may be room for improvement in the offense message.

[mikegee]

And I agree with @yujinakayama. The message is really the only problem here.

Here's another point of view:

render and return implies you don't want to return if render returns false or nil. I don't think that is what you mean with render and return. render; return is more likely your intent.

What I'm implying here is that rubocop is doing a fine job of enforcing good practices, and render and return is not a good practice.

[jonas054]

[_Edit:_ Didn't see the comments from @yujinakayama and @mikegee before I hit Comment. Seems like the three of us agree that current behavior is fine.]

I'm not so sure. If I understand you correctly, @bbatsov, you're proposing a configuration option that would allow

func arg and action

but still report an offense for

func and action

I think that's too inconsistent. The current behavior is fine IMO; the solution is

func(arg) && action

or maybe

action if func arg

and if you want to use keywords and/or for control flow or anything else, @itavero, you have to disable the AndOrcop.

[bbatsov]

OK, closing this.

[paulschreiber]

I naively switched some of these to render "foo" && return which _definitely_ doesn't work.

Sometime I can do this:

if @something
  render "foo"
  return
end

…but sometimes you end up with an error about not using a guard statement.

I ended up with some code like this:

def create
  ...
  render :create unless @person.save
  render 'thanks'
end

...which seems clunky and unnatural when compared to:

def create
  ...
  render 'thanks' and return if @person.save
end

How would you write this?

[bbatsov]

Obviously the simple solution is to add parentheses when needed.

[paulschreiber]

Where would the parens go in this case?

[mikegee]

def create
  ...
  render('thanks') && return if @person.save
end

Or

def create
  ...
  (render 'thanks') && return if @person.save
end

[krishantaylor]

This conflicts with the current Rails guides, meaning this issue should probably be reopened.

Make sure to use and return instead of && return because && return will not work due to the operator precedence in the Ruby Language.
http://guides.rubyonrails.org/layouts_and_rendering.html#avoiding-double-render-errors

[mikegee]

I'd rather see the guide changed to promote good practice rather than a common workaround.

The example there could be:

def show
  @book = Book.find(params[:id])
  if @book.special?
    render action: "special_show"
    return
  end
  render action: "regular_show"
end

or

def show
  @book = Book.find(params[:id])
  if @book.special?
    render action: "special_show"
  else
    render action: "regular_show"
  end
end

[wjdhamilton]

Part of the problem here is that the Rails community have decided that the and operator has different semantics than that which the and operator was originally intended for. Whereas && is still viewed as the logical and, and has come to mean 'do something and then do something else'.

[uri]

I don't think it's fair to call and return a hack or a workaround. Sure and/or can cause developers grief when used in a conditional, but I think they have a place when used as flow control operators (as in the case of and return). I've always had the impression that and return was considered idiomatic Ruby.

[jonas054]

@uri It depends. If your intention is to always return and you're using and as a more visually pleasing alternative to ;, depending on the left hand side to always return a truthy value, then I think it's an ugly hack. @mikegee said this earlier in the discussion.

[uri]

If your intention is to always return and you're using and as a more visually pleasing alternative to ;

This is not the case. I agree, if your intention is to always return then yes, use ;. Otherwise you have some kind of expectation from the left-hand side and you can use and/or to do something:

pass_or_fail and "it passed"
pass_or_fail or "it failed"
pass_or_fail ; "unknown"

That's a trivial example which can easily be replaced with if/unless but I hope it illustrates that ; is not and.

It's true that the intention for render is to always return. But by using ; you're not chaining the operations together. This is evident if you have a conditional:

some_conditional = false
render 'new' ; return if some_conditional
# ...
render 'form' # double render