`return-early` rule

[kevva]

As discussed in https://github.com/sindresorhus/execa/pull/15#discussion-diff-60815913. How I think when coding is to handle errors and exceptions as soon as possible and return early. If we take a look at the two examples below, the latter one makes more sense to me. foobar() in this case is the "real" body while the code in the if statements deal with exceptions. When using the style in the former one, this isn't as clear to me.

if (conditon) {
    someFn();
} else if (someOtherCondition) {
    someOtherFn();
} else {
    foobar();
}

if (condition) {
    someFn();
    return;
}

if (someOtherCondition) {
    someOtherFn();
    return;
}

foobar();

This might be worth reading too http://blog.timoxley.com/post/47041269194/avoid-else-return-early.

[SamVerschueren]

Like that style as well! Would be nice.

[jfmengels]

:+1: for the idea.

I found this recently https://github.com/Shopify/javascript/blob/master/packages%2Feslint-plugin-shopify%2Fdocs%2Frules%2Fprefer-early-return.md, which handles a related case.

I would also handle the following:

// handled by the above mentioned plugin
function foo() {
  if (condition) {
     ...
  }
  // nothing else
}

but not

function foo() {
  let value;
  if (condition) {
     value = X;
  } else if (condition2) {
     value = Y;
  } else {
     value = Z;
  }
  // do something with value like
  return foo(value);
  // return value or anything else
}

// the alternative would be
function foo() {
  let value;
  if (condition) {
     return foo(X);
  } else if (condition2) {
     return foo(Y);
  }
  return foo(Z);
}
// which is longer to refactor

[jfmengels]

I think I've already been bitten by http://eslint.org/docs/rules/no-negated-condition, that made it not possible to return early, so in XO you may have to relax the rule a bit (or create a new rule that handles both cases?).

[jamestalmage]

no-negated-condition does not prevent the style @kevva is proposing. It just tells you to un-negate the condition and flip the consequent and alternate blocks.

[jfmengels]

True, but it does in the case of my first example, which I think would be nice to also handle in this plugin (though it doesn't have to as there already exists a rule for it).

[jamestalmage]

I disagree with the rule as proposed. I do agree with the sentiments from @timoxley's blogpost. But those are different from the code being discussed in https://github.com/sindresorhus/execa/pull/15#discussion-diff-60815913.

In Tim's blog he recommends against this:

function (err, result) {
  if (err) {
    handleError(err);
  } else {
    processTheResult(result);
    processTheResultMore(result);
    stillMore();
    // ...
  }
}

In that case I agree with the refactor to use a return statement. You are saying: "If there is an error,handle it, but ignore the rest of this - the remaining function is moot because there is an error". Also, for every line you get to un-indent in that else block, using the return adds more value.

Up until this point, I'm in agreement with everyone I think.

Where I disagree is the simple if / else statement referenced:

function (input) {
  if (isStream(input)) {
    handleStream(input);
  } else {
    handleString(input);
  }
} 

In this case you're not bailing (returning) early because there is an error. There are two branches your code could take, and that is clearly reflected with the explicit if / and else blocks. The shape of your code reflects the flow. Adding the return only allows you to un-indent a single line.

IMO - prefer-early-return with some maxStatements set represents a pretty good compromise.

[fregante]

If you don't mind, I'll close this since (it's 4+ years old and) it exists: https://github.com/Shopify/web-configs/blob/841aa44aee288ef809235c7e33c130708d66a6b9/packages/eslint-plugin/docs/rules/prefer-early-return.md

[fregante]

This could/should also apply to continue.

Incorrect

for (x in y)
	if (isGreen(x))
		if (isBold(x))	
			if (isItalics(x))
				console.log('All good')

Correct

for (x in y) {
	if (!isGreen(x))
		continue

	if (!isBold(x))
		continue

	if (!isItalics(x))
		continue

	console.log('All good')
}

[kkmuffme]

There are similar rules for PHP code (https://github.com/slevomat/coding-standard#slevomatcodingstandardcontrolstructuresearlyexit-) and I think it would make a lot of sense here too, especially to avoid nested tons of nested ifs.

Generally: (what I came up with when checking some code samples) if we are in function context and an "if" does not have an "elseif", we can return/continue instead of nesting more. (if it has an elseif, all direct children "if" of if/elseif cannot be changed without breaking the logic, but all children "if" more than 1 level down, can again have this rule applied).

Logic:

• "if" contains an "if" but itself is not directly followed by an "elseif" - code can be simplified to return early/continue.
• "if" contains an "if" but itself is directly followed by an "elseif" - do not give an error for any "if" that are direct children of this "if" (attention: the "if" do not need to be in lines directly after each other, but it's more like parent-child level)

if ( foo )
   if ( bar ) => give error for this
   
   
if ( foo )
   if ( bar )
       if ( qwe ) => give error for this
elseif ( xyz )
   if ( abc )
       if ( bla ) => give error for this

[thigrand]

Any update on this topic? Do anyone know is there a rule like that implemented somewhere?

[belgattitude]

@thigrand might answer your question: https://github.com/Shopify/web-configs/blob/main/packages/eslint-plugin/lib/rules/prefer-early-return.js.

[karlhorky]

@belgattitude (also further up in the thread @jfmengels) nice find! Some additional details:

• Original implementation commit by @lemonmade from 2016 https://github.com/Shopify/web-configs/commit/0f83a64cbcd0f4b5cfe3b97e2be77365e0408f6e
• Docs: https://github.com/Shopify/web-configs/blob/main/packages/eslint-plugin/docs/rules/prefer-early-return.md

[fregante]

I tried that plugin. I think this should be allowed:

function bar() {
	if (foo) {
		onlyStatementHere()
	}
}

It does "unnecessarily" increase indentation, but I think changing it to this is kind of annoying and not any more readable:

function bar() {
	if (!foo) {
		return;
	}

	onlyStatementHere()
}

However in this case it starts to tip the scale a bit:

function bar() {
	if (foo) {
		many()
		other.statements = 1;
		if (here) {
			log(lotsOfLines)
		}
		how = 'many?'
	}
}

Maybe optionally it can allow it as long as there are no further statements outside the condition.

• 👆 allowed ✅
• 👇 not allowed ❌

function bar() {
	if (foo) {
		many()
		other.statements = 1;
		if (here) {
			log(lotsOfLines)
		}
		how = 'many?'
+		return;
	}

+	return 'nah'
}

[kkmuffme]

Just set maximumStatements: 1, in the config for that rule and it won't report an error