From ac86a79bd416f031beccc7bdac28a938cb354ba5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 20 Jun 2021 19:10:03 -0400 Subject: [PATCH] fix(eslint-plugin): [prefer-regexp-exec] factor in union types (#3434) --- .../src/rules/prefer-regexp-exec.ts | 114 +++++++++++------- .../tests/rules/prefer-regexp-exec.test.ts | 93 ++++---------- 2 files changed, 92 insertions(+), 115 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts index 636fe2b3825..0d6693715ba 100644 --- a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts +++ b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts @@ -2,6 +2,8 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; import { createRule, getParserServices, @@ -10,6 +12,13 @@ import { getWrappingFixer, } from '../util'; +enum ArgumentType { + Other = 0, + String = 1 << 0, + RegExp = 1 << 1, + Both = String | RegExp, +} + export default createRule({ name: 'prefer-regexp-exec', defaultOptions: [], @@ -37,25 +46,33 @@ export default createRule({ const sourceCode = context.getSourceCode(); /** - * Check if a given node is a string. - * @param node The node to check. + * Check if a given node type is a string. + * @param node The node type to check. */ - function isStringType(node: TSESTree.Node): boolean { - const objectType = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node), - ); - return getTypeName(typeChecker, objectType) === 'string'; + function isStringType(type: ts.Type): boolean { + return getTypeName(typeChecker, type) === 'string'; } /** - * Check if a given node is a RegExp. - * @param node The node to check. + * Check if a given node type is a RegExp. + * @param node The node type to check. */ - function isRegExpType(node: TSESTree.Node): boolean { - const objectType = typeChecker.getTypeAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node), - ); - return getTypeName(typeChecker, objectType) === 'RegExp'; + function isRegExpType(type: ts.Type): boolean { + return getTypeName(typeChecker, type) === 'RegExp'; + } + + function collectArgumentTypes(types: ts.Type[]): ArgumentType { + let result = ArgumentType.Other; + + for (const type of types) { + if (isRegExpType(type)) { + result |= ArgumentType.RegExp; + } else if (isStringType(type)) { + result |= ArgumentType.String; + } + } + + return result; } return { @@ -67,7 +84,13 @@ export default createRule({ const argumentNode = callNode.arguments[0]; const argumentValue = getStaticValue(argumentNode, globalScope); - if (!isStringType(objectNode)) { + if ( + !isStringType( + typeChecker.getTypeAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(objectNode), + ), + ) + ) { return; } @@ -97,38 +120,39 @@ export default createRule({ }); } - if (isRegExpType(argumentNode)) { - return context.report({ - node: memberNode.property, - messageId: 'regExpExecOverStringMatch', - fix: getWrappingFixer({ - sourceCode, - node: callNode, - innerNode: [objectNode, argumentNode], - wrap: (objectCode, argumentCode) => - `${argumentCode}.exec(${objectCode})`, - }), - }); - } + const argumentType = typeChecker.getTypeAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(argumentNode), + ); + const argumentTypes = collectArgumentTypes( + tsutils.unionTypeParts(argumentType), + ); + switch (argumentTypes) { + case ArgumentType.RegExp: + return context.report({ + node: memberNode.property, + messageId: 'regExpExecOverStringMatch', + fix: getWrappingFixer({ + sourceCode, + node: callNode, + innerNode: [objectNode, argumentNode], + wrap: (objectCode, argumentCode) => + `${argumentCode}.exec(${objectCode})`, + }), + }); - if (isStringType(argumentNode)) { - return context.report({ - node: memberNode.property, - messageId: 'regExpExecOverStringMatch', - fix: getWrappingFixer({ - sourceCode, - node: callNode, - innerNode: [objectNode, argumentNode], - wrap: (objectCode, argumentCode) => - `RegExp(${argumentCode}).exec(${objectCode})`, - }), - }); + case ArgumentType.String: + return context.report({ + node: memberNode.property, + messageId: 'regExpExecOverStringMatch', + fix: getWrappingFixer({ + sourceCode, + node: callNode, + innerNode: [objectNode, argumentNode], + wrap: (objectCode, argumentCode) => + `RegExp(${argumentCode}).exec(${objectCode})`, + }), + }); } - - return context.report({ - node: memberNode.property, - messageId: 'regExpExecOverStringMatch', - }); }, }; }, diff --git a/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts b/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts index 95cb567d78b..b4593510807 100644 --- a/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts @@ -33,6 +33,29 @@ function f(s: string | string[]) { s.match(/e/); } `, + "(Math.random() > 0.5 ? 'abc' : 123).match(2);", + "'212'.match(2);", + "'212'.match(+2);", + "'oNaNo'.match(NaN);", + "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(Infinity);", + "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(+Infinity);", + "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(-Infinity);", + "'void and null'.match(null);", + ` +const matchers = ['package-lock.json', /regexp/]; +const file = ''; +matchers.some(matcher => !!file.match(matcher)); + `, + ` +const matchers = [/regexp/, 'package-lock.json']; +const file = ''; +matchers.some(matcher => !!file.match(matcher)); + `, + ` +const matchers = [{ match: (s: RegExp) => false }]; +const file = ''; +matchers.some(matcher => !!file.match(matcher)); + `, ], invalid: [ { @@ -95,76 +118,6 @@ const search = 'thing'; RegExp(search).exec(text); `, }, - { - code: "'212'.match(2);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 7, - }, - ], - }, - { - code: "'212'.match(+2);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 7, - }, - ], - }, - { - code: "'oNaNo'.match(NaN);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 9, - }, - ], - }, - { - code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(Infinity);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 60, - }, - ], - }, - { - code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(+Infinity);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 60, - }, - ], - }, - { - code: "'Infinity contains -Infinity and +Infinity in JavaScript.'.match(-Infinity);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 60, - }, - ], - }, - { - code: "'void and null'.match(null);", - errors: [ - { - messageId: 'regExpExecOverStringMatch', - line: 1, - column: 17, - }, - ], - }, { code: ` function f(s: 'a' | 'b') {