diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md b/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md index d600bb7930c0..e2cda8643d00 100644 --- a/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md @@ -5,7 +5,7 @@ Comparing boolean values to boolean literals is unnecessary, those comparisons r ## Rule Details This rule ensures that you do not include unnecessary comparisons with boolean literals. -A comparison is considered unnecessary if it checks a boolean literal against any variable with just the `boolean` type. +A comparison is considered unnecessary if it checks a boolean literal against any expression with just the `boolean` type. A comparison is **_not_** considered unnecessary if the type is a union of booleans (`string | boolean`, `someObject | boolean`). **Note**: Throughout this page, only strict equality (`===` and `!==`) are @@ -44,23 +44,31 @@ The rule accepts an options object with the following properties. ```ts type Options = { - // if false, comparisons between a nullable boolean variable to `true` will be checked and fixed + // if false, comparisons between a nullable boolean expression to `true` will be checked and fixed allowComparingNullableBooleansToTrue?: boolean; - // if false, comparisons between a nullable boolean variable to `false` will be checked and fixed + // if false, comparisons between a nullable boolean expression to `false` will be checked and fixed allowComparingNullableBooleansToFalse?: boolean; + // if defined, comparisons between a nullable boolean expression to boolean literals are replaced + // with a preceeding null/undefined check + fixWithExplicitNullishCheck?: { + // whether the check should be against null or undefined. + // since loose equality (==, !=) is used for this check, this is just a style choice. + nullishSymbol: 'null' | 'undefined'; + }; }; ``` ### Defaults -This rule always checks comparisons between a boolean variable and a boolean -literal. Comparisons between nullable boolean variables and boolean literals +This rule always checks comparisons between a boolean expression and a boolean +literal. Comparisons between nullable boolean expressions and boolean literals are **not** checked by default. ```ts const defaults = { allowComparingNullableBooleansToTrue: true, allowComparingNullableBooleansToFalse: true, + fixWithExplicitNullishCheck: undefined, }; ``` @@ -110,25 +118,49 @@ Examples of **correct** code for this rule with `{ allowComparingNullableBoolean declare const someUndefinedCondition: boolean | undefined; if (someUndefinedCondition ?? true) { } +if (someUndefinedCondition != null && !someUndefinedCondition) { +} declare const someNullCondition: boolean | null; if (!(someNullCondition ?? true)) { } +if (someNullCondition == undefined || someNullCondition) { +} ``` ## Fixer +### Boolean Expressions + +The follow table shows the output of the fixer on comparisons that compare a boolean expression to a boolean literal. + +| Comparison | Fixer Output | +| :--------------------: | ----------------- | +| `booleanVar === true` | `booleanLiteral` | +| `booleanVar !== true` | `!booleanLiteral` | +| `booleanVar === false` | `!booleanLiteral` | +| `booleanVar !== false` | `booleanLiteral` | + +### Nullable Boolean Expressions + +The follow table shows the output of the fixer on comparisons that compare a boolean expression to a boolean literal when the `fixWithExplicitNullishCheck` option is `undefined`: + | Comparison | Fixer Output | Notes | | :----------------------------: | ------------------------------- | ----------------------------------------------------------------------------------- | -| `booleanVar === true` | `booleanLiteral` | | -| `booleanVar !== true` | `!booleanLiteral` | | -| `booleanVar === false` | `!booleanLiteral` | | -| `booleanVar !== false` | `booleanLiteral` | | | `nullableBooleanVar === true` | `nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToTrue` option is `false` | | `nullableBooleanVar !== true` | `!nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToTrue` option is `false` | | `nullableBooleanVar === false` | `nullableBooleanVar ?? true` | Only checked/fixed if the `allowComparingNullableBooleansToFalse` option is `false` | | `nullableBooleanVar !== false` | `!(nullableBooleanVar ?? true)` | Only checked/fixed if the `allowComparingNullableBooleansToFalse` option is `false` | +The follow table shows the output of the fixer on comparisons that compare a boolean expression to a boolean literal when the `fixWithExplicitNullishCheck` option is `{ nullishSymbol: "null" }`: + +| Comparison | Fixer Output | Notes | +| :----------------------------: | ----------------------------------------------------- | ----------------------------------------------------------------------------------- | +| `nullableBooleanVar === true` | `nullableBooleanVar != null && nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToTrue` option is `false` | +| `nullableBooleanVar !== true` | `nullableBooleanVar == null \|\| !nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToTrue` option is `false` | +| `nullableBooleanVar === false` | `nullableBooleanVar != null && !nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToFalse` option is `false` | +| `nullableBooleanVar !== false` | `nullableBooleanVar == null \|\| nullableBooleanVar` | Only checked/fixed if the `allowComparingNullableBooleansToFalse` option is `false` | + ## Related to - TSLint: [no-boolean-literal-compare](https://palantir.github.io/tslint/rules/no-boolean-literal-compare) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts index fe1fd24cc4f1..3e3c37c57b06 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts @@ -17,6 +17,9 @@ type Options = [ { allowComparingNullableBooleansToTrue?: boolean; allowComparingNullableBooleansToFalse?: boolean; + fixWithExplicitNullishCheck?: { + nullishSymbol: 'null' | 'undefined'; + }; }, ]; @@ -65,6 +68,14 @@ export default util.createRule({ allowComparingNullableBooleansToFalse: { type: 'boolean', }, + fixWithExplicitNullishCheck: { + type: 'object', + properties: { + nullishSymbol: { + type: 'string', + }, + }, + }, }, additionalProperties: false, }, @@ -201,6 +212,36 @@ export default util.createRule({ ); } + function getExplicitNullishCheck( + { literalBooleanInComparison, negated }: BooleanComparison, + expressionBeingCompared: string, + nullishSymbol: 'null' | 'undefined', + ): string { + if (literalBooleanInComparison) { + // replace `nullableBooleanVar === true` with + // nullableBooleanVar != null && nullableBooleanVar + if (!negated) { + return `${expressionBeingCompared} != ${nullishSymbol} && `; + } + + // replace `nullableBooleanVar !== true` with + // nullableBooleanVar == null || !nullableBooleanVar + if (negated) { + return `${expressionBeingCompared} == ${nullishSymbol} || !`; + } + } + + // replace `nullableBooleanVar === false` with + // nullableBooleanVar != null && !nullableBooleanVar + if (!negated) { + return `${expressionBeingCompared} != ${nullishSymbol} && !`; + } + + // replace `nullableBooleanVar !== false` with + // nullableBooleanVar == null || nullableBooleanVar + return `${expressionBeingCompared} == ${nullishSymbol} || `; + } + return { BinaryExpression(node): void { const comparison = getBooleanComparison(node); @@ -225,9 +266,41 @@ export default util.createRule({ context.report({ fix: function* (fixer) { + if ( + comparison.expressionIsNullableBoolean && + options.fixWithExplicitNullishCheck !== undefined + ) { + // if fixWithExplicitNullishCheck is true, then we're replacing + // something like `booleanOrFalse === true` with + // `booleanOrFalse != null && booleanOrFalse`. We only apply these + // fixes for identifiers, for more complicated expressions, we + // could change the program behavior (e.g. if the expression is a + // function call, we would be replacing one function call with two) + if (comparison.expression.type === AST_NODE_TYPES.Identifier) { + // remove the comparison and the boolean literal + yield fixer.removeRange(comparison.range); + + // add the null check + yield fixer.insertTextBefore( + node, + '(' + // with an opening parenthesis + getExplicitNullishCheck( + comparison, + context.getSourceCode().getText(comparison.expression), + options.fixWithExplicitNullishCheck.nullishSymbol, + ), + ); + + // add the closing parenthesis + yield fixer.insertTextAfter(node, ')'); + } + return; + } + + // remove the comparison and the boolean literal yield fixer.removeRange(comparison.range); - // if the expression `exp` isn't nullable, or we're comparing to `true`, + // if the expression `exp` isn't nullable, // we can just replace the entire comparison with `exp` or `!exp` if ( !comparison.expressionIsNullableBoolean || @@ -239,10 +312,13 @@ export default util.createRule({ return; } - // if we're here, then the expression is a nullable boolean and we're - // comparing to a literal `false` + // if we're here, then: + // 1) the expression is a nullable boolean + // 2) the "fixWithExplicitNullishCheck" option is undefined + // 3) comparing to a literal `false` - // if we're doing `== false` or `=== false`, then we need to negate the expression + // if the comparison is not negated (i.e. we're doing `== false` or `=== false`), + // then we need to negate the expression if (!comparison.negated) { const { parent } = node; // if the parent is a negation, we can instead just get rid of the parent's negation. diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts index 36eb36033de1..1d591070f709 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts @@ -224,5 +224,195 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, { } `, }, + { + code: ` + function getBooleanOrNull(): false | null { + Math.random() > 0.5 ? false : null; + } + if (getBooleanOrNull() === true) { + } + `, + options: [ + { + allowComparingNullableBooleansToTrue: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'null', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToTrueDirect', + }, + ], + // can't fix because fixing would call the function twice: + // `getBooleanOrNull() != null && getBooleanOrNull()` + output: ` + function getBooleanOrNull(): false | null { + Math.random() > 0.5 ? false : null; + } + if (getBooleanOrNull() === true) { + } + `, + }, + { + code: ` + declare const varBoolOrNull: boolean | null; + if (varBoolOrNull === true) { + } + `, + options: [ + { + allowComparingNullableBooleansToTrue: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'undefined', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToTrueDirect', + }, + ], + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` + declare const varBoolOrNull: boolean | null; + if ((varBoolOrNull != undefined && varBoolOrNull)) { + } + `, + }, + { + code: ` + declare const varBoolOrNull: boolean | null; + if (varBoolOrNull !== true) { + } + `, + options: [ + { + allowComparingNullableBooleansToTrue: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'undefined', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToTrueNegated', + }, + ], + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` + declare const varBoolOrNull: boolean | null; + if ((varBoolOrNull == undefined || !varBoolOrNull)) { + } + `, + }, + { + code: ` + declare const varBoolOrNull: boolean | null; + if (varBoolOrNull === false) { + } + `, + options: [ + { + allowComparingNullableBooleansToFalse: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'undefined', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToFalse', + }, + ], + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` + declare const varBoolOrNull: boolean | null; + if ((varBoolOrNull != undefined && !varBoolOrNull)) { + } + `, + }, + { + code: ` + declare const varBoolOrNull: boolean | null; + if (varBoolOrNull !== false) { + } + `, + options: [ + { + allowComparingNullableBooleansToFalse: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'undefined', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToFalse', + }, + ], + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` + declare const varBoolOrNull: boolean | null; + if ((varBoolOrNull == undefined || varBoolOrNull)) { + } + `, + }, + { + code: ` + declare const varBooleanOrNull: boolean | null; + declare const otherBoolean: boolean; + if (varBooleanOrNull === false && otherBoolean) { + } + `, + options: [ + { + allowComparingNullableBooleansToFalse: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'null', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToFalse', + }, + ], + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` + declare const varBooleanOrNull: boolean | null; + declare const otherBoolean: boolean; + if ((varBooleanOrNull != null && !varBooleanOrNull) && otherBoolean) { + } + `, + }, + { + code: ` + declare const varBoolOrUndefined: true | false | undefined; + declare const otherBoolean: boolean; + if (varBoolOrUndefined !== false && !otherBoolean) { + } + `, + options: [ + { + allowComparingNullableBooleansToFalse: false, + fixWithExplicitNullishCheck: { + nullishSymbol: 'null', + }, + }, + ], + errors: [ + { + messageId: 'comparingNullableToFalse', + }, + ], + output: ` + declare const varBoolOrUndefined: true | false | undefined; + declare const otherBoolean: boolean; + if ((varBoolOrUndefined == null || varBoolOrUndefined) && !otherBoolean) { + } + `, + }, ], });