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): added safe getTypeOfPropertyOfType wrapper #2567

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
10 changes: 8 additions & 2 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Expand Up @@ -24,6 +24,7 @@ import {
isTypeAnyType,
isTypeUnknownType,
getTypeName,
getTypeOfPropertyOfName,
} from '../util';

// Truthiness utilities
Expand Down Expand Up @@ -499,7 +500,8 @@ export default createRule<Options, MessageId>({
);
}
if (propertyType.isNumberLiteral() || propertyType.isStringLiteral()) {
const propType = checker.getTypeOfPropertyOfType(
const propType = getTypeOfPropertyOfName(
checker,
objType,
propertyType.value.toString(),
);
Expand Down Expand Up @@ -535,7 +537,11 @@ export default createRule<Options, MessageId>({
const propertyType = getNodeType(node.property);
return isNullablePropertyType(type, propertyType);
}
const propType = checker.getTypeOfPropertyOfType(type, property.name);
const propType = getTypeOfPropertyOfName(
checker,
type,
property.name,
);
return propType && isNullableType(propType, { allowUndefined: true });
});
return (
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -6,6 +6,7 @@ export * from './isTypeReadonly';
export * from './misc';
export * from './nullThrows';
export * from './objectIterators';
export * from './propertyTypes';
export * from './types';

// this is done for convenience - saves migrating all of the old rules
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/util/isTypeReadonly.ts
Expand Up @@ -6,7 +6,7 @@ import {
isPropertyReadonlyInType,
} from 'tsutils';
import * as ts from 'typescript';
import { nullThrows, NullThrowsReasons } from '.';
import { getTypeOfPropertyOfType, nullThrows, NullThrowsReasons } from '.';

const enum Readonlyness {
/** the type cannot be handled by the function */
Expand Down Expand Up @@ -101,7 +101,7 @@ function isTypeReadonlyObject(
// doing this deep, potentially expensive check.
for (const property of properties) {
const propertyType = nullThrows(
checker.getTypeOfPropertyOfType(type, property.getName()),
getTypeOfPropertyOfType(checker, type, property),
NullThrowsReasons.MissingToken(`property "${property.name}"`, 'type'),
);

Expand Down
36 changes: 36 additions & 0 deletions packages/eslint-plugin/src/util/propertyTypes.ts
@@ -0,0 +1,36 @@
import * as ts from 'typescript';

export function getTypeOfPropertyOfName(
checker: ts.TypeChecker,
type: ts.Type,
name: string,
escapedName?: ts.__String,
): ts.Type | undefined {
// Most names are directly usable in the checker and aren't different from escaped names
if (!escapedName || !name.startsWith('__')) {
return checker.getTypeOfPropertyOfType(type, name);
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
}

// Symbolic names may differ in their escaped name compared to their human-readable name
// https://github.com/typescript-eslint/typescript-eslint/issues/2143
const escapedProperty = type
.getProperties()
.find(property => property.escapedName === escapedName);

return escapedProperty
? checker.getDeclaredTypeOfSymbol(escapedProperty)
: undefined;
}

export function getTypeOfPropertyOfType(
checker: ts.TypeChecker,
type: ts.Type,
property: ts.Symbol,
): ts.Type | undefined {
return getTypeOfPropertyOfName(
checker,
type,
property.getName(),
property.getEscapedName(),
);
}
Expand Up @@ -238,6 +238,15 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
}
function foo(arg: Readonly<Foo>) {}
`,
`
const sym = Symbol('sym');

interface WithSymbol {
[sym]: number;
}

const willNotCrash = (foo: Readonly<WithSymbol>) => {};
`,
],
invalid: [
// arrays
Expand Down Expand Up @@ -643,5 +652,24 @@ ruleTester.run('prefer-readonly-parameter-types', rule, {
},
],
},
{
code: `
const sym = Symbol('sym');

interface WithSymbol {
[sym]: number;
}

const willNot = (foo: WithSymbol) => {};
`,
errors: [
{
messageId: 'shouldBeReadonly',
line: 8,
column: 26,
endColumn: 41,
},
],
},
],
});