From 56ea7c9581c0c99fe394bbcfc4128e8054c88ab2 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 29 Apr 2020 16:20:16 -0700 Subject: [PATCH] feat(eslint-plugin-internal): add rule no-poorly-typed-ts-props (#1949) --- .../eslint-plugin-internal/src/rules/index.ts | 2 + .../src/rules/no-poorly-typed-ts-props.ts | 110 ++++++++++++++++++ .../tests/fixtures/file.ts | 0 .../tests/fixtures/tsconfig.json | 12 ++ .../rules/no-poorly-typed-ts-props.test.ts | 94 +++++++++++++++ 5 files changed, 218 insertions(+) create mode 100644 packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts create mode 100644 packages/eslint-plugin-internal/tests/fixtures/file.ts create mode 100644 packages/eslint-plugin-internal/tests/fixtures/tsconfig.json create mode 100644 packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts diff --git a/packages/eslint-plugin-internal/src/rules/index.ts b/packages/eslint-plugin-internal/src/rules/index.ts index eb8d8efe852..17a0a192bf8 100644 --- a/packages/eslint-plugin-internal/src/rules/index.ts +++ b/packages/eslint-plugin-internal/src/rules/index.ts @@ -1,9 +1,11 @@ +import noPoorlyTypedTsProps from './no-poorly-typed-ts-props'; import noTypescriptDefaultImport from './no-typescript-default-import'; import noTypescriptEstreeImport from './no-typescript-estree-import'; import pluginTestFormatting from './plugin-test-formatting'; import preferASTTypesEnum from './prefer-ast-types-enum'; export default { + 'no-poorly-typed-ts-props': noPoorlyTypedTsProps, 'no-typescript-default-import': noTypescriptDefaultImport, 'no-typescript-estree-import': noTypescriptEstreeImport, 'plugin-test-formatting': pluginTestFormatting, diff --git a/packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts b/packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts new file mode 100644 index 00000000000..a1485e2435f --- /dev/null +++ b/packages/eslint-plugin-internal/src/rules/no-poorly-typed-ts-props.ts @@ -0,0 +1,110 @@ +import { + TSESTree, + ESLintUtils, + TSESLint, +} from '@typescript-eslint/experimental-utils'; +import { createRule } from '../util'; + +/* +TypeScript declares some bad types for certain properties. +See: https://github.com/microsoft/TypeScript/issues/24706 + +This rule simply warns against using them, as using them will likely introduce type safety holes. +*/ + +const BANNED_PROPERTIES = [ + // { + // type: 'Node', + // property: 'parent', + // fixWith: null, + // }, + { + type: 'Symbol', + property: 'declarations', + fixWith: 'getDeclarations()', + }, + { + type: 'Type', + property: 'symbol', + fixWith: 'getSymbol()', + }, +]; + +export default createRule({ + name: 'no-poorly-typed-ts-props', + meta: { + type: 'problem', + docs: { + description: + "Enforces rules don't use TS API properties with known bad type definitions", + category: 'Possible Errors', + recommended: 'error', + requiresTypeChecking: true, + }, + fixable: 'code', + schema: [], + messages: { + doNotUse: 'Do not use {{type}}.{{property}} because it is poorly typed.', + doNotUseWithFixer: + 'Do not use {{type}}.{{property}} because it is poorly typed. Use {{type}}.{{fixWith}} instead.', + suggestedFix: 'Use {{type}}.{{fixWith}} instead.', + }, + }, + defaultOptions: [], + create(context) { + const { program, esTreeNodeToTSNodeMap } = ESLintUtils.getParserServices( + context, + ); + const checker = program.getTypeChecker(); + + return { + ':matches(MemberExpression, OptionalMemberExpression)[computed = false]'( + node: + | TSESTree.MemberExpressionNonComputedName + | TSESTree.OptionalMemberExpressionNonComputedName, + ): void { + for (const banned of BANNED_PROPERTIES) { + if (node.property.name !== banned.property) { + continue; + } + + // make sure the type name matches + const tsObjectNode = esTreeNodeToTSNodeMap.get(node.object); + const objectType = checker.getTypeAtLocation(tsObjectNode); + const objectSymbol = objectType.getSymbol(); + if (objectSymbol?.getName() !== banned.type) { + continue; + } + + const tsNode = esTreeNodeToTSNodeMap.get(node.property); + const symbol = checker.getSymbolAtLocation(tsNode); + const decls = symbol?.getDeclarations(); + const isFromTs = decls?.some(decl => + decl.getSourceFile().fileName.includes('/node_modules/typescript/'), + ); + if (isFromTs !== true) { + continue; + } + + return context.report({ + node, + messageId: banned.fixWith ? 'doNotUseWithFixer' : 'doNotUse', + data: banned, + suggest: [ + { + messageId: 'suggestedFix', + fix(fixer): TSESLint.RuleFix | null { + if (banned.fixWith == null) { + return null; + } + + return fixer.replaceText(node.property, banned.fixWith); + }, + }, + ], + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin-internal/tests/fixtures/file.ts b/packages/eslint-plugin-internal/tests/fixtures/file.ts new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/eslint-plugin-internal/tests/fixtures/tsconfig.json b/packages/eslint-plugin-internal/tests/fixtures/tsconfig.json new file mode 100644 index 00000000000..7e9126b848c --- /dev/null +++ b/packages/eslint-plugin-internal/tests/fixtures/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "jsx": "preserve", + "target": "es5", + "module": "commonjs", + "strict": true, + "esModuleInterop": true, + "lib": ["es2015", "es2017", "esnext"], + "experimentalDecorators": true + }, + "include": ["file.ts"] +} diff --git a/packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts b/packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts new file mode 100644 index 00000000000..63b1f8d5726 --- /dev/null +++ b/packages/eslint-plugin-internal/tests/rules/no-poorly-typed-ts-props.test.ts @@ -0,0 +1,94 @@ +import rule from '../../src/rules/no-poorly-typed-ts-props'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: getFixturesRootDir(), + sourceType: 'module', + }, +}); + +ruleTester.run('no-poorly-typed-ts-props', rule, { + valid: [ + ` +declare const foo: { declarations: string[] }; +foo.declarations.map(decl => console.log(decl)); + `, + ` +declare const bar: Symbol; +bar.declarations.map(decl => console.log(decl)); + `, + ` +declare const baz: Type; +baz.symbol.name; + `, + ], + invalid: [ + { + code: ` +import ts from 'typescript'; +declare const thing: ts.Symbol; +thing.declarations.map(decl => {}); + `.trimRight(), + errors: [ + { + messageId: 'doNotUseWithFixer', + data: { + type: 'Symbol', + property: 'declarations', + fixWith: 'getDeclarations()', + }, + line: 4, + suggestions: [ + { + messageId: 'suggestedFix', + data: { + type: 'Symbol', + fixWith: 'getDeclarations()', + }, + output: ` +import ts from 'typescript'; +declare const thing: ts.Symbol; +thing.getDeclarations().map(decl => {}); + `.trimRight(), + }, + ], + }, + ], + }, + { + code: ` +import ts from 'typescript'; +declare const thing: ts.Type; +thing.symbol; + `.trimRight(), + errors: [ + { + messageId: 'doNotUseWithFixer', + data: { + type: 'Type', + property: 'symbol', + fixWith: 'getSymbol()', + }, + line: 4, + suggestions: [ + { + messageId: 'suggestedFix', + data: { + type: 'Type', + fixWith: 'getSymbol()', + }, + output: ` +import ts from 'typescript'; +declare const thing: ts.Type; +thing.getSymbol(); + `.trimRight(), + }, + ], + }, + ], + }, + ], +});