Skip to content

Commit

Permalink
fix(eslint-plugin): [no-unnecessary-condition] handle computed member…
Browse files Browse the repository at this point in the history
… access (#2288)
  • Loading branch information
yeonjuan committed Jul 13, 2020
1 parent 98ab010 commit 3a187ca
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 1 deletion.
33 changes: 32 additions & 1 deletion packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Expand Up @@ -24,6 +24,7 @@ import {
isIdentifier,
isTypeAnyType,
isTypeUnknownType,
getTypeName,
} from '../util';

// Truthiness utilities
Expand Down Expand Up @@ -435,6 +436,33 @@ export default createRule<Options, MessageId>({
return false;
}

function isNullablePropertyType(
objType: ts.Type,
propertyType: ts.Type,
): boolean {
if (propertyType.isUnion()) {
return propertyType.types.some(type =>
isNullablePropertyType(objType, type),
);
}
if (propertyType.isNumberLiteral() || propertyType.isStringLiteral()) {
const propType = checker.getTypeOfPropertyOfType(
objType,
propertyType.value.toString(),
);
if (propType) {
return isNullableType(propType, { allowUndefined: true });
}
}
const typeName = getTypeName(checker, propertyType);
return !!(
(typeName === 'string' &&
checker.getIndexInfoOfType(objType, ts.IndexKind.String)) ||
(typeName === 'number' &&
checker.getIndexInfoOfType(objType, ts.IndexKind.Number))
);
}

// Checks whether a member expression is nullable or not regardless of it's previous node.
// Example:
// ```
Expand All @@ -450,10 +478,13 @@ export default createRule<Options, MessageId>({
const property = node.property;
if (prevType.isUnion() && isIdentifier(property)) {
const isOwnNullable = prevType.types.some(type => {
if (node.computed) {
const propertyType = getNodeType(node.property);
return isNullablePropertyType(type, propertyType);
}
const propType = checker.getTypeOfPropertyOfType(type, property.name);
return propType && isNullableType(propType, { allowUndefined: true });
});

return (
!isOwnNullable && isNullableType(prevType, { allowUndefined: true })
);
Expand Down
171 changes: 171 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
Expand Up @@ -398,6 +398,53 @@ type Bar = { baz: null | string | { qux: string } };
declare const foo: { fooOrBar: Foo | Bar } | null;
foo?.fooOrBar?.baz?.qux;
`,
`
type Foo = { [key: string]: string } | null;
declare const foo: Foo;
const key = '1';
foo?.[key]?.trim();
`,
`
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
type Key = 'bar' | 'foo';
declare const foo: Foo;
declare const key: Key;
foo?.[key].trim();
`,
`
interface Outer {
inner?: {
[key: string]: string | undefined;
};
}
function Foo(outer: Outer, key: string): number | undefined {
return outer.inner?.[key]?.charCodeAt(0);
}
`,
`
interface Outer {
inner?: {
[key: string]: string | undefined;
bar: 'bar';
};
}
type Foo = 'foo';
function Foo(outer: Outer, key: Foo): number | undefined {
return outer.inner?.[key]?.charCodeAt(0);
}
`,
`
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
type Key = 'bar' | 'foo' | 'baz';
declare const foo: Foo;
declare const key: Key;
foo?.[key]?.trim();
`,
],
invalid: [
// Ensure that it's checking in all the right places
Expand Down Expand Up @@ -1196,5 +1243,129 @@ foo?.fooOrBar.baz?.qux;
},
],
},
{
code: `
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
type Key = 'bar' | 'foo';
declare const foo: Foo;
declare const key: Key;
foo?.[key]?.trim();
`,
output: `
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
type Key = 'bar' | 'foo';
declare const foo: Foo;
declare const key: Key;
foo?.[key].trim();
`,
errors: [
{
messageId: 'neverOptionalChain',
line: 7,
endLine: 7,
column: 11,
endColumn: 13,
},
],
},
{
code: `
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
declare const foo: Foo;
const key = 'bar';
foo?.[key]?.trim();
`,
output: `
type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
declare const foo: Foo;
const key = 'bar';
foo?.[key].trim();
`,
errors: [
{
messageId: 'neverOptionalChain',
line: 5,
endLine: 5,
column: 11,
endColumn: 13,
},
],
},
{
code: `
interface Outer {
inner?: {
[key: string]: string | undefined;
bar: 'bar';
};
}
export function test(outer: Outer): number | undefined {
const key = 'bar';
return outer.inner?.[key]?.charCodeAt(0);
}
`,
output: `
interface Outer {
inner?: {
[key: string]: string | undefined;
bar: 'bar';
};
}
export function test(outer: Outer): number | undefined {
const key = 'bar';
return outer.inner?.[key].charCodeAt(0);
}
`,
errors: [
{
messageId: 'neverOptionalChain',
line: 11,
endLine: 11,
column: 28,
endColumn: 30,
},
],
},
{
code: `
interface Outer {
inner?: {
[key: string]: string | undefined;
bar: 'bar';
};
}
type Bar = 'bar';
function Foo(outer: Outer, key: Bar): number | undefined {
return outer.inner?.[key]?.charCodeAt(0);
}
`,
output: `
interface Outer {
inner?: {
[key: string]: string | undefined;
bar: 'bar';
};
}
type Bar = 'bar';
function Foo(outer: Outer, key: Bar): number | undefined {
return outer.inner?.[key].charCodeAt(0);
}
`,
errors: [
{
messageId: 'neverOptionalChain',
line: 11,
endLine: 11,
column: 28,
endColumn: 30,
},
],
},
],
});

0 comments on commit 3a187ca

Please sign in to comment.