From 0a110eb680749c8c4a2a3dc1375c1a83056e4c14 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Tue, 4 Feb 2020 06:56:39 +1300 Subject: [PATCH] feat(eslint-plugin): [unbound-method] support bound builtins (#1526) Co-authored-by: Brad Zacher --- .../eslint-plugin/src/rules/unbound-method.ts | 68 +++++++++++++++++++ .../eslint-plugin/tests/fixtures/class.ts | 3 + .../tests/rules/unbound-method.test.ts | 36 ++++++++++ packages/eslint-plugin/typings/node.d.ts | 8 +++ 4 files changed, 115 insertions(+) create mode 100644 packages/eslint-plugin/typings/node.d.ts diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 32df694637e..bdd740fb4d4 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -18,6 +18,59 @@ export type Options = [Config]; export type MessageIds = 'unbound'; +const nativelyBoundMembers = ([ + 'Promise', + 'Number', + 'Object', + 'String', // eslint-disable-line @typescript-eslint/internal/prefer-ast-types-enum + 'RegExp', + 'Symbol', + 'Array', + 'Proxy', + 'Date', + 'Infinity', + 'Atomics', + 'Reflect', + 'console', + 'Math', + 'JSON', + 'Intl', +] as const) + .map(namespace => { + const object = global[namespace]; + return Object.getOwnPropertyNames(object) + .filter( + name => + !name.startsWith('_') && + typeof (object as Record)[name] === 'function', + ) + .map(name => `${namespace}.${name}`); + }) + .reduce((arr, names) => arr.concat(names), []); + +const isMemberNotImported = ( + symbol: ts.Symbol, + currentSourceFile: ts.SourceFile | undefined, +): boolean => { + const { valueDeclaration } = symbol; + if (!valueDeclaration) { + // working around https://github.com/microsoft/TypeScript/issues/31294 + return false; + } + + return ( + !!currentSourceFile && + currentSourceFile !== valueDeclaration.getSourceFile() + ); +}; + +const getNodeName = (node: TSESTree.Node): string | null => + node.type === AST_NODE_TYPES.Identifier ? node.name : null; + +const getMemberFullName = ( + node: TSESTree.MemberExpression | TSESTree.OptionalMemberExpression, +): string => `${getNodeName(node.object)}.${getNodeName(node.property)}`; + export default util.createRule({ name: 'unbound-method', meta: { @@ -53,6 +106,9 @@ export default util.createRule({ create(context, [{ ignoreStatic }]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); + const currentSourceFile = parserServices.program.getSourceFile( + context.getFilename(), + ); return { 'MemberExpression, OptionalMemberExpression'( @@ -62,6 +118,18 @@ export default util.createRule({ return; } + const objectSymbol = checker.getSymbolAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(node.object), + ); + + if ( + objectSymbol && + nativelyBoundMembers.includes(getMemberFullName(node)) && + isMemberNotImported(objectSymbol, currentSourceFile) + ) { + return; + } + const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); const symbol = checker.getSymbolAtLocation(originalNode); diff --git a/packages/eslint-plugin/tests/fixtures/class.ts b/packages/eslint-plugin/tests/fixtures/class.ts index 9b8aac7d7e3..74f222338ff 100644 --- a/packages/eslint-plugin/tests/fixtures/class.ts +++ b/packages/eslint-plugin/tests/fixtures/class.ts @@ -1,2 +1,5 @@ // used by no-throw-literal test case to validate custom error export class Error {} + +// used by unbound-method test case to test imports +export const console = { log() {} }; diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index dcf58b6447e..2bf38e1c83e 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -7,6 +7,7 @@ const rootPath = getFixturesRootDir(); const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', parserOptions: { + sourceType: 'module', tsconfigRootDir: rootPath, project: './tsconfig.json', }, @@ -43,6 +44,10 @@ function addContainsMethodsClassInvalid( ruleTester.run('unbound-method', rule, { valid: [ + 'Promise.resolve().then(console.log)', + '["1", "2", "3"].map(Number.parseInt)', + '[5.2, 7.1, 3.6].map(Math.floor);', + 'const x = console.log', ...[ 'instance.bound();', 'instance.unbound();', @@ -208,6 +213,37 @@ if(myCondition || x.mightBeDefined) { `, ], invalid: [ + { + code: ` +class Console { + log(str) { + process.stdout.write(str); + } +} + +const console = new Console(); + +Promise.resolve().then(console.log); + `, + errors: [ + { + line: 10, + messageId: 'unbound', + }, + ], + }, + { + code: ` +import { console } from './class'; +const x = console.log; + `, + errors: [ + { + line: 3, + messageId: 'unbound', + }, + ], + }, { code: addContainsMethodsClass(` function foo(arg: ContainsMethods | null) { diff --git a/packages/eslint-plugin/typings/node.d.ts b/packages/eslint-plugin/typings/node.d.ts new file mode 100644 index 00000000000..6caf4f04554 --- /dev/null +++ b/packages/eslint-plugin/typings/node.d.ts @@ -0,0 +1,8 @@ +// augment nodejs global with ES2015+ things +declare namespace NodeJS { + interface Global { + Atomics: typeof Atomics; + Proxy: typeof Proxy; + Reflect: typeof Reflect; + } +}