Disallow negated conditions (no-negated-condition)

[feross]

This rule disallows negated conditions in either of the following:

• if statements which have an else branch
• ternary expressions

Examples of incorrect code for this rule:

/*eslint no-negated-condition: "error"*/

if (!a) {
    doSomething();
} else {
    doSomethingElse();
}

if (a != b) {
    doSomething();
} else {
    doSomethingElse();
}

if (a !== b) {
    doSomething();
} else {
    doSomethingElse();
}

!a ? c : b

Examples of correct code for this rule:

/*eslint no-negated-condition: "error"*/

if (!a) {
    doSomething();
}

if (!a) {
    doSomething();
} else if (b) {
    doSomething();
}

if (a != b) {
    doSomething();
}

a ? b : c

http://eslint.org/docs/rules/no-negated-condition

[feross]

Probably too disruptive for a rule that isn't automatically fixable:

1..422
# tests 422
# pass  364
# fail  58

[LinusU]

It would be fun to take a stab at implementing fixing of this rule, it should be technically possible but maybe a bit complicated.

I've been thinking of trying to put some work into getting higher fixable coverage. Producing a table of the rules we use and which are currently fixable or not would be awesome. Hopefully I'll get some time for this soon :)

[feross]

@LinusU Nice, that would be greatly useful!

[dcousens]

Maybe I'm not understanding that example, but what is the benefit of this?

if (!a) {
    doSomething();
} else if (b) {
    doSomething(); // is this meant to be doSomethingElse
}

Is not the same as

if (a != b) {
    doSomething();
} else {
    doSomethingElse();
}

[feross]

Yeah -- the examples aren't meant to be the same. They're just examples of valid and invalid code copied from the eslint docs. Basically, this boils down to: Prefer statements that aren't needlessly negated.

For example, this is worse:

if (!a) {
  notA()
} else {
  isA()
}

Is strictly worse than this:

if (a) {
  isA()
} else {
  notA()
}

[timoxley]

While I agree with this for the trivial examples given, I disagree with this when one of the conditional branches is considerably longer than the other. I find it far better to place the shortest branch body first, this gets that condition out of the way and avoids having any "hidden" else dangling around at the end of the if. Such an else may require some scrolling to discover what if the else belongs to, and can be easily missed when reading the if.

The principle I try to follow is to get as much logic "resolved" and out of the way (i.e. don't need to continue to keep mental track of it) in the fewest possible lines.

e.g.

if (a) {
  // some lengthy logic, ignore content
  for (let i = 0; i < items.length; i++) {
    result += items[i]
  }
  for (let i = 0, token; (token = tokens[i]); i++) {
    const token = tokens[i]
    result += token
  }
  items.forEach((item) => {
    result += item
  })
  return result
} else {
  console.log(a) // what if does this belong to again?
}

vs

if (!a) {
   console.log(a) // easy to see what conditional this belongs to. else is now done & out of the way.
} else {
  // some lengthy logic, ignore content
  for (let i = 0; i < items.length; i++) {
    result += items[i]
  }
  for (let i = 0, token; (token = tokens[i]); i++) {
    const token = tokens[i]
    result += token
  }
  items.forEach((item) => {
    result += item
  })
  return result
}

[feross]

@timoxley Fair enough.

I'll close this.

[dcousens]

The principle I try to follow is to get as much logic "resolved" and out of the way (i.e. don't need to continue to keep mental track of it) in the fewest possible lines.

Amen, I wonder if we can capture this more often in standard.
It works incredibly well at preventing indentation blowout too when resolving it with early exits etc.

[timoxley]

It works incredibly well at preventing indentation blowout too when resolving it with early exits etc.

https://youtu.be/_wO8toxinoc?t=145

[reywood]

An alternative solution to the lengthy if vs lengthy else is:

if (a) {
  doSomethingWithA(a);
} else {
  console.log(a);
}

function doSomethingWithA(a) {
  // some lengthy logic, ignore content
  for (let i = 0; i < items.length; i++) {
    result += items[i]
  }
  for (let i = 0, token; (token = tokens[i]); i++) {
    const token = tokens[i]
    result += token
  }
  items.forEach((item) => {
    result += item
  })
  return result
}

Some find this preferable to inverting the conditional.