Skip to content

Commit c87cfaf

Browse files
yeonjuanyeonjuan
authored
fix(eslint-plugin): [no-unnecessary-condition] improve optional chain handling 2 - electric boogaloo (#2138)
1 parent 1cb8ca4 commit c87cfaf

File tree

2 files changed

+274
-9
lines changed

2 files changed

+274
-9
lines changed

Diff for: ‎packages/eslint-plugin/src/rules/no-unnecessary-condition.ts

+46-9
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
nullThrows,
2222
NullThrowsReasons,
2323
isMemberOrOptionalMemberExpression,
24+
isIdentifier,
2425
} from '../util';
2526

2627
// Truthiness utilities
@@ -426,6 +427,46 @@ export default createRule<Options, MessageId>({
426427
return false;
427428
}
428429

430+
// Checks whether a member expression is nullable or not regardless of it's previous node.
431+
// Example:
432+
// ```
433+
// // 'bar' is nullable if 'foo' is null.
434+
// // but this function checks regardless of 'foo' type, so returns 'true'.
435+
// declare const foo: { bar : { baz: string } } | null
436+
// foo?.bar;
437+
// ```
438+
function isNullableOriginFromPrev(
439+
node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression,
440+
): boolean {
441+
const prevType = getNodeType(node.object);
442+
const property = node.property;
443+
if (prevType.isUnion() && isIdentifier(property)) {
444+
const isOwnNullable = prevType.types.some(type => {
445+
const propType = checker.getTypeOfPropertyOfType(type, property.name);
446+
return propType && isNullableType(propType, { allowUndefined: true });
447+
});
448+
449+
return (
450+
!isOwnNullable && isNullableType(prevType, { allowUndefined: true })
451+
);
452+
}
453+
return false;
454+
}
455+
456+
function isOptionableExpression(
457+
node: TSESTree.LeftHandSideExpression,
458+
): boolean {
459+
const type = getNodeType(node);
460+
const isOwnNullable = isMemberOrOptionalMemberExpression(node)
461+
? !isNullableOriginFromPrev(node)
462+
: true;
463+
return (
464+
isTypeFlagSet(type, ts.TypeFlags.Any) ||
465+
isTypeFlagSet(type, ts.TypeFlags.Unknown) ||
466+
(isNullableType(type, { allowUndefined: true }) && isOwnNullable)
467+
);
468+
}
469+
429470
function checkOptionalChain(
430471
node: TSESTree.OptionalMemberExpression | TSESTree.OptionalCallExpression,
431472
beforeOperator: TSESTree.Node,
@@ -444,16 +485,12 @@ export default createRule<Options, MessageId>({
444485
return;
445486
}
446487

447-
const nodeToCheck = isMemberOrOptionalMemberExpression(node)
448-
? node.object
449-
: node;
450-
const type = getNodeType(nodeToCheck);
488+
const nodeToCheck =
489+
node.type === AST_NODE_TYPES.OptionalCallExpression
490+
? node.callee
491+
: node.object;
451492

452-
if (
453-
isTypeFlagSet(type, ts.TypeFlags.Any) ||
454-
isTypeFlagSet(type, ts.TypeFlags.Unknown) ||
455-
isNullableType(type, { allowUndefined: true })
456-
) {
493+
if (isOptionableExpression(nodeToCheck)) {
457494
return;
458495
}
459496

Diff for: ‎packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts

+228
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,23 @@ let unknownValue: unknown;
329329
unknownValue?.();
330330
`,
331331
'const foo = [1, 2, 3][0];',
332+
`
333+
declare const foo: { bar?: { baz: { c: string } } } | null;
334+
foo?.bar?.baz;
335+
`,
336+
`
337+
foo?.bar?.baz?.qux;
338+
`,
339+
`
340+
declare const foo: { bar: { baz: string } };
341+
foo.bar.qux?.();
342+
`,
343+
`
344+
type Foo = { baz: number } | null;
345+
type Bar = { baz: null | string | { qux: string } };
346+
declare const foo: { fooOrBar: Foo | Bar } | null;
347+
foo?.fooOrBar?.baz?.qux;
348+
`,
332349
],
333350
invalid: [
334351
// Ensure that it's checking in all the right places
@@ -836,5 +853,216 @@ x.a;
836853
},
837854
],
838855
},
856+
{
857+
code: `
858+
declare const foo: { bar: { baz: { c: string } } } | null;
859+
foo?.bar?.baz;
860+
`,
861+
output: `
862+
declare const foo: { bar: { baz: { c: string } } } | null;
863+
foo?.bar.baz;
864+
`,
865+
errors: [
866+
{
867+
messageId: 'neverOptionalChain',
868+
line: 3,
869+
endLine: 3,
870+
column: 9,
871+
endColumn: 11,
872+
},
873+
],
874+
},
875+
{
876+
code: `
877+
declare const foo: { bar?: { baz: { qux: string } } } | null;
878+
foo?.bar?.baz?.qux;
879+
`,
880+
output: `
881+
declare const foo: { bar?: { baz: { qux: string } } } | null;
882+
foo?.bar?.baz.qux;
883+
`,
884+
errors: [
885+
{
886+
messageId: 'neverOptionalChain',
887+
line: 3,
888+
endLine: 3,
889+
column: 14,
890+
endColumn: 16,
891+
},
892+
],
893+
},
894+
{
895+
code: `
896+
declare const foo: { bar: { baz: { qux?: () => {} } } } | null;
897+
foo?.bar?.baz?.qux?.();
898+
`,
899+
output: `
900+
declare const foo: { bar: { baz: { qux?: () => {} } } } | null;
901+
foo?.bar.baz.qux?.();
902+
`,
903+
errors: [
904+
{
905+
messageId: 'neverOptionalChain',
906+
line: 3,
907+
endLine: 3,
908+
column: 9,
909+
endColumn: 11,
910+
},
911+
{
912+
messageId: 'neverOptionalChain',
913+
line: 3,
914+
endLine: 3,
915+
column: 14,
916+
endColumn: 16,
917+
},
918+
],
919+
},
920+
{
921+
code: `
922+
declare const foo: { bar: { baz: { qux: () => {} } } } | null;
923+
foo?.bar?.baz?.qux?.();
924+
`,
925+
output: `
926+
declare const foo: { bar: { baz: { qux: () => {} } } } | null;
927+
foo?.bar.baz.qux();
928+
`,
929+
errors: [
930+
{
931+
messageId: 'neverOptionalChain',
932+
line: 3,
933+
endLine: 3,
934+
column: 9,
935+
endColumn: 11,
936+
},
937+
{
938+
messageId: 'neverOptionalChain',
939+
line: 3,
940+
endLine: 3,
941+
column: 14,
942+
endColumn: 16,
943+
},
944+
{
945+
messageId: 'neverOptionalChain',
946+
line: 3,
947+
endLine: 3,
948+
column: 19,
949+
endColumn: 21,
950+
},
951+
],
952+
},
953+
{
954+
code: `
955+
type baz = () => { qux: () => {} };
956+
declare const foo: { bar: { baz: baz } } | null;
957+
foo?.bar?.baz?.().qux?.();
958+
`,
959+
output: `
960+
type baz = () => { qux: () => {} };
961+
declare const foo: { bar: { baz: baz } } | null;
962+
foo?.bar.baz().qux();
963+
`,
964+
errors: [
965+
{
966+
messageId: 'neverOptionalChain',
967+
line: 4,
968+
endLine: 4,
969+
column: 9,
970+
endColumn: 11,
971+
},
972+
{
973+
messageId: 'neverOptionalChain',
974+
line: 4,
975+
endLine: 4,
976+
column: 14,
977+
endColumn: 16,
978+
},
979+
{
980+
messageId: 'neverOptionalChain',
981+
line: 4,
982+
endLine: 4,
983+
column: 22,
984+
endColumn: 24,
985+
},
986+
],
987+
},
988+
{
989+
code: `
990+
type baz = null | (() => { qux: () => {} });
991+
declare const foo: { bar: { baz: baz } } | null;
992+
foo?.bar?.baz?.().qux?.();
993+
`,
994+
output: `
995+
type baz = null | (() => { qux: () => {} });
996+
declare const foo: { bar: { baz: baz } } | null;
997+
foo?.bar.baz?.().qux();
998+
`,
999+
errors: [
1000+
{
1001+
messageId: 'neverOptionalChain',
1002+
line: 4,
1003+
endLine: 4,
1004+
column: 9,
1005+
endColumn: 11,
1006+
},
1007+
{
1008+
messageId: 'neverOptionalChain',
1009+
line: 4,
1010+
endLine: 4,
1011+
column: 22,
1012+
endColumn: 24,
1013+
},
1014+
],
1015+
},
1016+
{
1017+
code: `
1018+
type baz = null | (() => { qux: () => {} } | null);
1019+
declare const foo: { bar: { baz: baz } } | null;
1020+
foo?.bar?.baz?.()?.qux?.();
1021+
`,
1022+
output: `
1023+
type baz = null | (() => { qux: () => {} } | null);
1024+
declare const foo: { bar: { baz: baz } } | null;
1025+
foo?.bar.baz?.()?.qux();
1026+
`,
1027+
errors: [
1028+
{
1029+
messageId: 'neverOptionalChain',
1030+
line: 4,
1031+
endLine: 4,
1032+
column: 9,
1033+
endColumn: 11,
1034+
},
1035+
{
1036+
messageId: 'neverOptionalChain',
1037+
line: 4,
1038+
endLine: 4,
1039+
column: 23,
1040+
endColumn: 25,
1041+
},
1042+
],
1043+
},
1044+
{
1045+
code: `
1046+
type Foo = { baz: number };
1047+
type Bar = { baz: null | string | { qux: string } };
1048+
declare const foo: { fooOrBar: Foo | Bar } | null;
1049+
foo?.fooOrBar?.baz?.qux;
1050+
`,
1051+
output: `
1052+
type Foo = { baz: number };
1053+
type Bar = { baz: null | string | { qux: string } };
1054+
declare const foo: { fooOrBar: Foo | Bar } | null;
1055+
foo?.fooOrBar.baz?.qux;
1056+
`,
1057+
errors: [
1058+
{
1059+
messageId: 'neverOptionalChain',
1060+
line: 5,
1061+
endLine: 5,
1062+
column: 14,
1063+
endColumn: 16,
1064+
},
1065+
],
1066+
},
8391067
],
8401068
});

0 commit comments

Comments
 (0)