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.

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

fix(eslint-plugin): false-positive/negative with array index in no-unnecessary-condition #3805

Merged
merged 3 commits into from Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 17 additions & 11 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Expand Up @@ -261,12 +261,6 @@ export default createRule<Options, MessageId>({
}

function checkNodeForNullish(node: TSESTree.Expression): void {
// Since typescript array index signature types don't represent the
// possibility of out-of-bounds access, if we're indexing into an array
// just skip the check, to avoid false positives
if (isArrayIndexExpression(node)) {
return;
}
const type = getNodeType(node);
// Conditional is always necessary if it involves `any` or `unknown`
if (isTypeAnyType(type) || isTypeUnknownType(type)) {
Expand All @@ -277,7 +271,19 @@ export default createRule<Options, MessageId>({
if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
messageId = 'never';
} else if (!isPossiblyNullish(type)) {
messageId = 'neverNullish';
// Since typescript array index signature types don't represent the
// possibility of out-of-bounds access, if we're indexing into an array
// just skip the check, to avoid false positives
if (
!isArrayIndexExpression(node) &&
!(
node.type === AST_NODE_TYPES.ChainExpression &&
node.expression.type !== AST_NODE_TYPES.TSNonNullExpression &&
optionChainContainsOptionArrayIndex(node.expression)
)
) {
messageId = 'neverNullish';
}
} else if (isAlwaysNullish(type)) {
messageId = 'alwaysNullish';
}
Expand Down Expand Up @@ -477,19 +483,19 @@ export default createRule<Options, MessageId>({
// ?.x // type is {y: "z"}
// ?.y // This access is considered "unnecessary" according to the types
// ```
function optionChainContainsArrayIndex(
function optionChainContainsOptionArrayIndex(
node: TSESTree.MemberExpression | TSESTree.CallExpression,
): boolean {
const lhsNode =
node.type === AST_NODE_TYPES.CallExpression ? node.callee : node.object;
if (isArrayIndexExpression(lhsNode)) {
if (node.optional && isArrayIndexExpression(lhsNode)) {
return true;
}
if (
lhsNode.type === AST_NODE_TYPES.MemberExpression ||
lhsNode.type === AST_NODE_TYPES.CallExpression
) {
return optionChainContainsArrayIndex(lhsNode);
return optionChainContainsOptionArrayIndex(lhsNode);
}
return false;
}
Expand Down Expand Up @@ -584,7 +590,7 @@ export default createRule<Options, MessageId>({
// Since typescript array index signature types don't represent the
// possibility of out-of-bounds access, if we're indexing into an array
// just skip the check, to avoid false positives
if (optionChainContainsArrayIndex(node)) {
if (optionChainContainsOptionArrayIndex(node)) {
return;
}

Expand Down
Expand Up @@ -315,6 +315,11 @@ returnsArr?.()[42]?.toUpperCase();
`
declare const arr: string[][];
arr[x] ?? [];
`,
// nullish + optional array index
`
declare const arr: { foo: number }[];
const bar = arr[42]?.foo ?? 0;
`,
// Doesn't check the right-hand side of a logical expression
// in a non-conditional context
Expand Down Expand Up @@ -751,6 +756,15 @@ function test(a: string) {
code: `
function test(a: string | false) {
return a ?? 'default';
}
`,
errors: [ruleError(3, 10, 'neverNullish')],
},
// nullish + array index without optional chaining
{
code: `
function test(a: { foo: string }[]) {
return a[0].foo ?? 'default';
}
`,
errors: [ruleError(3, 10, 'neverNullish')],
Expand All @@ -765,6 +779,14 @@ function test(a: null) {
},
{
code: `
function test(a: null[]) {
return a[0] ?? 'default';
}
`,
errors: [ruleError(3, 10, 'alwaysNullish')],
},
{
code: `
function test(a: never) {
return a ?? 'default';
}
Expand Down Expand Up @@ -1315,6 +1337,17 @@ foo?.fooOrBar.baz?.qux;
},
{
code: `
declare const x: { a: { b: number } }[];
x[0].a?.b;
`,
output: `
declare const x: { a: { b: number } }[];
x[0].a.b;
`,
errors: [ruleError(3, 7, 'neverOptionalChain')],
},
{
code: `
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
type Key = 'bar' | 'foo';
declare const foo: Foo;
Expand Down