I like this, its a common pitfall I've encountered many times myself and is most likely not what the programmer intended.

Let's add this as a test (works fine already, no offense):

@current_user ||= User.find_by(id: session[:user_id]) || User.anon

@current_user ||= session[:user_id] ? User.find_by(id: session[:user_id]) : nil

and this pattern (also fine, correct autocorrect):

@current_user ||= User.find_by(id: session[:user_id]) unless session[:user_id].nil?

For what I presume would be the most common case, I'd write the correction as follows:

def current_user
  @current_user ||= User.find_by(id: session[:user_id])
end

# =>

def current_user
  return @current_user if instance_variable_defined?(:@current_user)

  @current_user = User.find_by(id: session[:user_id])
end

I just find it easier to parse than the if/else assignment to a single variable, but that only really works when the memoization is the only line of code, so eh.

I would exclude controllers from being inspected by this cop. They execute once and don't benefit from changing the code.. Well, actions wouldn't but those aren't the only methods a controller can have. I guess my thought here doesn't really work out.

If this is a new cop, consider making a corresponding update to the Rails Style Guide.

Can you open to the Rails Style Guide first?

Thanks, I just proposed a pull request to rubocop/rails-style-guide:

• Add find-by-memoization section rails-style-guide#358

Just updated. The same is true of a lot of belongs_to or has_one methods, e.g. @country ||= @user.country where country fires an SQL query to fetch the record, but could be nil. But without type analysis, I imagine this would be impossible to detect.