Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add rule no-constant-binary-expression #15296

Merged
merged 15 commits into from Apr 22, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 70 additions & 0 deletions docs/src/rules/no-constant-binary-expression.md
@@ -0,0 +1,70 @@
# no-constant-binary-expression

mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
Disallows expressions where the operation doesn't affect the value.

Comparisons which will always evaluate to true or false and logical expressions (`||`, `&&`, `??`) which either always short circuit or never short circuit are both likely indications of programmer error.
captbaritone marked this conversation as resolved.
Show resolved Hide resolved

These errors are especially common in complex expressions where operator precedence is easy to misjudge. For example:

```js
// One might think this would evaluate as `x + (b ?? c)`:
const x = a + b ?? c;

// But it actually evaluates as `(a + b) ?? c`. Since `a + b` can never be null,
// the `?? c` has no effect.
```

Additionally, we detect comparisons to newly constructed objects/arrays/functions/etc. In JavaScript, where objects are compared by reference, a newly constructed object can _never_ `===` any other value. This can be surprising for programmers coming from languages where objects are compared by value.
captbaritone marked this conversation as resolved.
Show resolved Hide resolved

```js
// Programmers coming from a language where objects are compared by value might expect this to work:
const isEmpty = x === [];

// However, this will always result in `isEmpty` being `false`.
```

## Rule Details

This rule identifies `==` and `===` comparisons which, based on the semantics of the JavaScript language, will always evaluate to `true` or `false`.

It also identifies `||`, `&&` and `??` logical expressions which will either always or never short circuit.
captbaritone marked this conversation as resolved.
Show resolved Hide resolved

Examples of **incorrect** code for this rule:

```js
/*eslint no-constant-binary-expression: "error"*/

const value1 = +x == null;

const value2 = condition ? x : {} || DEFAULT;

const value3 = !foo == null;

const value4 = new Boolean(foo) === true;

const objIsEmpty = someObj === {};

const arrIsEmpty = someArr === [];
```

Examples of **correct** code for this rule:

```js
/*eslint no-constant-binary-expression: "error"*/

const value1 = x == null;

const value2 = (condition ? x : {}) || DEFAULT;

const value3 = !(foo == null);

const value4 = Boolean(foo) === true;

const objIsEmpty = Object.keys(someObj).length === 0;

const arrIsEmpty = someArr.length === 0;
```

Related Rules:

* [no-constant-condition](no-constant-condition.md)
4 changes: 4 additions & 0 deletions docs/src/rules/no-constant-condition.md
Expand Up @@ -125,3 +125,7 @@ do {
}
} while (true)
```

## Related Rules

* [no-constant-binary-expression](no-constant-binary-expression.md)
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -103,6 +103,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
"no-confusing-arrow": () => require("./no-confusing-arrow"),
"no-console": () => require("./no-console"),
"no-const-assign": () => require("./no-const-assign"),
"no-constant-binary-expression": () => require("./no-constant-binary-expression"),
"no-constant-condition": () => require("./no-constant-condition"),
"no-constructor-return": () => require("./no-constructor-return"),
"no-continue": () => require("./no-continue"),
Expand Down