Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(eslint-plugin): [no-unnecessary-condition] improve optional chain handling 2 - electric boogaloo #2138

Merged
merged 9 commits into from Jun 1, 2020
Merged

fix(eslint-plugin): [no-unnecessary-condition] improve optional chain handling 2 - electric boogaloo #2138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f7b727b
fix(eslint-plugin): [no-unnecessary-condition] improve optional chain…
yeonjuan May 31, 2020
8ab4cb5
fix formatting
yeonjuan May 31, 2020
ef4ff94
refactor
yeonjuan May 31, 2020
2a2e9ec
add test case
yeonjuan May 31, 2020
378cae0
fix lint
yeonjuan May 31, 2020
2c3c383
trailing comma
yeonjuan May 31, 2020
77721a6
delete not occurred case
yeonjuan May 31, 2020
07fde54
refactor
yeonjuan May 31, 2020
16e5a14
fix false neg
yeonjuan Jun 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 46 additions & 9 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
View file
Open in desktop
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
Open in desktop
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,
},
],
},
],
});