Skip to content

Commit

Permalink
feat: Add rule no-constant-binary-expression (#15296)
Browse files Browse the repository at this point in the history
* feat: Add rule no-constant-binary-expression

I proposed the core idea of this rule in
#13752 as an addition to
`no-constant-condition`, but on the advice of the TSC, it was
restructured as a standalone rule.

* Share `no-constant-condition`'s `isConstant`

Here we extract `isConstant` into ast-utils and share it between
`no-constant-condition` and `no-constant-binary-expression`.

* Update title of  docs page

* Avoid false positive on shadowed builtins

* Use isLogicalAssignmentOperator

* Ensure we don't treat user defined new expressions as always new

* Move docs to the new docs directory

* Address review feedback

* Address more review feedback

* Address review feedback

* Fix typo

* Update lib/rules/utils/ast-utils.js

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* Update docs/src/rules/no-constant-binary-expression.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* Update docs/src/rules/no-constant-binary-expression.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

* Update docs/src/rules/no-constant-binary-expression.md

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
  • Loading branch information
captbaritone and nzakas committed Apr 22, 2022
1 parent db28f2c commit ab6363d
Show file tree
Hide file tree
Showing 8 changed files with 1,093 additions and 198 deletions.
70 changes: 70 additions & 0 deletions docs/src/rules/no-constant-binary-expression.md
@@ -0,0 +1,70 @@
# no-constant-binary-expression

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.

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, this rule detects 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.
```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.
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 @@ -132,3 +132,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

0 comments on commit ab6363d

Please sign in to comment.