From aa7545b8926ab05f8f6c8b8a92f2cdc0d3f41894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Tue, 3 Sep 2024 23:30:49 +0900 Subject: [PATCH 01/13] feat: make new option --- .../docs/rules/prefer-nullish-coalescing.mdx | 28 ++++ .../src/rules/prefer-nullish-coalescing.ts | 48 +++++++ .../rules/prefer-nullish-coalescing.test.ts | 131 ++++++++++++++++++ 3 files changed, 207 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx index f36acad8e05f..7bd567e73c67 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx @@ -172,6 +172,34 @@ foo ?? 'a string'; Also, if you would like to ignore all primitives types, you can set `ignorePrimitives: true`. It is equivalent to `ignorePrimitives: { string: true, number: true, bigint: true, boolean: true }`. +### `ignorePrimitives` + +Whether to ignore any make boolean type value like `Boolan`, `!` . + +like !, Boolean, etc. You can use this when you do not want to apply the rule to cases that make the value a boolean. + +Incorrect code for `ignoreMakeBoolean: false`, and correct code for `ignoreMakeBoolean: true`: + +```ts option='{ "ignoreMakeBoolean": true }' showPlaygroundButton +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a || b); +const y = !!(a || b); +const z = Boolean(a || b); +``` + +Correct code for `ignoreMakeBoolean: false`: + +```ts option='{ "ignoreMakeBoolean": false }' showPlaygroundButton +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a ?? b); +const y = !!(a ?? b); +const z = Boolean(a ?? b); +``` + ### `allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing` Unless this is set to `true`, the rule will error on every file whose `tsconfig.json` does _not_ have the `strictNullChecks` compiler option (or `strict`) set to `true`. diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 05de88296a4f..555ede56f831 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -31,6 +31,7 @@ export type Options = [ } | true; ignoreTernaryTests?: boolean; + ignoreMakeBoolean?: boolean; }, ]; @@ -103,6 +104,11 @@ export default createRule({ 'Whether to ignore any ternary expressions that could be simplified by using the nullish coalescing operator.', type: 'boolean', }, + ignoreMakeBoolean: { + description: + 'Whether to ignore any make boolean type value like `Boolan`, `!`', + type: 'boolean', + }, }, additionalProperties: false, }, @@ -120,6 +126,7 @@ export default createRule({ number: false, string: false, }, + ignoreMakeBoolean: false, }, ], create( @@ -131,6 +138,7 @@ export default createRule({ ignoreMixedLogicalExpressions, ignorePrimitives, ignoreTernaryTests, + ignoreMakeBoolean, }, ], ) { @@ -324,6 +332,10 @@ export default createRule({ return; } + if (ignoreMakeBoolean === true && isMakeBoolean(node)) { + return; + } + const isMixedLogical = isMixedLogicalExpression(node); if (ignoreMixedLogicalExpressions === true && isMixedLogical) { return; @@ -436,6 +448,42 @@ function isConditionalTest(node: TSESTree.Node): boolean { return false; } +function isMakeBoolean(node: TSESTree.Node): boolean { + const parents = new Set([node]); + let current = node.parent; + while (current) { + parents.add(current); + + if ( + (current.type === AST_NODE_TYPES.UnaryExpression && + current.operator === '!') || + (current.type === AST_NODE_TYPES.CallExpression && + current.callee.type === 'Identifier' && + current.callee.name === 'Boolean') + ) { + return true; + } + + if ( + [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.FunctionExpression, + ].includes(current.type) + ) { + /** + * This is a weird situation like: + * `if (() => a || b) {}` + * `if (function () { return a || b }) {}` + */ + return false; + } + + current = current.parent; + } + + return false; +} + function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean { const seen = new Set(); const queue = [node.parent, node.left, node.right]; diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index c61384b6f24e..611c71053b26 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -355,6 +355,46 @@ x || y; }, ], }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; +let c: boolean | undefined; + +const x = Boolean(a || b); + `, + options: [ + { + ignoreMakeBoolean: true, + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !!(a || b); + `, + options: [ + { + ignoreMakeBoolean: true, + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a || b); + `, + options: [ + { + ignoreMakeBoolean: true, + }, + ], + }, ], invalid: [ ...nullishTypeTest((nullish, type) => ({ @@ -1823,5 +1863,96 @@ x ?? y; }, ], }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; +let c: boolean | undefined; + +const x = Boolean(a || b); + `, + options: [ + { + ignoreMakeBoolean: false, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; +let c: boolean | undefined; + +const x = Boolean(a ?? b); + `, + }, + ], + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; +let c: boolean | undefined; + +const x = !!(a || b); + `, + options: [ + { + ignoreMakeBoolean: false, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; +let c: boolean | undefined; + +const x = !!(a ?? b); + `, + }, + ], + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a || b); + `, + options: [ + { + ignoreMakeBoolean: false, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a ?? b); + `, + }, + ], + }, + ], + }, ], }); From 029f64ea661afee23d5b3934d10e0c0140644e77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Wed, 4 Sep 2024 22:31:55 +0900 Subject: [PATCH 02/13] fix: test error --- .../docs/rules/prefer-nullish-coalescing.mdx | 2 +- .../src/rules/prefer-nullish-coalescing.ts | 2 +- .../prefer-nullish-coalescing.shot | 24 +++++++++++++++++++ .../prefer-nullish-coalescing.shot | 6 +++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx index 7bd567e73c67..6d4c3c27b8df 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx @@ -172,7 +172,7 @@ foo ?? 'a string'; Also, if you would like to ignore all primitives types, you can set `ignorePrimitives: true`. It is equivalent to `ignorePrimitives: { string: true, number: true, bigint: true, boolean: true }`. -### `ignorePrimitives` +### `ignoreMakeBoolean` Whether to ignore any make boolean type value like `Boolan`, `!` . diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 555ede56f831..3a58532818c5 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -458,7 +458,7 @@ function isMakeBoolean(node: TSESTree.Node): boolean { (current.type === AST_NODE_TYPES.UnaryExpression && current.operator === '!') || (current.type === AST_NODE_TYPES.CallExpression && - current.callee.type === 'Identifier' && + current.callee.type === AST_NODE_TYPES.Identifier && current.callee.name === 'Boolean') ) { return true; diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot index d2c8afaf5096..7a7a3078da85 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot @@ -136,3 +136,27 @@ const foo: string | undefined = 'bar'; foo ?? 'a string'; " `; + +exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint output 9`] = ` +"Options: { "ignoreMakeBoolean": true } + +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a || b); +const y = !!(a || b); +const z = Boolean(a || b); +" +`; + +exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint output 10`] = ` +"Options: { "ignoreMakeBoolean": false } + +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = !(a ?? b); +const y = !!(a ?? b); +const z = Boolean(a ?? b); +" +`; diff --git a/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot b/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot index 7afce381e977..d37829bf791f 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot @@ -16,6 +16,10 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos "description": "Whether to ignore cases that are located within a conditional test.", "type": "boolean" }, + "ignoreMakeBoolean": { + "description": "Whether to ignore any make boolean type value like \`Boolan\`, \`!\`", + "type": "boolean" + }, "ignoreMixedLogicalExpressions": { "description": "Whether to ignore any logical or expressions that are part of a mixed logical expression (with \`&&\`).", "type": "boolean" @@ -64,6 +68,8 @@ type Options = [ allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing?: boolean; /** Whether to ignore cases that are located within a conditional test. */ ignoreConditionalTests?: boolean; + /** Whether to ignore any make boolean type value like \`Boolan\`, \`!\` */ + ignoreMakeBoolean?: boolean; /** Whether to ignore any logical or expressions that are part of a mixed logical expression (with \`&&\`). */ ignoreMixedLogicalExpressions?: boolean; /** Whether to ignore all (\`true\`) or some (an object with properties) primitive types. */ From ce4df828dd4c11a76271905201ea0111f2816a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Mon, 9 Sep 2024 21:45:55 +0900 Subject: [PATCH 03/13] fix: lint error --- packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 3a58532818c5..175d9145dc9f 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -459,6 +459,7 @@ function isMakeBoolean(node: TSESTree.Node): boolean { current.operator === '!') || (current.type === AST_NODE_TYPES.CallExpression && current.callee.type === AST_NODE_TYPES.Identifier && + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum current.callee.name === 'Boolean') ) { return true; From 9e11e2284fd6a6256d22f7750a6222a3289908e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Thu, 3 Oct 2024 23:53:05 +0900 Subject: [PATCH 04/13] fix: apply code review --- .../docs/rules/prefer-nullish-coalescing.mdx | 14 +- .../src/rules/prefer-nullish-coalescing.ts | 39 +++-- .../rules/prefer-nullish-coalescing.test.ts | 149 +++++++++++++++++- 3 files changed, 177 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx index 6d4c3c27b8df..c97f379054cf 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx @@ -172,15 +172,13 @@ foo ?? 'a string'; Also, if you would like to ignore all primitives types, you can set `ignorePrimitives: true`. It is equivalent to `ignorePrimitives: { string: true, number: true, bigint: true, boolean: true }`. -### `ignoreMakeBoolean` +### `ignoreBooleanCoercion` -Whether to ignore any make boolean type value like `Boolan`, `!` . +Whether to ignore expressions that coerce a value into a boolean: `Boolean(...)` and `!(...)`. -like !, Boolean, etc. You can use this when you do not want to apply the rule to cases that make the value a boolean. +Incorrect code for `ignoreBooleanCoercion: false`, and correct code for `ignoreBooleanCoercion: true`: -Incorrect code for `ignoreMakeBoolean: false`, and correct code for `ignoreMakeBoolean: true`: - -```ts option='{ "ignoreMakeBoolean": true }' showPlaygroundButton +```ts option='{ "ignoreBooleanCoercion": true }' showPlaygroundButton let a: string | true | undefined; let b: string | boolean | undefined; @@ -189,9 +187,9 @@ const y = !!(a || b); const z = Boolean(a || b); ``` -Correct code for `ignoreMakeBoolean: false`: +Correct code for `ignoreBooleanCoercion: false`: -```ts option='{ "ignoreMakeBoolean": false }' showPlaygroundButton +```ts option='{ "ignoreBooleanCoercion": false }' showPlaygroundButton let a: string | true | undefined; let b: string | boolean | undefined; diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 175d9145dc9f..83e205995ec4 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -31,7 +31,7 @@ export type Options = [ } | true; ignoreTernaryTests?: boolean; - ignoreMakeBoolean?: boolean; + ignoreBooleanCoercion?: boolean; }, ]; @@ -104,7 +104,7 @@ export default createRule({ 'Whether to ignore any ternary expressions that could be simplified by using the nullish coalescing operator.', type: 'boolean', }, - ignoreMakeBoolean: { + ignoreBooleanCoercion: { description: 'Whether to ignore any make boolean type value like `Boolan`, `!`', type: 'boolean', @@ -126,7 +126,7 @@ export default createRule({ number: false, string: false, }, - ignoreMakeBoolean: false, + ignoreBooleanCoercion: false, }, ], create( @@ -138,7 +138,7 @@ export default createRule({ ignoreMixedLogicalExpressions, ignorePrimitives, ignoreTernaryTests, - ignoreMakeBoolean, + ignoreBooleanCoercion, }, ], ) { @@ -332,7 +332,7 @@ export default createRule({ return; } - if (ignoreMakeBoolean === true && isMakeBoolean(node)) { + if (ignoreBooleanCoercion === true && isMakeBoolean(node, context)) { return; } @@ -448,7 +448,10 @@ function isConditionalTest(node: TSESTree.Node): boolean { return false; } -function isMakeBoolean(node: TSESTree.Node): boolean { +function isMakeBoolean( + node: TSESTree.Node, + context: Readonly>, +): boolean { const parents = new Set([node]); let current = node.parent; while (current) { @@ -458,9 +461,7 @@ function isMakeBoolean(node: TSESTree.Node): boolean { (current.type === AST_NODE_TYPES.UnaryExpression && current.operator === '!') || (current.type === AST_NODE_TYPES.CallExpression && - current.callee.type === AST_NODE_TYPES.Identifier && - // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum - current.callee.name === 'Boolean') + isBuiltInBoolanCall(current, context)) ) { return true; } @@ -473,8 +474,8 @@ function isMakeBoolean(node: TSESTree.Node): boolean { ) { /** * This is a weird situation like: - * `if (() => a || b) {}` - * `if (function () { return a || b }) {}` + * `Boolean(() => a || b)` + * `Boolean(function () { return a || b })` */ return false; } @@ -485,6 +486,22 @@ function isMakeBoolean(node: TSESTree.Node): boolean { return false; } +function isBuiltInBoolanCall( + node: TSESTree.CallExpression, + context: Readonly>, +): boolean { + if ( + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === 'Boolean' && + node.arguments[0] + ) { + const scope = context.sourceCode.getScope(node); + const variable = scope.set.get('Boolean'); + return !variable?.defs.length; + } + return false; +} + function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean { const seen = new Set(); const queue = [node.parent, node.left, node.right]; diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index 611c71053b26..b31e08f6afb4 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -365,7 +365,7 @@ const x = Boolean(a || b); `, options: [ { - ignoreMakeBoolean: true, + ignoreBooleanCoercion: true, }, ], }, @@ -378,7 +378,7 @@ const x = !!(a || b); `, options: [ { - ignoreMakeBoolean: true, + ignoreBooleanCoercion: true, }, ], }, @@ -391,7 +391,20 @@ const x = !(a || b); `, options: [ { - ignoreMakeBoolean: true, + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(1 + (a || b)); + `, + options: [ + { + ignoreBooleanCoercion: true, }, ], }, @@ -1873,7 +1886,7 @@ const x = Boolean(a || b); `, options: [ { - ignoreMakeBoolean: false, + ignoreBooleanCoercion: false, }, ], errors: [ @@ -1904,7 +1917,7 @@ const x = !!(a || b); `, options: [ { - ignoreMakeBoolean: false, + ignoreBooleanCoercion: false, }, ], errors: [ @@ -1934,7 +1947,7 @@ const x = !(a || b); `, options: [ { - ignoreMakeBoolean: false, + ignoreBooleanCoercion: false, }, ], errors: [ @@ -1954,5 +1967,129 @@ const x = !(a ?? b); }, ], }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +function Boolean(value){ + return !!value +} + +const x = Boolean(a || b); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +function Boolean(value){ + return !!value +} + +const x = Boolean(a ?? b); + `, + }, + ], + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = String(a || b); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = String(a ?? b); + `, + }, + ], + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(()=> a || b); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(()=> a ?? b); + `, + }, + ], + }, + ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(function weird(){ return a || b}); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(function weird(){ return a ?? b}); + `, + }, + ], + }, + ], + }, ], }); From 15ac9220315d56c3ff5073f771217892cf207c83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Thu, 10 Oct 2024 22:03:52 +0900 Subject: [PATCH 05/13] fix: typo correction --- .../src/rules/prefer-nullish-coalescing.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 83e205995ec4..b9639e61b1de 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -106,7 +106,7 @@ export default createRule({ }, ignoreBooleanCoercion: { description: - 'Whether to ignore any make boolean type value like `Boolan`, `!`', + 'Whether to ignore any make boolean type value like `Boolean`, `!`', type: 'boolean', }, }, @@ -461,7 +461,7 @@ function isMakeBoolean( (current.type === AST_NODE_TYPES.UnaryExpression && current.operator === '!') || (current.type === AST_NODE_TYPES.CallExpression && - isBuiltInBoolanCall(current, context)) + isBuiltInBooleanCall(current, context)) ) { return true; } @@ -486,17 +486,17 @@ function isMakeBoolean( return false; } -function isBuiltInBoolanCall( +function isBuiltInBooleanCall( node: TSESTree.CallExpression, context: Readonly>, ): boolean { if ( node.callee.type === AST_NODE_TYPES.Identifier && - node.callee.name === 'Boolean' && + node.callee.name === AST_TOKEN_TYPES.Boolean && node.arguments[0] ) { const scope = context.sourceCode.getScope(node); - const variable = scope.set.get('Boolean'); + const variable = scope.set.get(AST_TOKEN_TYPES.Boolean); return !variable?.defs.length; } return false; From 723d0a8ec655f89f0383ca8ed76158368b6d699b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Thu, 10 Oct 2024 22:39:16 +0900 Subject: [PATCH 06/13] fix: remove not operator logic --- .../docs/rules/prefer-nullish-coalescing.mdx | 10 +- .../src/rules/prefer-nullish-coalescing.ts | 8 +- .../rules/prefer-nullish-coalescing.test.ts | 137 +----------------- 3 files changed, 14 insertions(+), 141 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx index c97f379054cf..046ce5d97e88 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx @@ -174,7 +174,7 @@ Also, if you would like to ignore all primitives types, you can set `ignorePrimi ### `ignoreBooleanCoercion` -Whether to ignore expressions that coerce a value into a boolean: `Boolean(...)` and `!(...)`. +Whether to ignore expressions that coerce a value into a boolean: `Boolean(...)`. Incorrect code for `ignoreBooleanCoercion: false`, and correct code for `ignoreBooleanCoercion: true`: @@ -182,9 +182,7 @@ Incorrect code for `ignoreBooleanCoercion: false`, and correct code for `ignoreB let a: string | true | undefined; let b: string | boolean | undefined; -const x = !(a || b); -const y = !!(a || b); -const z = Boolean(a || b); +const x = Boolean(a || b); ``` Correct code for `ignoreBooleanCoercion: false`: @@ -193,9 +191,7 @@ Correct code for `ignoreBooleanCoercion: false`: let a: string | true | undefined; let b: string | boolean | undefined; -const x = !(a ?? b); -const y = !!(a ?? b); -const z = Boolean(a ?? b); +const x = Boolean(a ?? b); ``` ### `allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing` diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index b9639e61b1de..be9a2abe36bb 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -106,7 +106,7 @@ export default createRule({ }, ignoreBooleanCoercion: { description: - 'Whether to ignore any make boolean type value like `Boolean`, `!`', + 'Whether to ignore any make boolean type value like `Boolean`', type: 'boolean', }, }, @@ -458,10 +458,8 @@ function isMakeBoolean( parents.add(current); if ( - (current.type === AST_NODE_TYPES.UnaryExpression && - current.operator === '!') || - (current.type === AST_NODE_TYPES.CallExpression && - isBuiltInBooleanCall(current, context)) + current.type === AST_NODE_TYPES.CallExpression && + isBuiltInBooleanCall(current, context) ) { return true; } diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index b31e08f6afb4..1072075cc35d 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -374,34 +374,8 @@ const x = Boolean(a || b); let a: string | true | undefined; let b: string | boolean | undefined; -const x = !!(a || b); - `, - options: [ - { - ignoreBooleanCoercion: true, - }, - ], - }, - { - code: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - -const x = !(a || b); - `, - options: [ - { - ignoreBooleanCoercion: true, - }, - ], - }, - { - code: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - const x = Boolean(1 + (a || b)); - `, + `, options: [ { ignoreBooleanCoercion: true, @@ -1900,103 +1874,6 @@ let a: string | true | undefined; let b: string | boolean | undefined; let c: boolean | undefined; -const x = Boolean(a ?? b); - `, - }, - ], - }, - ], - }, - { - code: ` -let a: string | true | undefined; -let b: string | boolean | undefined; -let c: boolean | undefined; - -const x = !!(a || b); - `, - options: [ - { - ignoreBooleanCoercion: false, - }, - ], - errors: [ - { - messageId: 'preferNullishOverOr', - suggestions: [ - { - messageId: 'suggestNullish', - output: ` -let a: string | true | undefined; -let b: string | boolean | undefined; -let c: boolean | undefined; - -const x = !!(a ?? b); - `, - }, - ], - }, - ], - }, - { - code: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - -const x = !(a || b); - `, - options: [ - { - ignoreBooleanCoercion: false, - }, - ], - errors: [ - { - messageId: 'preferNullishOverOr', - suggestions: [ - { - messageId: 'suggestNullish', - output: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - -const x = !(a ?? b); - `, - }, - ], - }, - ], - }, - { - code: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - -function Boolean(value){ - return !!value -} - -const x = Boolean(a || b); - `, - options: [ - { - ignoreBooleanCoercion: true, - }, - ], - errors: [ - { - messageId: 'preferNullishOverOr', - suggestions: [ - { - messageId: 'suggestNullish', - output: ` -let a: string | true | undefined; -let b: string | boolean | undefined; - -function Boolean(value){ - return !!value -} - const x = Boolean(a ?? b); `, }, @@ -2010,7 +1887,7 @@ let a: string | true | undefined; let b: string | boolean | undefined; const x = String(a || b); - `, + `, options: [ { ignoreBooleanCoercion: true, @@ -2038,8 +1915,8 @@ const x = String(a ?? b); let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(()=> a || b); - `, +const x = Boolean(() => a || b); + `, options: [ { ignoreBooleanCoercion: true, @@ -2067,8 +1944,10 @@ const x = Boolean(()=> a ?? b); let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(function weird(){ return a || b}); - `, +const x = Boolean(function weird() { + return a || b; +}); + `, options: [ { ignoreBooleanCoercion: true, From d5f780f51c607310f0a7b10f3ec88106b2a655c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Tue, 15 Oct 2024 03:22:32 +0900 Subject: [PATCH 07/13] fix: apply code review --- .../src/rules/prefer-nullish-coalescing.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index be9a2abe36bb..1e3f41c46207 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -332,7 +332,7 @@ export default createRule({ return; } - if (ignoreBooleanCoercion === true && isMakeBoolean(node, context)) { + if (ignoreBooleanCoercion === true && isBooleanConstructorContext(node, context)) { return; } @@ -448,15 +448,12 @@ function isConditionalTest(node: TSESTree.Node): boolean { return false; } -function isMakeBoolean( +function isBooleanConstructorContext( node: TSESTree.Node, context: Readonly>, ): boolean { - const parents = new Set([node]); let current = node.parent; while (current) { - parents.add(current); - if ( current.type === AST_NODE_TYPES.CallExpression && isBuiltInBooleanCall(current, context) @@ -490,7 +487,8 @@ function isBuiltInBooleanCall( ): boolean { if ( node.callee.type === AST_NODE_TYPES.Identifier && - node.callee.name === AST_TOKEN_TYPES.Boolean && + // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum + node.callee.name === 'Boolean' && node.arguments[0] ) { const scope = context.sourceCode.getScope(node); From a46d2af5ed6a635c50c52e91cc5e9da48cd1c881 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Sun, 20 Oct 2024 20:59:33 +0900 Subject: [PATCH 08/13] change logical logic --- .../src/rules/prefer-nullish-coalescing.ts | 97 +++--- .../rules/prefer-nullish-coalescing.test.ts | 326 +++++++++++++++++- 2 files changed, 352 insertions(+), 71 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 1e3f41c46207..727048eb583a 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -411,81 +411,74 @@ export default createRule({ }, }); -function isConditionalTest(node: TSESTree.Node): boolean { - const parents = new Set([node]); - let current = node.parent; - while (current) { - parents.add(current); +function isConditionalTest(node: TSESTree.Node,parents=new Set([node])): boolean { + if(node.parent) {parents.add(node.parent);} - if ( - (current.type === AST_NODE_TYPES.ConditionalExpression || - current.type === AST_NODE_TYPES.DoWhileStatement || - current.type === AST_NODE_TYPES.IfStatement || - current.type === AST_NODE_TYPES.ForStatement || - current.type === AST_NODE_TYPES.WhileStatement) && - parents.has(current.test) - ) { - return true; - } + if(node.parent?.type === AST_NODE_TYPES.LogicalExpression ){ + return isConditionalTest(node.parent,parents) + } + + if ( + node.parent?.type === AST_NODE_TYPES.ConditionalExpression && + (node.parent.consequent === node || node.parent.alternate === node) + ) { + return isConditionalTest(node.parent,parents); + } + + if ( + node.parent?.type === AST_NODE_TYPES.SequenceExpression && + node.parent.expressions.at(-1) === node + ) { + return isConditionalTest(node.parent,parents); + } if ( - [ - AST_NODE_TYPES.ArrowFunctionExpression, - AST_NODE_TYPES.FunctionExpression, - ].includes(current.type) + (node.parent?.type === AST_NODE_TYPES.ConditionalExpression || + node.parent?.type === AST_NODE_TYPES.DoWhileStatement || + node.parent?.type === AST_NODE_TYPES.IfStatement || + node.parent?.type === AST_NODE_TYPES.ForStatement || + node.parent?.type === AST_NODE_TYPES.WhileStatement) && + parents.has(node.parent.test) ) { - /** - * This is a weird situation like: - * `if (() => a || b) {}` - * `if (function () { return a || b }) {}` - */ - return false; + return true; } - current = current.parent; - } + return false - return false; } function isBooleanConstructorContext( node: TSESTree.Node, context: Readonly>, ): boolean { - let current = node.parent; - while (current) { - if ( - current.type === AST_NODE_TYPES.CallExpression && - isBuiltInBooleanCall(current, context) - ) { - return true; - } + if(node.parent?.type === AST_NODE_TYPES.LogicalExpression ){ + return isBooleanConstructorContext(node.parent,context) + } - if ( - [ - AST_NODE_TYPES.ArrowFunctionExpression, - AST_NODE_TYPES.FunctionExpression, - ].includes(current.type) - ) { - /** - * This is a weird situation like: - * `Boolean(() => a || b)` - * `Boolean(function () { return a || b })` - */ - return false; - } + if ( + node.parent?.type === AST_NODE_TYPES.ConditionalExpression && + (node.parent.consequent === node || node.parent.alternate === node) + ) { + return isBooleanConstructorContext(node.parent,context); + } - current = current.parent; + if ( + node.parent?.type === AST_NODE_TYPES.SequenceExpression && + node.parent.expressions.at(-1) === node + ) { + return isBooleanConstructorContext(node.parent,context); } - return false; + return isBuiltInBooleanCall(node.parent, context); } function isBuiltInBooleanCall( - node: TSESTree.CallExpression, + node: TSESTree.Node | undefined, context: Readonly>, ): boolean { if ( + !!node && + node.type === AST_NODE_TYPES.CallExpression && node.callee.type === AST_NODE_TYPES.Identifier && // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum node.callee.name === 'Boolean' && diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index 1072075cc35d..b8fffbb23d44 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -359,7 +359,6 @@ x || y; code: ` let a: string | true | undefined; let b: string | boolean | undefined; -let c: boolean | undefined; const x = Boolean(a || b); `, @@ -371,10 +370,95 @@ const x = Boolean(a || b); }, { code: ` -let a: string | true | undefined; +let a: string | boolean | undefined; let b: string | boolean | undefined; +let c: string | boolean | undefined; -const x = Boolean(1 + (a || b)); +const test = Boolean(a || b || c); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean(a || (b && c)); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean((a || b) ?? c); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean(a ?? (b || c)); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean(a ? b || c : 'fail'); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean(a ? 'success' : b || c); + `, + options: [ + { + ignoreBooleanCoercion: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +const test = Boolean(((a = b), b || c)); `, options: [ { @@ -382,6 +466,111 @@ const x = Boolean(1 + (a || b)); }, ], }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (a || b || c) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (a || (b && c)) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if ((a || b) ?? c) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (a ?? (b || c)) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (a ? b || c : 'fail') { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (a ? 'success' : b || c) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, + { + code: ` +let a: string | boolean | undefined; +let b: string | boolean | undefined; +let c: string | boolean | undefined; + +if (((a = b), b || c)) { +} + `, + options: [ + { + ignoreConditionalTests: true, + }, + ], + }, ], invalid: [ ...nullishTypeTest((nullish, type) => ({ @@ -1829,7 +2018,6 @@ enum Enum { declare const x: Enum.A | Enum.B | undefined; x || y; `, - output: null, errors: [ { messageId: 'preferNullishOverOr', @@ -1849,6 +2037,7 @@ x ?? y; ], }, ], + output: null, }, { code: ` @@ -1858,11 +2047,6 @@ let c: boolean | undefined; const x = Boolean(a || b); `, - options: [ - { - ignoreBooleanCoercion: false, - }, - ], errors: [ { messageId: 'preferNullishOverOr', @@ -1880,6 +2064,11 @@ const x = Boolean(a ?? b); ], }, ], + options: [ + { + ignoreBooleanCoercion: false, + }, + ], }, { code: ` @@ -1888,11 +2077,35 @@ let b: string | boolean | undefined; const x = String(a || b); `, + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = String(a ?? b); + `, + }, + ], + }, + ], options: [ { ignoreBooleanCoercion: true, }, ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(() => a || b); + `, errors: [ { messageId: 'preferNullishOverOr', @@ -1903,25 +2116,60 @@ const x = String(a || b); let a: string | true | undefined; let b: string | boolean | undefined; -const x = String(a ?? b); - `, +const x = Boolean(() => a ?? b); + `, }, ], }, ], + options: [ + { + ignoreBooleanCoercion: true, + }, + ], }, { code: ` let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(() => a || b); +const x = Boolean(function weird() { + return a || b; +}); + `, + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(function weird() { + return a ?? b; +}); `, + }, + ], + }, + ], options: [ { ignoreBooleanCoercion: true, }, ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +declare function f(x: unknown): unknown; + +const x = Boolean(f(a || b)); + `, errors: [ { messageId: 'preferNullishOverOr', @@ -1932,27 +2180,59 @@ const x = Boolean(() => a || b); let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(()=> a ?? b); - `, +declare function f(x: unknown): unknown; + +const x = Boolean(f(a ?? b)); + `, }, ], }, ], + options: [ + { + ignoreBooleanCoercion: true, + }, + ], }, { code: ` let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(function weird() { - return a || b; -}); +const x = Boolean(1 + (a || b)); `, + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +const x = Boolean(1 + (a ?? b)); + `, + }, + ], + }, + ], options: [ { ignoreBooleanCoercion: true, }, ], + }, + { + code: ` +let a: string | true | undefined; +let b: string | boolean | undefined; + +declare function f(x: unknown): unknown; + +if (f(a || b)) { +} + `, errors: [ { messageId: 'preferNullishOverOr', @@ -1963,12 +2243,20 @@ const x = Boolean(function weird() { let a: string | true | undefined; let b: string | boolean | undefined; -const x = Boolean(function weird(){ return a ?? b}); - `, +declare function f(x: unknown): unknown; + +if (f(a ?? b)) { +} + `, }, ], }, ], + options: [ + { + ignoreBooleanCoercion: true, + }, + ], }, ], }); From dfcc469a8ec4d979e7e2c5131acb55419fc807fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Tue, 22 Oct 2024 23:46:29 +0900 Subject: [PATCH 09/13] refactor: fix isBooleanConstructorContext and isConditionalTest fn --- .../src/rules/prefer-nullish-coalescing.ts | 79 +++++++++++-------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 727048eb583a..716605af6cfd 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -332,7 +332,10 @@ export default createRule({ return; } - if (ignoreBooleanCoercion === true && isBooleanConstructorContext(node, context)) { + if ( + ignoreBooleanCoercion === true && + isBooleanConstructorContext(node, context) + ) { return; } @@ -411,73 +414,79 @@ export default createRule({ }, }); -function isConditionalTest(node: TSESTree.Node,parents=new Set([node])): boolean { - if(node.parent) {parents.add(node.parent);} +function isConditionalTest(node: TSESTree.Node): boolean { + const parent = node.parent; + if (parent == null) { + return false; + } - if(node.parent?.type === AST_NODE_TYPES.LogicalExpression ){ - return isConditionalTest(node.parent,parents) + if (parent.type === AST_NODE_TYPES.LogicalExpression) { + return isConditionalTest(parent); } if ( - node.parent?.type === AST_NODE_TYPES.ConditionalExpression && - (node.parent.consequent === node || node.parent.alternate === node) + parent.type === AST_NODE_TYPES.ConditionalExpression && + (parent.consequent === node || parent.alternate === node) ) { - return isConditionalTest(node.parent,parents); + return isConditionalTest(parent); } if ( - node.parent?.type === AST_NODE_TYPES.SequenceExpression && - node.parent.expressions.at(-1) === node + parent.type === AST_NODE_TYPES.SequenceExpression && + parent.expressions.at(-1) === node ) { - return isConditionalTest(node.parent,parents); + return isConditionalTest(parent); } - if ( - (node.parent?.type === AST_NODE_TYPES.ConditionalExpression || - node.parent?.type === AST_NODE_TYPES.DoWhileStatement || - node.parent?.type === AST_NODE_TYPES.IfStatement || - node.parent?.type === AST_NODE_TYPES.ForStatement || - node.parent?.type === AST_NODE_TYPES.WhileStatement) && - parents.has(node.parent.test) - ) { - return true; - } - - return false + if ( + (parent.type === AST_NODE_TYPES.ConditionalExpression || + parent.type === AST_NODE_TYPES.DoWhileStatement || + parent.type === AST_NODE_TYPES.IfStatement || + parent.type === AST_NODE_TYPES.ForStatement || + parent.type === AST_NODE_TYPES.WhileStatement) && + parent.test === node + ) { + return true; + } + return false; } function isBooleanConstructorContext( node: TSESTree.Node, context: Readonly>, ): boolean { - if(node.parent?.type === AST_NODE_TYPES.LogicalExpression ){ - return isBooleanConstructorContext(node.parent,context) + const parent = node.parent; + if (parent == null) { + return false; + } + + if (parent.type === AST_NODE_TYPES.LogicalExpression) { + return isBooleanConstructorContext(parent, context); } if ( - node.parent?.type === AST_NODE_TYPES.ConditionalExpression && - (node.parent.consequent === node || node.parent.alternate === node) + parent.type === AST_NODE_TYPES.ConditionalExpression && + (parent.consequent === node || parent.alternate === node) ) { - return isBooleanConstructorContext(node.parent,context); + return isBooleanConstructorContext(parent, context); } if ( - node.parent?.type === AST_NODE_TYPES.SequenceExpression && - node.parent.expressions.at(-1) === node + parent.type === AST_NODE_TYPES.SequenceExpression && + parent.expressions.at(-1) === node ) { - return isBooleanConstructorContext(node.parent,context); + return isBooleanConstructorContext(parent, context); } - return isBuiltInBooleanCall(node.parent, context); + return isBuiltInBooleanCall(parent, context); } function isBuiltInBooleanCall( - node: TSESTree.Node | undefined, + node: TSESTree.Node, context: Readonly>, ): boolean { if ( - !!node && node.type === AST_NODE_TYPES.CallExpression && node.callee.type === AST_NODE_TYPES.Identifier && // eslint-disable-next-line @typescript-eslint/internal/prefer-ast-types-enum @@ -486,7 +495,7 @@ function isBuiltInBooleanCall( ) { const scope = context.sourceCode.getScope(node); const variable = scope.set.get(AST_TOKEN_TYPES.Boolean); - return !variable?.defs.length; + return variable == null || variable.defs.length === 0; } return false; } From 9d8c4e4f96cb539894eee3d58544b901e6c06cd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Sun, 27 Oct 2024 15:58:41 +0900 Subject: [PATCH 10/13] fix:docs{ --- packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 716605af6cfd..623f0781c426 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -106,7 +106,7 @@ export default createRule({ }, ignoreBooleanCoercion: { description: - 'Whether to ignore any make boolean type value like `Boolean`', + 'Whether to ignore arguments to the `Boolean` constructor', type: 'boolean', }, }, From f59d7cceacb53c08780b91c7dfe66f8acccd9d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Sat, 2 Nov 2024 11:44:17 +0900 Subject: [PATCH 11/13] fix: merge confict miss add --- .../eslint-plugin/src/rules/prefer-nullish-coalescing.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 33e4b1503f29..dd1b80f108b0 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -439,6 +439,13 @@ export default createRule({ 'LogicalExpression[operator = "||"]'( node: TSESTree.LogicalExpression, ): void { + if ( + ignoreBooleanCoercion === true && + isBooleanConstructorContext(node, context) + ) { + return; + } + checkAssignmentOrLogicalExpression(node, 'or', ''); }, }; From f738414dbf4b1dca69d56a19620f86489f417f33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Sat, 2 Nov 2024 12:00:57 +0900 Subject: [PATCH 12/13] fix: lint error --- .../src/rules/prefer-nullish-coalescing.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index dd1b80f108b0..9f5a6661cca6 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -21,6 +21,7 @@ import { export type Options = [ { allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing?: boolean; + ignoreBooleanCoercion?: boolean; ignoreConditionalTests?: boolean; ignoreMixedLogicalExpressions?: boolean; ignorePrimitives?: @@ -32,7 +33,6 @@ export type Options = [ } | true; ignoreTernaryTests?: boolean; - ignoreBooleanCoercion?: boolean; }, ]; @@ -72,6 +72,11 @@ export default createRule({ description: 'Unless this is set to `true`, the rule will error on every file whose `tsconfig.json` does _not_ have the `strictNullChecks` compiler option (or `strict`) set to `true`.', }, + ignoreBooleanCoercion: { + type: 'boolean', + description: + 'Whether to ignore arguments to the `Boolean` constructor', + }, ignoreConditionalTests: { type: 'boolean', description: @@ -120,11 +125,6 @@ export default createRule({ description: 'Whether to ignore any ternary expressions that could be simplified by using the nullish coalescing operator.', }, - ignoreBooleanCoercion: { - description: - 'Whether to ignore arguments to the `Boolean` constructor', - type: 'boolean', - }, }, }, ], @@ -132,6 +132,7 @@ export default createRule({ defaultOptions: [ { allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: false, + ignoreBooleanCoercion: false, ignoreConditionalTests: true, ignoreMixedLogicalExpressions: false, ignorePrimitives: { @@ -140,7 +141,6 @@ export default createRule({ number: false, string: false, }, - ignoreBooleanCoercion: false, ignoreTernaryTests: false, }, ], @@ -149,11 +149,11 @@ export default createRule({ [ { allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing, + ignoreBooleanCoercion, ignoreConditionalTests, ignoreMixedLogicalExpressions, ignorePrimitives, ignoreTernaryTests, - ignoreBooleanCoercion, }, ], ) { From 52c3596fe8cab42fb2853b52f64ddac9172f7c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=80=E1=85=B5=E1=86=B7=E1=84=89=E1=85=A1=E1=86=BC?= =?UTF-8?q?=E1=84=83=E1=85=AE?= Date: Sat, 2 Nov 2024 13:39:09 +0900 Subject: [PATCH 13/13] fix: snapshot update --- .../prefer-nullish-coalescing.shot | 12 ++++-------- .../schema-snapshots/prefer-nullish-coalescing.shot | 12 ++++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot index 7a7a3078da85..dd5126a91def 100644 --- a/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot +++ b/packages/eslint-plugin/tests/docs-eslint-output-snapshots/prefer-nullish-coalescing.shot @@ -138,25 +138,21 @@ foo ?? 'a string'; `; exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint output 9`] = ` -"Options: { "ignoreMakeBoolean": true } +"Options: { "ignoreBooleanCoercion": true } let a: string | true | undefined; let b: string | boolean | undefined; -const x = !(a || b); -const y = !!(a || b); -const z = Boolean(a || b); +const x = Boolean(a || b); " `; exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint output 10`] = ` -"Options: { "ignoreMakeBoolean": false } +"Options: { "ignoreBooleanCoercion": false } let a: string | true | undefined; let b: string | boolean | undefined; -const x = !(a ?? b); -const y = !!(a ?? b); -const z = Boolean(a ?? b); +const x = Boolean(a ?? b); " `; diff --git a/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot b/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot index 9ea391bfa827..ea441036f3a0 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/prefer-nullish-coalescing.shot @@ -12,12 +12,12 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos "description": "Unless this is set to \`true\`, the rule will error on every file whose \`tsconfig.json\` does _not_ have the \`strictNullChecks\` compiler option (or \`strict\`) set to \`true\`.", "type": "boolean" }, - "ignoreConditionalTests": { - "description": "Whether to ignore cases that are located within a conditional test.", + "ignoreBooleanCoercion": { + "description": "Whether to ignore arguments to the \`Boolean\` constructor", "type": "boolean" }, - "ignoreMakeBoolean": { - "description": "Whether to ignore any make boolean type value like \`Boolan\`, \`!\`", + "ignoreConditionalTests": { + "description": "Whether to ignore cases that are located within a conditional test.", "type": "boolean" }, "ignoreMixedLogicalExpressions": { @@ -72,10 +72,10 @@ type Options = [ { /** Unless this is set to \`true\`, the rule will error on every file whose \`tsconfig.json\` does _not_ have the \`strictNullChecks\` compiler option (or \`strict\`) set to \`true\`. */ allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing?: boolean; + /** Whether to ignore arguments to the \`Boolean\` constructor */ + ignoreBooleanCoercion?: boolean; /** Whether to ignore cases that are located within a conditional test. */ ignoreConditionalTests?: boolean; - /** Whether to ignore any make boolean type value like \`Boolan\`, \`!\` */ - ignoreMakeBoolean?: boolean; /** Whether to ignore any logical or expressions that are part of a mixed logical expression (with \`&&\`). */ ignoreMixedLogicalExpressions?: boolean; /** Whether to ignore all (\`true\`) or some (an object with properties) primitive types. */