Merge branch 'master' into feat/internal

typescript-eslint · Sep 21, 2021 · ccd20cd · ccd20cd
2 parents 431afbd + dda9cee
commit ccd20cd
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 31 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-require-imports.ts b/packages/eslint-plugin/src/rules/no-require-imports.ts
@@ -1,4 +1,4 @@
-import { TSESTree } from '@typescript-eslint/experimental-utils';
+import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
 import * as util from '../util';
 
 export default util.createRule({
@@ -18,13 +18,19 @@ export default util.createRule({
   defaultOptions: [],
   create(context) {
     return {
-      'CallExpression > Identifier[name="require"]'(
-        node: TSESTree.Identifier,
+      'CallExpression[callee.name="require"]'(
+        node: TSESTree.CallExpression,
       ): void {
-        context.report({
-          node: node.parent!,
-          messageId: 'noRequireImports',
-        });
+        const variable = ASTUtils.findVariable(context.getScope(), 'require');
+
+        // ignore non-global require usage as it's something user-land custom instead
+        // of the commonjs standard
+        if (!variable?.identifiers.length) {
+          context.report({
+            node,
+            messageId: 'noRequireImports',
+          });
+        }
       },
       TSExternalModuleReference(node): void {
         context.report({

diff --git a/packages/eslint-plugin/src/rules/no-shadow.ts b/packages/eslint-plugin/src/rules/no-shadow.ts
@@ -3,7 +3,12 @@ import {
   TSESLint,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
-import { DefinitionType, ScopeType } from '@typescript-eslint/scope-manager';
+import {
+  Definition,
+  DefinitionType,
+  ImportBindingDefinition,
+  ScopeType,
+} from '@typescript-eslint/scope-manager';
 import * as util from '../util';
 
 type MessageIds = 'noShadow';
@@ -88,6 +93,15 @@ export default util.createRule<Options, MessageIds>({
       );
     }
 
+    function isTypeImport(
+      definition: Definition,
+    ): definition is ImportBindingDefinition {
+      return (
+        definition.type === DefinitionType.ImportBinding &&
+        definition.parent.importKind === 'type'
+      );
+    }
+
     function isTypeValueShadow(
       variable: TSESLint.Scope.Variable,
       shadowed: TSESLint.Scope.Variable,
@@ -101,8 +115,11 @@ export default util.createRule<Options, MessageIds>({
         return false;
       }
 
+      const [firstDefinition] = shadowed.defs;
       const isShadowedValue =
-        'isValueVariable' in shadowed ? shadowed.isValueVariable : true;
+        !('isValueVariable' in shadowed) ||
+        !firstDefinition ||
+        (!isTypeImport(firstDefinition) && shadowed.isValueVariable);
       return variable.isValueVariable !== isShadowedValue;
     }
 

diff --git a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts
@@ -75,13 +75,31 @@ export default createRule({
       return result;
     }
 
+    function isLikelyToContainGlobalFlag(
+      node: TSESTree.CallExpressionArgument,
+    ): boolean {
+      if (
+        node.type === AST_NODE_TYPES.CallExpression ||
+        node.type === AST_NODE_TYPES.NewExpression
+      ) {
+        const [, flags] = node.arguments;
+        return (
+          flags.type === AST_NODE_TYPES.Literal &&
+          typeof flags.value === 'string' &&
+          flags.value.includes('g')
+        );
+      }
+
+      return node.type === AST_NODE_TYPES.Identifier;
+    }
+
     return {
       "CallExpression[arguments.length=1] > MemberExpression.callee[property.name='match'][computed=false]"(
         memberNode: TSESTree.MemberExpression,
       ): void {
         const objectNode = memberNode.object;
         const callNode = memberNode.parent as TSESTree.CallExpression;
-        const argumentNode = callNode.arguments[0];
+        const [argumentNode] = callNode.arguments;
         const argumentValue = getStaticValue(argumentNode, globalScope);
 
         if (
@@ -96,9 +114,10 @@ export default createRule({
 
         // Don't report regular expressions with global flag.
         if (
-          argumentValue &&
-          argumentValue.value instanceof RegExp &&
-          argumentValue.value.flags.includes('g')
+          (!argumentValue && isLikelyToContainGlobalFlag(argumentNode)) ||
+          (argumentValue &&
+            argumentValue.value instanceof RegExp &&
+            argumentValue.value.flags.includes('g'))
         ) {
           return;
         }

diff --git a/packages/eslint-plugin/src/rules/prefer-return-this-type.ts b/packages/eslint-plugin/src/rules/prefer-return-this-type.ts
@@ -1,9 +1,9 @@
 import {
-  TSESTree,
   AST_NODE_TYPES,
+  TSESTree,
 } from '@typescript-eslint/experimental-utils';
-import { createRule, forEachReturnStatement, getParserServices } from '../util';
 import * as ts from 'typescript';
+import { createRule, forEachReturnStatement, getParserServices } from '../util';
 
 type ClassLikeDeclaration =
   | TSESTree.ClassDeclaration
@@ -40,13 +40,13 @@ export default createRule({
     function tryGetNameInType(
       name: string,
       typeNode: TSESTree.TypeNode,
-    ): TSESTree.Identifier | undefined {
+    ): TSESTree.TSTypeReference | undefined {
       if (
         typeNode.type === AST_NODE_TYPES.TSTypeReference &&
         typeNode.typeName.type === AST_NODE_TYPES.Identifier &&
         typeNode.typeName.name === name
       ) {
-        return typeNode.typeName;
+        return typeNode;
       }
 
       if (typeNode.type === AST_NODE_TYPES.TSUnionType) {
@@ -130,29 +130,23 @@ export default createRule({
       originalClass: ClassLikeDeclaration,
     ): void {
       const className = originalClass.id?.name;
-      if (!className) {
-        return;
-      }
-
-      if (!originalFunc.returnType) {
+      if (!className || !originalFunc.returnType) {
         return;
       }
 
-      const classNameRef = tryGetNameInType(
+      const node = tryGetNameInType(
         className,
         originalFunc.returnType.typeAnnotation,
       );
-      if (!classNameRef) {
+      if (!node) {
         return;
       }
 
       if (isFunctionReturningThis(originalFunc, originalClass)) {
         context.report({
-          node: classNameRef,
+          node,
           messageId: 'useThisType',
-          fix(fixer) {
-            return fixer.replaceText(classNameRef, 'this');
-          },
+          fix: fixer => fixer.replaceText(node, 'this'),
         });
       }
     }

diff --git a/packages/eslint-plugin/tests/rules/no-require-imports.test.ts b/packages/eslint-plugin/tests/rules/no-require-imports.test.ts
@@ -17,6 +17,11 @@ ruleTester.run('no-require-imports', rule, {
     'import lib9 = lib2.anotherSubImport;',
     "import lib10 from 'lib10';",
     "var lib3 = load?.('not_an_import');",
+    `
+import { createRequire } from 'module';
+const require = createRequire();
+require('remark-preset-prettier');
+    `,
   ],
   invalid: [
     {

diff --git a/packages/eslint-plugin/tests/rules/no-shadow.test.ts b/packages/eslint-plugin/tests/rules/no-shadow.test.ts
@@ -1,6 +1,6 @@
+import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
 import rule from '../../src/rules/no-shadow';
 import { RuleTester } from '../RuleTester';
-import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
 
 const ruleTester = new RuleTester({
   parserOptions: {
@@ -172,6 +172,18 @@ export class Wrapper<Wrapped> {
   }
 }
     `,
+    {
+      // https://github.com/typescript-eslint/typescript-eslint/issues/3862
+      code: `
+import type { foo } from './foo';
+type bar = number;
+
+// 'foo' is already declared in the upper scope
+// 'bar' is fine
+function doThing(foo: number, bar: number) {}
+      `,
+      options: [{ ignoreTypeValueShadow: true }],
+    },
   ],
   invalid: [
     {
@@ -1398,5 +1410,37 @@ function foo(cb) {
         },
       ],
     },
+    {
+      code: `
+import type { foo } from './foo';
+function doThing(foo: number, bar: number) {}
+      `,
+      options: [{ ignoreTypeValueShadow: false }],
+      errors: [
+        {
+          messageId: 'noShadow',
+          data: { name: 'foo' },
+          type: AST_NODE_TYPES.Identifier,
+          line: 3,
+          column: 18,
+        },
+      ],
+    },
+    {
+      code: `
+import { foo } from './foo';
+function doThing(foo: number, bar: number) {}
+      `,
+      options: [{ ignoreTypeValueShadow: true }],
+      errors: [
+        {
+          messageId: 'noShadow',
+          data: { name: 'foo' },
+          type: AST_NODE_TYPES.Identifier,
+          line: 3,
+          column: 18,
+        },
+      ],
+    },
   ],
 });
diff --git a/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts b/packages/eslint-plugin/tests/rules/prefer-regexp-exec.test.ts
@@ -1,5 +1,5 @@
 import rule from '../../src/rules/prefer-regexp-exec';
-import { RuleTester, getFixturesRootDir } from '../RuleTester';
+import { getFixturesRootDir, RuleTester } from '../RuleTester';
 
 const rootPath = getFixturesRootDir();
 
@@ -56,6 +56,24 @@ const matchers = [{ match: (s: RegExp) => false }];
 const file = '';
 matchers.some(matcher => !!file.match(matcher));
     `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/3477
+    `
+function test(pattern: string) {
+  'hello hello'.match(RegExp(pattern, 'g'))?.reduce(() => []);
+}
+    `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/3477
+    `
+function test(pattern: string) {
+  'hello hello'.match(new RegExp(pattern, 'gi'))?.reduce(() => []);
+}
+    `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/3477
+    `
+const matchCount = (str: string, re: RegExp) => {
+  return (str.match(re) || []).length;
+};
+    `,
   ],
   invalid: [
     {
@@ -174,6 +192,44 @@ function f<T extends 'a' | 'b'>(s: T) {
       output: `
 function f<T extends 'a' | 'b'>(s: T) {
   /thing/.exec(s);
+}
+      `,
+    },
+    {
+      code: `
+const text = 'something';
+const search = new RegExp('test', '');
+text.match(search);
+      `,
+      errors: [
+        {
+          messageId: 'regExpExecOverStringMatch',
+          line: 4,
+          column: 6,
+        },
+      ],
+      output: `
+const text = 'something';
+const search = new RegExp('test', '');
+search.exec(text);
+      `,
+    },
+    {
+      code: `
+function test(pattern: string) {
+  'check'.match(new RegExp(pattern, undefined));
+}
+      `,
+      errors: [
+        {
+          messageId: 'regExpExecOverStringMatch',
+          line: 3,
+          column: 11,
+        },
+      ],
+      output: `
+function test(pattern: string) {
+  new RegExp(pattern, undefined).exec('check');
 }
       `,
     },

diff --git a/packages/eslint-plugin/tests/rules/prefer-return-this-type.test.ts b/packages/eslint-plugin/tests/rules/prefer-return-this-type.test.ts
@@ -1,5 +1,5 @@
 import rule from '../../src/rules/prefer-return-this-type';
-import { RuleTester, getFixturesRootDir } from '../RuleTester';
+import { getFixturesRootDir, RuleTester } from '../RuleTester';
 
 const rootPath = getFixturesRootDir();
 
@@ -281,6 +281,33 @@ class Foo {
         return this;
     }
   }
+}
+      `,
+    },
+    {
+      // https://github.com/typescript-eslint/typescript-eslint/issues/3842
+      code: `
+class Animal<T> {
+  eat(): Animal<T> {
+    console.log("I'm moving!");
+    return this;
+  }
+}
+      `,
+      errors: [
+        {
+          messageId: 'useThisType',
+          line: 3,
+          column: 10,
+          endColumn: 19,
+        },
+      ],
+      output: `
+class Animal<T> {
+  eat(): this {
+    console.log("I'm moving!");
+    return this;
+  }
 }
       `,
     },