Rubocop: New cop: Style/ReturnNilCondition

[Ana06]

Introduce a new cop, which could be called Style/ReturnNilCondition, to avoid only returning nil in a condition branch.

Examples

bad

user.present? ? user.name : nil

if user.present?
  user.name
else
  nil
end

emails.empty? nil : emails.count

if emails.empty?
  nil
else
  emails.count
end

good

user.name if user.present?

emails.count unless emails.empty?

[Ana06]

I think there is not a cop which complains about that already :thinking:

[Edouard-chin]

FYI there was some interesting discussion here on using else nil which is sometimes preferred and intentional.

Not saying that we shouldn't do this cop but just wanted to give some context

[dvandersluis]

FYI Style/EmptyElse will catch

if user.present?
  user.name
else
  nil
end

but it doesn't look like anything catches any of the other examples.

[bbatsov]

That's a great cop idea! Returning nil has ruined too many apis. :-)

@dvandersluis Yeah, the point of the other cop is to just to encourage/discourage implicit else branches. It's not related to what's suggested here.

[Drenmi]

That's a great cop idea! Returning nil has ruined too many apis.

If I understand correctly, this won't disallow returning nil, just making the return value implicit. 😅

[bbatsov]

Ops, I misread the ticket then. Well, now I'm not that excited anymore. 🤣

[bbatsov]

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.

[Ana06]

:sob: