From f68b09fdeca79895214fd6b758f83fcc7db5f92d Mon Sep 17 00:00:00 2001 From: Cparros Date: Sun, 5 Feb 2023 22:07:56 -0700 Subject: [PATCH 1/3] Update error message per #5255 discussion --- packages/eslint-plugin/src/rules/no-unnecessary-condition.ts | 3 ++- 1 file changed, 2 insertions(+), 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 42f12748af90..d50ab45e9611 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -126,7 +126,8 @@ export default createRule({ noOverlapBooleanExpression: 'Unnecessary conditional, the types have no overlap.', never: 'Unnecessary conditional, value is `never`.', - neverOptionalChain: 'Unnecessary optional chain on a non-nullish value.', + neverOptionalChain: + 'Unnecessary optional chain on a non-nullish value. Please ensure you are not declaring a variable with the `void` type outside of a return type or generic type argument. Use `undefined` instead.', noStrictNullCheck: 'This rule requires the `strictNullChecks` compiler option to be turned on to function correctly.', }, From 25ccd33dfcf328145542f1e73cfec3040b418dec Mon Sep 17 00:00:00 2001 From: Cparros Date: Mon, 6 Feb 2023 17:57:27 -0700 Subject: [PATCH 2/3] Add void check to no-unnecessary-condition and test case --- .../src/rules/no-unnecessary-condition.ts | 10 +++++++--- .../tests/rules/no-unnecessary-condition.test.ts | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index d50ab45e9611..f79fa98d22fd 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -126,8 +126,7 @@ export default createRule({ noOverlapBooleanExpression: 'Unnecessary conditional, the types have no overlap.', never: 'Unnecessary conditional, value is `never`.', - neverOptionalChain: - 'Unnecessary optional chain on a non-nullish value. Please ensure you are not declaring a variable with the `void` type outside of a return type or generic type argument. Use `undefined` instead.', + neverOptionalChain: 'Unnecessary optional chain on a non-nullish value.', noStrictNullCheck: 'This rule requires the `strictNullChecks` compiler option to be turned on to function correctly.', }, @@ -575,10 +574,15 @@ export default createRule({ node.type === AST_NODE_TYPES.MemberExpression ? !isNullableOriginFromPrev(node) : true; + const unionParts = unionTypeParts(type); + const possiblyVoid = unionParts.some( + part => part.flags & ts.TypeFlags.Void, + ); return ( isTypeAnyType(type) || isTypeUnknownType(type) || - (isNullableType(type, { allowUndefined: true }) && isOwnNullable) + (isNullableType(type, { allowUndefined: true }) && isOwnNullable) || + (possiblyVoid && isOwnNullable) ); } diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index af8b189f8994..7f7f89216536 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -545,6 +545,10 @@ type OptionalFoo = Foo | undefined; declare const foo: OptionalFoo; foo?.[1]?.length; `, + ` +let variable = 'abc' as string | void; +variable?.[0]; + `, ], invalid: [ // Ensure that it's checking in all the right places From c63f6944e5519a90639839e85f22c78ac62fbf67 Mon Sep 17 00:00:00 2001 From: Cparros Date: Tue, 7 Feb 2023 22:22:19 -0700 Subject: [PATCH 3/3] Cleanup, update type checks --- .../src/rules/no-unnecessary-condition.ts | 12 ++++-------- .../tests/rules/no-unnecessary-condition.test.ts | 8 ++++---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index f79fa98d22fd..596dcf798467 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -574,15 +574,11 @@ export default createRule({ node.type === AST_NODE_TYPES.MemberExpression ? !isNullableOriginFromPrev(node) : true; - const unionParts = unionTypeParts(type); - const possiblyVoid = unionParts.some( - part => part.flags & ts.TypeFlags.Void, - ); + const possiblyVoid = isTypeFlagSet(type, ts.TypeFlags.Void); return ( - isTypeAnyType(type) || - isTypeUnknownType(type) || - (isNullableType(type, { allowUndefined: true }) && isOwnNullable) || - (possiblyVoid && isOwnNullable) + isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown) || + (isOwnNullable && + (isNullableType(type, { allowUndefined: true }) || possiblyVoid)) ); } diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 7f7f89216536..6a5f803605f0 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -346,6 +346,10 @@ do {} while (true); options: [{ allowConstantLoopConditions: true }], }, ` +let variable = 'abc' as string | void; +variable?.[0]; + `, + ` let foo: undefined | { bar: true }; foo?.bar; `, @@ -545,10 +549,6 @@ type OptionalFoo = Foo | undefined; declare const foo: OptionalFoo; foo?.[1]?.length; `, - ` -let variable = 'abc' as string | void; -variable?.[0]; - `, ], invalid: [ // Ensure that it's checking in all the right places