From 1e04761cc76c328a87cb7f68e130fe455bbbe925 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:06:52 -0700 Subject: [PATCH 1/7] treat any/unknown as eligible for nullish coalescing --- .../src/rules/prefer-nullish-coalescing.ts | 27 ++- .../rules/prefer-nullish-coalescing.test.ts | 203 ++++++++++++------ 2 files changed, 159 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 e52f83543af7..fbf9ceb3d0fe 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -13,7 +13,7 @@ import { isNodeEqual, isNodeOfTypes, isNullLiteral, - isPossiblyNullish, + isNullableType, isUndefinedIdentifier, nullThrows, NullThrowsReasons, @@ -190,7 +190,7 @@ export default createRule({ * a nullishness check, taking into account the rule's configuration. */ function isTypeEligibleForPreferNullish(type: ts.Type): boolean { - if (!isPossiblyNullish(type)) { + if (!isNullableType(type)) { return false; } @@ -208,14 +208,23 @@ export default createRule({ ] .filter((flag): flag is number => typeof flag === 'number') .reduce((previous, flag) => previous | flag, 0); + if ( - type.flags !== ts.TypeFlags.Null && - type.flags !== ts.TypeFlags.Undefined && - (type as ts.UnionOrIntersectionType).types.some(t => - tsutils - .intersectionTypeParts(t) - .some(t => tsutils.isTypeFlagSet(t, ignorableFlags)), - ) + // if the type is any, undefined, or void, we can't make any assumptions + // about the value, so it could be any primitive, even though the flags + // won't be set. + (tsutils.isTypeFlagSet( + type, + ts.TypeFlags.Any | ts.TypeFlags.Undefined | ts.TypeFlags.Void, + ) && + ignorableFlags !== 0) || + tsutils + .typeParts(type) + .some(t => + tsutils + .intersectionTypeParts(t) + .some(t => tsutils.isTypeFlagSet(t, ignorableFlags)), + ) ) { return false; } 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 99cca5c7369c..644e6c734b35 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -199,22 +199,6 @@ x ? x : y; `, ` declare let x: boolean; -!x ? y : x; - `, - ` -declare let x: any; -x ? x : y; - `, - ` -declare let x: any; -!x ? y : x; - `, - ` -declare let x: unknown; -x ? x : y; - `, - ` -declare let x: unknown; !x ? y : x; `, ` @@ -311,22 +295,6 @@ x.n ? x.n : y; `, ` declare let x: { n: boolean }; -!x.n ? y : x.n; - `, - ` -declare let x: { n: any }; -x.n ? x.n : y; - `, - ` -declare let x: { n: any }; -!x.n ? y : x.n; - `, - ` -declare let x: { n: unknown }; -x.n ? x.n : y; - `, - ` -declare let x: { n: unknown }; !x.n ? y : x.n; `, ` @@ -536,41 +504,11 @@ declare let x: (${type} & { __brand?: any }) | undefined; `, options: [{ ignorePrimitives: true }], })), - ` - declare let x: any; - declare let y: number; - x || y; - `, - ` - declare let x: unknown; - declare let y: number; - x || y; - `, ` declare let x: never; declare let y: number; x || y; `, - ` - declare let x: any; - declare let y: number; - x ? x : y; - `, - ` - declare let x: any; - declare let y: number; - !x ? y : x; - `, - ` - declare let x: unknown; - declare let y: number; - x ? x : y; - `, - ` - declare let x: unknown; - declare let y: number; - !x ? y : x; - `, ` declare let x: never; declare let y: number; @@ -1333,6 +1271,34 @@ if (c || (!a ? b : a)) { }, ], }, + + { + code: ` +declare const a: any; +declare const b: any; +a ? a : b; + `, + options: [ + { + ignorePrimitives: true, + }, + ], + }, + + { + code: ` +declare const a: any; +declare const b: any; +a ? a : b; + `, + options: [ + { + ignorePrimitives: { + number: true, + }, + }, + ], + }, ], invalid: [ ...nullishTypeTest((nullish, type, equals) => ({ @@ -4698,5 +4664,118 @@ defaultBox ?? getFallbackBox(); options: [{ ignoreTernaryTests: false }], output: null, }, + + { + code: ` +declare const x: any; +declare const y: any; +x || y; + `, + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare const x: any; +declare const y: any; +x ?? y; + `, + }, + ], + }, + ], + }, + + { + code: ` +declare const x: unknown; +declare const y: any; +x || y; + `, + errors: [ + { + messageId: 'preferNullishOverOr', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare const x: unknown; +declare const y: any; +x ?? y; + `, + }, + ], + }, + ], + }, + + { + code: ` +declare let x: unknown; +declare let y: number; +!x ? y : x; + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare let x: unknown; +declare let y: number; +x ?? y; + `, + }, + ], + }, + ], + }, + + { + code: ` +declare let x: unknown; +declare let y: number; +x ? x : y; + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare let x: unknown; +declare let y: number; +x ?? y; + `, + }, + ], + }, + ], + }, + + { + code: ` +declare let x: { n: unknown }; +!x.n ? y : x.n; + `, + errors: [ + { + messageId: 'preferNullishOverTernary', + suggestions: [ + { + messageId: 'suggestNullish', + output: ` +declare let x: { n: unknown }; +x.n ?? y; + `, + }, + ], + }, + ], + }, ], }); From b1fee0c2061c6bec3d6612012a38644d3068e55d Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Fri, 21 Feb 2025 16:38:37 -0700 Subject: [PATCH 2/7] more stuff --- .../docs/rules/prefer-nullish-coalescing.mdx | 8 +++---- .../src/rules/no-unnecessary-condition.ts | 21 +++++++++++++++++-- packages/eslint-plugin/src/util/index.ts | 2 +- ...sAndNullishUtils.ts => truthinessUtils.ts} | 11 ---------- .../prefer-nullish-coalescing.shot | 11 ++++++---- 5 files changed, 31 insertions(+), 22 deletions(-) rename packages/eslint-plugin/src/util/{truthinessAndNullishUtils.ts => truthinessUtils.ts} (74%) diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx index 87cf88527046..438cc97d6af6 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.mdx @@ -84,7 +84,7 @@ Examples of code for this rule with `{ ignoreConditionalTests: false }`: ```ts option='{ "ignoreConditionalTests": false }' -declare const a: string | null; +declare let a: string | null; declare const b: string | null; if (a || b) { @@ -102,7 +102,7 @@ a || b ? true : false; ```ts option='{ "ignoreConditionalTests": false }' -declare const a: string | null; +declare let a: string | null; declare const b: string | null; if (a ?? b) { @@ -133,7 +133,7 @@ Examples of code for this rule with `{ ignoreMixedLogicalExpressions: false }`: ```ts option='{ "ignoreMixedLogicalExpressions": false }' -declare const a: string | null; +declare let a: string | null; declare const b: string | null; declare const c: string | null; declare const d: string | null; @@ -149,7 +149,7 @@ a || (b && c && d); ```ts option='{ "ignoreMixedLogicalExpressions": false }' -declare const a: string | null; +declare let a: string | null; declare const b: string | null; declare const c: string | null; declare const d: string | null; diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index cb93dfae3ba3..d9c11f53d868 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -12,12 +12,10 @@ import { getTypeName, getTypeOfPropertyOfName, getValueOfLiteralType, - isAlwaysNullish, isArrayMethodCallWithPredicate, isIdentifier, isNullableType, isPossiblyFalsy, - isPossiblyNullish, isPossiblyTruthy, isTypeAnyType, isTypeFlagSet, @@ -31,6 +29,25 @@ import { } from '../util/assertionFunctionUtils'; // #region + +const nullishFlag = ts.TypeFlags.Undefined | ts.TypeFlags.Null; + +function isNullishType(type: ts.Type): boolean { + return tsutils.isTypeFlagSet(type, nullishFlag); +} + +function isAlwaysNullish(type: ts.Type): boolean { + return tsutils.unionTypeParts(type).every(isNullishType); +} + +/** + * Note that this differs from {@link isNullableType} in that it doesn't + * `any` or `unknown` to be nullable. + */ +function isPossiblyNullish(type: ts.Type): boolean { + return tsutils.unionTypeParts(type).some(isNullishType); +} + function toStaticValue( type: ts.Type, ): diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index c8a0927b162b..d8202417fa8d 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -25,7 +25,7 @@ export * from './scopeUtils'; export * from './types'; export * from './getConstraintInfo'; export * from './getValueOfLiteralType'; -export * from './truthinessAndNullishUtils'; +export * from './truthinessUtils'; // this is done for convenience - saves migrating all of the old rules export * from '@typescript-eslint/type-utils'; diff --git a/packages/eslint-plugin/src/util/truthinessAndNullishUtils.ts b/packages/eslint-plugin/src/util/truthinessUtils.ts similarity index 74% rename from packages/eslint-plugin/src/util/truthinessAndNullishUtils.ts rename to packages/eslint-plugin/src/util/truthinessUtils.ts index b35e0334719d..048a7a60500e 100644 --- a/packages/eslint-plugin/src/util/truthinessAndNullishUtils.ts +++ b/packages/eslint-plugin/src/util/truthinessUtils.ts @@ -28,14 +28,3 @@ export const isPossiblyTruthy = (type: ts.Type): boolean => // like `"" & { __brand: string }`. intersectionParts.every(type => !tsutils.isFalsyType(type)), ); - -// Nullish utilities -const nullishFlag = ts.TypeFlags.Undefined | ts.TypeFlags.Null; -const isNullishType = (type: ts.Type): boolean => - tsutils.isTypeFlagSet(type, nullishFlag); - -export const isPossiblyNullish = (type: ts.Type): boolean => - tsutils.unionTypeParts(type).some(isNullishType); - -export const isAlwaysNullish = (type: ts.Type): boolean => - tsutils.unionTypeParts(type).every(isNullishType); 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 124ed091013c..babba8f3ad55 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 @@ -55,17 +55,19 @@ exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint "Incorrect Options: { "ignoreConditionalTests": false } -declare const a: string | null; +declare let a: string | null; declare const b: string | null; if (a || b) { ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. } if ((a ||= b)) { + ~~~ Prefer using nullish coalescing operator (\`??=\`) instead of a logical assignment (\`||=\`), as it is a safer operator. } while (a || b) {} ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. while ((a ||= b)) {} + ~~~ Prefer using nullish coalescing operator (\`??=\`) instead of a logical assignment (\`||=\`), as it is a safer operator. do {} while (a || b); ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. for (let i = 0; a || b; i += 1) {} @@ -79,7 +81,7 @@ exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint "Correct Options: { "ignoreConditionalTests": false } -declare const a: string | null; +declare let a: string | null; declare const b: string | null; if (a ?? b) { @@ -98,7 +100,7 @@ exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint "Incorrect Options: { "ignoreMixedLogicalExpressions": false } -declare const a: string | null; +declare let a: string | null; declare const b: string | null; declare const c: string | null; declare const d: string | null; @@ -106,6 +108,7 @@ declare const d: string | null; a || (b && c); ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. a ||= b && c; + ~~~ Prefer using nullish coalescing operator (\`??=\`) instead of a logical assignment (\`||=\`), as it is a safer operator. (a && b) || c || d; ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. ~~ Prefer using nullish coalescing operator (\`??\`) instead of a logical or (\`||\`), as it is a safer operator. @@ -121,7 +124,7 @@ exports[`Validating rule docs prefer-nullish-coalescing.mdx code examples ESLint "Correct Options: { "ignoreMixedLogicalExpressions": false } -declare const a: string | null; +declare let a: string | null; declare const b: string | null; declare const c: string | null; declare const d: string | null; From b1aa570e44e1d3f82a8e5572b9fe1189b09c1431 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 3 Mar 2025 00:53:59 -0700 Subject: [PATCH 3/7] fix typo Co-authored-by: Ronen Amiel --- packages/eslint-plugin/src/rules/no-unnecessary-condition.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index d9c11f53d868..19e734179127 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -41,7 +41,7 @@ function isAlwaysNullish(type: ts.Type): boolean { } /** - * Note that this differs from {@link isNullableType} in that it doesn't + * Note that this differs from {@link isNullableType} in that it doesn't consider * `any` or `unknown` to be nullable. */ function isPossiblyNullish(type: ts.Type): boolean { From 9a39d66f73e89397021a8319c4bfcbaa98543ae3 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 3 Mar 2025 01:19:49 -0700 Subject: [PATCH 4/7] feedback --- .../src/rules/prefer-nullish-coalescing.ts | 15 ++++++----- .../rules/prefer-nullish-coalescing.test.ts | 26 ++++++++++++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index fbf9ceb3d0fe..3b61e0a58f9a 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -210,14 +210,17 @@ export default createRule({ .reduce((previous, flag) => previous | flag, 0); if ( - // if the type is any, undefined, or void, we can't make any assumptions + // if the type is `any` or `unknown` we can't make any assumptions // about the value, so it could be any primitive, even though the flags // won't be set. - (tsutils.isTypeFlagSet( - type, - ts.TypeFlags.Any | ts.TypeFlags.Undefined | ts.TypeFlags.Void, - ) && - ignorableFlags !== 0) || + // + // technically, this is true of `void` as well, however, it's a TS error + // to test `void` for truthiness, so we won't bother checking for it. + (ignorableFlags !== 0 && + tsutils.isTypeFlagSet( + type, + ts.TypeFlags.Any | ts.TypeFlags.Unknown, + )) || tsutils .typeParts(type) .some(t => 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 644e6c734b35..1e1d88797435 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -1299,7 +1299,25 @@ a ? a : b; }, ], }, + + { + code: ` +declare const u2: unknown; +const foo2 = u2 || 'bar'; + `, + options: [ + { + ignorePrimitives: { + bigint: true, + boolean: false, + number: false, + string: false, + }, + }, + ], + }, ], + invalid: [ ...nullishTypeTest((nullish, type, equals) => ({ code: ` @@ -4716,7 +4734,7 @@ x ?? y; declare let x: unknown; declare let y: number; !x ? y : x; - `, + `, errors: [ { messageId: 'preferNullishOverTernary', @@ -4727,7 +4745,7 @@ declare let y: number; declare let x: unknown; declare let y: number; x ?? y; - `, + `, }, ], }, @@ -4739,7 +4757,7 @@ x ?? y; declare let x: unknown; declare let y: number; x ? x : y; - `, + `, errors: [ { messageId: 'preferNullishOverTernary', @@ -4750,7 +4768,7 @@ x ? x : y; declare let x: unknown; declare let y: number; x ?? y; - `, + `, }, ], }, From c356e31d80d419067d7c321de0f104e1d310feeb Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 3 Mar 2025 01:34:34 -0700 Subject: [PATCH 5/7] tweaks --- .../src/rules/prefer-nullish-coalescing.ts | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index 3b61e0a58f9a..3400c44ca5dc 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -209,18 +209,25 @@ export default createRule({ .filter((flag): flag is number => typeof flag === 'number') .reduce((previous, flag) => previous | flag, 0); + if (ignorableFlags === 0) { + // any types are eligible for conversion. + return true; + } + + // if the type is `any` or `unknown` we can't make any assumptions + // about the value, so it could be any primitive, even though the flags + // won't be set. + // + // technically, this is true of `void` as well, however, it's a TS error + // to test `void` for truthiness, so we don't need to bother checking for + // it in valid code. + if ( + tsutils.isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown) + ) { + return false; + } + if ( - // if the type is `any` or `unknown` we can't make any assumptions - // about the value, so it could be any primitive, even though the flags - // won't be set. - // - // technically, this is true of `void` as well, however, it's a TS error - // to test `void` for truthiness, so we won't bother checking for it. - (ignorableFlags !== 0 && - tsutils.isTypeFlagSet( - type, - ts.TypeFlags.Any | ts.TypeFlags.Unknown, - )) || tsutils .typeParts(type) .some(t => From 62600ca4b992353bb6d1a796f414876138e4d1b4 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Mon, 3 Mar 2025 17:49:53 -0700 Subject: [PATCH 6/7] merge mistake --- .../eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts | 2 ++ 1 file changed, 2 insertions(+) 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 a4e18e976018..cbce3baa3199 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -5057,6 +5057,8 @@ defaultBoxOptional.a?.b ?? getFallbackBox(); ], }, ], + options: [{ ignoreTernaryTests: false }], + output: null, }, { code: ` From f82db50201d1cd1ead4a666f3820ffaaefd386d4 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger <53019676+kirkwaiblinger@users.noreply.github.com> Date: Wed, 5 Mar 2025 17:50:25 -0700 Subject: [PATCH 7/7] rename vars in test Co-authored-by: Ronen Amiel --- .../tests/rules/prefer-nullish-coalescing.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 cbce3baa3199..a57db7e341be 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -1342,8 +1342,8 @@ a ? a : b; { code: ` -declare const u2: unknown; -const foo2 = u2 || 'bar'; +declare const a: unknown; +const b = a || 'bar'; `, options: [ {