consistent_return.md

As developers it seems like we're in a land of plenty when it comes to code quality tools. There are many options, some more controversial than others. One such option I'd like to talk about today is the use of ESLint in Javascript with the consistent-return rule.

When you enable the consistent-return rule in ESLint the tool will automatically check that all code paths in a function either explicitly return a value or don't return a value. In functions that branch one might forget to return a value in one of the branches, and the consistent-return rule will warn you when that happens.

The rule can be controversial though, because it disallows using return statements as control flow. I think this is the biggest benefit of the consistent-return rule, but others on my team think this is a large drawback worth disabling the rule. Consider the following code to charge an order:

class Order {
  charge() {
    // if there's no items, then there's nothing to charge
    if(this.items.length === 0) { return; }

    // charge order
    const chargeToken = magicChargeHelper(this);

    return chargeToken;
  }
}

The above code attempts to charge an order. If there aren't any items in the order it returns early, skipping the code that actually charges the order. The consistent-return rule will flag this code as an error, because we aren't returning a value from the conditional branch. We're using inconsistent return semantics. To remove the flag you simply return a value from that branch. Any value will do, even null or undefined. Doing so gives us back consistent return semantics; we're consistently using return to pass values rather than using it as control flow and also using it to pass values.

While this satisfies the letter of the rule, it does not satisfy the spirit of the rule. When this rule flags your code it's really telling you that you might not have considered every edge case in your code. In the above example, I would be very concerned about how orders can ever have no items in them. There might be a data corruption issue that is hiding somewhere that we need to address. Or this might be a bug fix that doesn't quite address the root cause. I would only fall back to this kind of conditional guard if I'd exhausted all other options. Even then I would make sure I keep this code in my working memory as it might be a source of problems later.

I think that the consistent-return rule for ESLint is a valuable rule to enable on all of my Javascript projects. It keeps me honest and forces me to consider all possible edge cases. This helps to cut down on potential sources of bugs and leads me to a better architecture. While it can be irritating at times, when you consider it in this light I think the pain is worth it.