Skip to content

Commit

Permalink
chore: turn on no-poorly-typed-ts-props (#1955)
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher committed Apr 30, 2020
1 parent b609b43 commit 80d934b
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 42 deletions.
14 changes: 8 additions & 6 deletions .eslintrc.js
Expand Up @@ -41,6 +41,14 @@ module.exports = {
{ allow: ['arrowFunctions'] },
],

//
// Internal repo rules
//

'@typescript-eslint/internal/no-poorly-typed-ts-props': 'error',
'@typescript-eslint/internal/no-typescript-default-import': 'error',
'@typescript-eslint/internal/prefer-ast-types-enum': 'error',

//
// eslint base
//
Expand Down Expand Up @@ -119,12 +127,6 @@ module.exports = {
'import/no-self-import': 'error',
// Require modules with a single export to use a default export
'import/prefer-default-export': 'off', // we want everything to be named

//
// Internal repo rules
//
'@typescript-eslint/internal/no-typescript-default-import': 'error',
'@typescript-eslint/internal/prefer-ast-types-enum': 'error',
},
parserOptions: {
sourceType: 'module',
Expand Down
5 changes: 3 additions & 2 deletions packages/eslint-plugin/src/rules/no-base-to-string.ts
Expand Up @@ -93,7 +93,8 @@ export default util.createRule<Options, MessageIds>({

function collectToStringCertainty(type: ts.Type): Usefulness {
const toString = typeChecker.getPropertyOfType(type, 'toString');
if (toString === undefined || toString.declarations.length === 0) {
const declarations = toString?.getDeclarations();
if (!toString || !declarations || declarations.length === 0) {
return Usefulness.Always;
}

Expand All @@ -107,7 +108,7 @@ export default util.createRule<Options, MessageIds>({
}

if (
toString.declarations.every(
declarations.every(
({ parent }) =>
!ts.isInterfaceDeclaration(parent) || parent.name.text !== 'Object',
)
Expand Down
Expand Up @@ -111,12 +111,13 @@ function getTypeParametersFromType(
}

const sym = getAliasedSymbol(symAtLocation, checker);
const declarations = sym.getDeclarations();

if (!sym.declarations) {
if (!declarations) {
return undefined;
}

return findFirstResult(sym.declarations, decl =>
return findFirstResult(declarations, decl =>
tsutils.isClassLikeDeclaration(decl) ||
ts.isTypeAliasDeclaration(decl) ||
ts.isInterfaceDeclaration(decl)
Expand Down
18 changes: 11 additions & 7 deletions packages/eslint-plugin/src/rules/prefer-includes.ts
Expand Up @@ -136,23 +136,27 @@ export default createRule({

// Get the symbol of `indexOf` method.
const tsNode = services.esTreeNodeToTSNodeMap.get(node.property);
const indexofMethodSymbol = types.getSymbolAtLocation(tsNode);
const indexofMethodDeclarations = types
.getSymbolAtLocation(tsNode)
?.getDeclarations();
if (
indexofMethodSymbol == null ||
indexofMethodSymbol.declarations.length === 0
indexofMethodDeclarations == null ||
indexofMethodDeclarations.length === 0
) {
return;
}

// Check if every declaration of `indexOf` method has `includes` method
// and the two methods have the same parameters.
for (const instanceofMethodDecl of indexofMethodSymbol.declarations) {
for (const instanceofMethodDecl of indexofMethodDeclarations) {
const typeDecl = instanceofMethodDecl.parent;
const type = types.getTypeAtLocation(typeDecl);
const includesMethodSymbol = type.getProperty('includes');
const includesMethodDecl = type
.getProperty('includes')
?.getDeclarations();
if (
includesMethodSymbol == null ||
!includesMethodSymbol.declarations.some(includesMethodDecl =>
includesMethodDecl == null ||
!includesMethodDecl.some(includesMethodDecl =>
hasSameParameters(includesMethodDecl, instanceofMethodDecl),
)
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/prefer-readonly.ts
Expand Up @@ -321,7 +321,7 @@ class ClassScope {
public addVariableModification(node: ts.PropertyAccessExpression): void {
const modifierType = this.checker.getTypeAtLocation(node.expression);
if (
modifierType.symbol === undefined ||
!modifierType.getSymbol() ||
!typeIsOrHasBaseType(modifierType, this.classType)
) {
return;
Expand Down
Expand Up @@ -129,7 +129,7 @@ export default createRule({
missingBranches: missingBranchTypes
.map(missingType =>
isTypeFlagSet(missingType, ts.TypeFlags.ESSymbolLike)
? `typeof ${missingType.symbol.escapedName}`
? `typeof ${missingType.getSymbol()?.escapedName}`
: checker.typeToString(missingType),
)
.join(' | '),
Expand Down
36 changes: 17 additions & 19 deletions packages/eslint-plugin/src/util/types.ts
Expand Up @@ -48,10 +48,8 @@ export function containsAllTypesByName(
type = type.target;
}

if (
typeof type.symbol !== 'undefined' &&
allowedNames.has(type.symbol.name)
) {
const symbol = type.getSymbol();
if (symbol && allowedNames.has(symbol.name)) {
return true;
}

Expand Down Expand Up @@ -90,11 +88,16 @@ export function getTypeName(
// `type.getConstraint()` method doesn't return the constraint type of
// the type parameter for some reason. So this gets the constraint type
// via AST.
const node = type.symbol.declarations[0] as ts.TypeParameterDeclaration;
if (node.constraint != null) {
const symbol = type.getSymbol();
const decls = symbol?.getDeclarations();
const typeParamDecl = decls?.[0] as ts.TypeParameterDeclaration;
if (
ts.isTypeParameterDeclaration(typeParamDecl) &&
typeParamDecl.constraint != null
) {
return getTypeName(
typeChecker,
typeChecker.getTypeFromTypeNode(node.constraint),
typeChecker.getTypeFromTypeNode(typeParamDecl.constraint),
);
}
}
Expand Down Expand Up @@ -174,12 +177,8 @@ export function getDeclaration(
if (!symbol) {
return null;
}
const declarations = symbol.declarations;
if (!declarations) {
return null;
}

return declarations[0];
const declarations = symbol.getDeclarations();
return declarations?.[0] ?? null;
}

/**
Expand Down Expand Up @@ -218,22 +217,21 @@ export function typeIsOrHasBaseType(
type: ts.Type,
parentType: ts.Type,
): boolean {
if (type.symbol === undefined || parentType.symbol === undefined) {
const parentSymbol = parentType.getSymbol();
if (!type.getSymbol() || !parentSymbol) {
return false;
}

const typeAndBaseTypes = [type];
const ancestorTypes = type.getBaseTypes();

if (ancestorTypes !== undefined) {
if (ancestorTypes) {
typeAndBaseTypes.push(...ancestorTypes);
}

for (const baseType of typeAndBaseTypes) {
if (
baseType.symbol !== undefined &&
baseType.symbol.name === parentType.symbol.name
) {
const baseSymbol = baseType.getSymbol();
if (baseSymbol && baseSymbol.name === parentSymbol.name) {
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/eslint-plugin/tests/rules/prefer-includes.test.ts
Expand Up @@ -57,7 +57,7 @@ ruleTester.run('prefer-includes', rule, {
`,
`
type UserDefined = {
indexOf(x: any): number // don't have 'includes'.
indexOf(x: any): number // don't have 'includes'
}
function f(a: UserDefined): void {
a.indexOf(b) !== -1
Expand All @@ -66,7 +66,7 @@ ruleTester.run('prefer-includes', rule, {
`
type UserDefined = {
indexOf(x: any, fromIndex?: number): number
includes(x: any): boolean // different parameters.
includes(x: any): boolean // different parameters
}
function f(a: UserDefined): void {
a.indexOf(b) !== -1
Expand All @@ -75,7 +75,7 @@ ruleTester.run('prefer-includes', rule, {
`
type UserDefined = {
indexOf(x: any, fromIndex?: number): number
includes(x: any, fromIndex: number): boolean // different parameters.
includes(x: any, fromIndex: number): boolean // different parameters
}
function f(a: UserDefined): void {
a.indexOf(b) !== -1
Expand All @@ -84,7 +84,7 @@ ruleTester.run('prefer-includes', rule, {
`
type UserDefined = {
indexOf(x: any, fromIndex?: number): number
includes: boolean // different type.
includes: boolean // different type
}
function f(a: UserDefined): void {
a.indexOf(b) !== -1
Expand Down

0 comments on commit 80d934b

Please sign in to comment.