Rubocop: Forcing explicit returns (optionally!)

[uberbrady]

I understand completely that 'standard ruby style' prefers implicit returns to explicit ones, and that's fine. However, I prefer explicit returns, and don't actually like implicit ones (except, maybe, in a short one-liner function, perhaps?).

I was wondering if a new (optional, off-by-default) setting could be added to RedundantReturn, or somewhere else, to help enforce this style preference that we have? (And, perhaps, we could rename it to something like ExplicitReturns with a default value of 'forbid', an optional value of 'require', or maybe something like 'disabled' if you don't care either way?)

If this is something that people are interested in, I'd be happy to help code something up.

Expected behavior

I'd like to require explicit returns in all of our ruby code.

Actual behavior

The only option I can do is disable the RedundantReturn rule.

Steps to reproduce the problem

Write code with an explicit return at the end of a function or method. Run rubocop. Get error telling you return is redundant.

RuboCop version

Include the output of rubocop -V. Here's an example:

rubocop -V
0.47.1 (using Parser 2.4.0.0, running on ruby 2.3.0 x86_64-darwin15)

[Drenmi]

RuboCop has moved away from only enforcing the community preferred style, towards being almost infinitely configurable, so this is not a far reach. The caveat is you normally need to convince someone of the value, to have them work on it (or do it yourself, which is always an option.) 🙂

I personally prefer implicit returns, but sometimes I end up with an awkward looking method that has a mix of explicit (non guard clause) returns at the start, and an implicit one at the end, and it almost always irks me. 😉

[uberbrady]

I'm happy to do it myself (or to make my team do it for me:) ) I just want to be sure that if we did, it would actually get merged - and to make sure that the maintainers got to weigh in on any details of how to do it, too.

[bbatsov]

I'm fine with adding this, but the cop should probably be renamed indeed. I'd go with a name like MethodReturn.

[noraj]

Redundant return is not Explicit return. So yes I can disable RedundantReturn.
But I want to keep check about redundant return but allow explicit returns. It's not the same thing, there should be RedundantReturn and ExplicitReturns.

[revolter]

Was there a PR created for this?

[bbatsov]

@revolter No, it wasn't.

[revolter]

Couldn't there be created a generic solution for this kind of issue? Asking because I'm having the same exact problem with some of the other cops. I can disable them, but then they allow either style. I'd like to enforce the opposite of them instead.

[bbatsov]

I'm totally for having cops customisable, but this can't happen automatically. It always requires some amount of manual work. Don't see a way to magically support opposing styles, as there are often some nuances to them.

[revolter]

Ok, thanks for the info. Is there a way to push this idea then? Some examples (without having experience with creating cops):

• Generate an OppositeName cop when running bundle exec rake new_cop[Department/Name] and add a new step to the instructions: 4. Rename the "opposite" cop accordingly and then implement it in the generated file!
• Simply add a warning on that rake task like: It is strongly recommended to create a cop that enforces the opposite of the main cop's rule!
• Create a list for cops that support or not an opposite variant so users know exactly the current state
• Add an automatic warning on a cop adding PR that tells the OP about opposite cops

[bbatsov]

I certainly don't think we need different/opposite cops - just configurable styles in cops (what we are normally doing most of the time). Lately I've been asking everyone introducing such cops to supply both styles, but for older code we simply did not think of this.

For this particular problem - someone just needs to patch and rename this cop, which is not complex at all.

Create a list for cops that support or not an opposite variant so users know exactly the current state

Probably it'd be nice if we kept track of all the cops that need to be made configurable. I doubt there are that many of them at this point.

[Darhazer]

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.