Skip to content

Commit

Permalink
fix(eslint-plugin): [no-unnecessary-condition] improve optional chain…
Browse files Browse the repository at this point in the history 
… handling 2 - electric boogaloo (#2138)
  • Loading branch information
@yeonjuan
yeonjuan authored Jun 1, 2020
1 parent 1cb8ca4 commit c87cfaf
Show file tree
Hide file tree
Showing 2 changed files with 274 additions and 9 deletions.
55 changes: 46 additions & 9 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
nullThrows,
NullThrowsReasons,
isMemberOrOptionalMemberExpression,
isIdentifier,
} from '../util';

// Truthiness utilities
Expand Down Expand Up @@ -426,6 +427,46 @@ export default createRule<Options, MessageId>({
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,
Expand All @@ -444,16 +485,12 @@ export default createRule<Options, MessageId>({
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;
}

Expand Down
228 changes: 228 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
},
],
},
],
});

0 comments on commit c87cfaf

Please sign in to comment.