From c87cfaf6746775bb8ad9eb45b0002f068a822dbe Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Mon, 1 Jun 2020 14:51:05 +0900 Subject: [PATCH] fix(eslint-plugin): [no-unnecessary-condition] improve optional chain handling 2 - electric boogaloo (#2138) --- .../src/rules/no-unnecessary-condition.ts | 55 ++++- .../rules/no-unnecessary-condition.test.ts | 228 ++++++++++++++++++ 2 files changed, 274 insertions(+), 9 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 5f04366b5fd..0834c5a3b57 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -21,6 +21,7 @@ import { nullThrows, NullThrowsReasons, isMemberOrOptionalMemberExpression, + isIdentifier, } from '../util'; // Truthiness utilities @@ -426,6 +427,46 @@ export default createRule({ return false; } + // Checks whether a member expression is nullable or not regardless of it's previous node. + // Example: + // ``` + // // 'bar' is nullable if 'foo' is null. + // // but this function checks regardless of 'foo' type, so returns 'true'. + // declare const foo: { bar : { baz: string } } | null + // foo?.bar; + // ``` + function isNullableOriginFromPrev( + node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, + ): boolean { + const prevType = getNodeType(node.object); + const property = node.property; + if (prevType.isUnion() && isIdentifier(property)) { + const isOwnNullable = prevType.types.some(type => { + const propType = checker.getTypeOfPropertyOfType(type, property.name); + return propType && isNullableType(propType, { allowUndefined: true }); + }); + + return ( + !isOwnNullable && isNullableType(prevType, { allowUndefined: true }) + ); + } + return false; + } + + function isOptionableExpression( + node: TSESTree.LeftHandSideExpression, + ): boolean { + const type = getNodeType(node); + const isOwnNullable = isMemberOrOptionalMemberExpression(node) + ? !isNullableOriginFromPrev(node) + : true; + return ( + isTypeFlagSet(type, ts.TypeFlags.Any) || + isTypeFlagSet(type, ts.TypeFlags.Unknown) || + (isNullableType(type, { allowUndefined: true }) && isOwnNullable) + ); + } + function checkOptionalChain( node: TSESTree.OptionalMemberExpression | TSESTree.OptionalCallExpression, beforeOperator: TSESTree.Node, @@ -444,16 +485,12 @@ export default createRule({ return; } - const nodeToCheck = isMemberOrOptionalMemberExpression(node) - ? node.object - : node; - const type = getNodeType(nodeToCheck); + const nodeToCheck = + node.type === AST_NODE_TYPES.OptionalCallExpression + ? node.callee + : node.object; - if ( - isTypeFlagSet(type, ts.TypeFlags.Any) || - isTypeFlagSet(type, ts.TypeFlags.Unknown) || - isNullableType(type, { allowUndefined: true }) - ) { + if (isOptionableExpression(nodeToCheck)) { return; } 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 1efb193df42..a823d1ab6f1 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -329,6 +329,23 @@ let unknownValue: unknown; unknownValue?.(); `, 'const foo = [1, 2, 3][0];', + ` +declare const foo: { bar?: { baz: { c: string } } } | null; +foo?.bar?.baz; + `, + ` +foo?.bar?.baz?.qux; + `, + ` +declare const foo: { bar: { baz: string } }; +foo.bar.qux?.(); + `, + ` +type Foo = { baz: number } | null; +type Bar = { baz: null | string | { qux: string } }; +declare const foo: { fooOrBar: Foo | Bar } | null; +foo?.fooOrBar?.baz?.qux; + `, ], invalid: [ // Ensure that it's checking in all the right places @@ -836,5 +853,216 @@ x.a; }, ], }, + { + code: ` +declare const foo: { bar: { baz: { c: string } } } | null; +foo?.bar?.baz; + `, + output: ` +declare const foo: { bar: { baz: { c: string } } } | null; +foo?.bar.baz; + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 9, + endColumn: 11, + }, + ], + }, + { + code: ` +declare const foo: { bar?: { baz: { qux: string } } } | null; +foo?.bar?.baz?.qux; + `, + output: ` +declare const foo: { bar?: { baz: { qux: string } } } | null; +foo?.bar?.baz.qux; + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 14, + endColumn: 16, + }, + ], + }, + { + code: ` +declare const foo: { bar: { baz: { qux?: () => {} } } } | null; +foo?.bar?.baz?.qux?.(); + `, + output: ` +declare const foo: { bar: { baz: { qux?: () => {} } } } | null; +foo?.bar.baz.qux?.(); + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 9, + endColumn: 11, + }, + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 14, + endColumn: 16, + }, + ], + }, + { + code: ` +declare const foo: { bar: { baz: { qux: () => {} } } } | null; +foo?.bar?.baz?.qux?.(); + `, + output: ` +declare const foo: { bar: { baz: { qux: () => {} } } } | null; +foo?.bar.baz.qux(); + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 9, + endColumn: 11, + }, + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 14, + endColumn: 16, + }, + { + messageId: 'neverOptionalChain', + line: 3, + endLine: 3, + column: 19, + endColumn: 21, + }, + ], + }, + { + code: ` +type baz = () => { qux: () => {} }; +declare const foo: { bar: { baz: baz } } | null; +foo?.bar?.baz?.().qux?.(); + `, + output: ` +type baz = () => { qux: () => {} }; +declare const foo: { bar: { baz: baz } } | null; +foo?.bar.baz().qux(); + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 9, + endColumn: 11, + }, + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 14, + endColumn: 16, + }, + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 22, + endColumn: 24, + }, + ], + }, + { + code: ` +type baz = null | (() => { qux: () => {} }); +declare const foo: { bar: { baz: baz } } | null; +foo?.bar?.baz?.().qux?.(); + `, + output: ` +type baz = null | (() => { qux: () => {} }); +declare const foo: { bar: { baz: baz } } | null; +foo?.bar.baz?.().qux(); + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 9, + endColumn: 11, + }, + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 22, + endColumn: 24, + }, + ], + }, + { + code: ` +type baz = null | (() => { qux: () => {} } | null); +declare const foo: { bar: { baz: baz } } | null; +foo?.bar?.baz?.()?.qux?.(); + `, + output: ` +type baz = null | (() => { qux: () => {} } | null); +declare const foo: { bar: { baz: baz } } | null; +foo?.bar.baz?.()?.qux(); + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 9, + endColumn: 11, + }, + { + messageId: 'neverOptionalChain', + line: 4, + endLine: 4, + column: 23, + endColumn: 25, + }, + ], + }, + { + code: ` +type Foo = { baz: number }; +type Bar = { baz: null | string | { qux: string } }; +declare const foo: { fooOrBar: Foo | Bar } | null; +foo?.fooOrBar?.baz?.qux; + `, + output: ` +type Foo = { baz: number }; +type Bar = { baz: null | string | { qux: string } }; +declare const foo: { fooOrBar: Foo | Bar } | null; +foo?.fooOrBar.baz?.qux; + `, + errors: [ + { + messageId: 'neverOptionalChain', + line: 5, + endLine: 5, + column: 14, + endColumn: 16, + }, + ], + }, ], });