Merge branch 'master' into 2109-special-case-empty-map-unsafe-assignment

Author: bradzacher

Diff for: packages/eslint-plugin/docs/rules/dot-notation.md

@@ -3,7 +3,10 @@
 ## Rule Details
 
 This rule extends the base [`eslint/dot-notation`](https://eslint.org/docs/rules/dot-notation) rule.
-It adds support for optionally ignoring computed `private` member access.
+It adds:
+
+- Support for optionally ignoring computed `private` and/or `protected` member access.
+- Compatibility with TypeScript's `noPropertyAccessFromIndexSignature` option.
 
 ## How to use
 
@@ -24,14 +27,18 @@ This rule adds the following options:
 interface Options extends BaseDotNotationOptions {
   allowPrivateClassPropertyAccess?: boolean;
   allowProtectedClassPropertyAccess?: boolean;
+  allowIndexSignaturePropertyAccess?: boolean;
 }
 const defaultOptions: Options = {
   ...baseDotNotationDefaultOptions,
   allowPrivateClassPropertyAccess: false,
   allowProtectedClassPropertyAccess: false,
+  allowIndexSignaturePropertyAccess: false,
 };
 ```
 
+If the TypeScript compiler option `noPropertyAccessFromIndexSignature` is set to `true`, then this rule always allows the use of square bracket notation to access properties of types that have a `string` index signature, even if `allowIndexSignaturePropertyAccess` is `false`.
+
 ### `allowPrivateClassPropertyAccess`
 
 Example of a correct code when `allowPrivateClassPropertyAccess` is set to `true`
@@ -58,4 +65,19 @@ const x = new X();
 x['protected_prop'] = 123;
 ```
 
+### `allowIndexSignaturePropertyAccess`
+
+Example of correct code when `allowIndexSignaturePropertyAccess` is set to `true`
+
+```ts
+class X {
+  [key: string]: number;
+}
+
+const x = new X();
+x['hello'] = 123;
+```
+
+If the TypeScript compiler option `noPropertyAccessFromIndexSignature` is set to `true`, then the above code is always allowed, even if `allowIndexSignaturePropertyAccess` is `false`.
+
 <sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/dot-notation.md)</sup>

Diff for: packages/eslint-plugin/docs/rules/prefer-regexp-exec.md

@@ -1,18 +1,16 @@
 # Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided (`prefer-regexp-exec`)
 
-`RegExp#exec` is faster than `String#match` and both work the same when not using the `/g` flag.
+As `String#match` is defined to be the same as `RegExp#exec` when the regular expression does not include the `g` flag, prefer a consistent usage.
 
 ## Rule Details
 
-This rule is aimed at enforcing the more performant way of applying regular expressions on strings.
+This rule is aimed at enforcing a consistent way to apply regular expressions to strings.
 
 From [`String#match` on MDN](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match):
 
 > If the regular expression does not include the g flag, returns the same result as `RegExp.exec()`.
 
-From [Stack Overflow](https://stackoverflow.com/questions/9214754/what-is-the-difference-between-regexp-s-exec-function-and-string-s-match-fun)
-
-> `RegExp.prototype.exec` is a lot faster than `String.prototype.match`, but that’s because they are not exactly the same thing, they are different.
+`RegExp#exec` may also be slightly faster than `String#match`; this is the reason to choose it as the preferred usage.
 
 Examples of **incorrect** code for this rule:
 

Diff for: packages/eslint-plugin/src/rules/dot-notation.ts

@@ -1,11 +1,12 @@
 import { TSESTree } from '@typescript-eslint/experimental-utils';
 import * as ts from 'typescript';
+import * as tsutils from 'tsutils';
 import baseRule from 'eslint/lib/rules/dot-notation';
 import {
-  InferOptionsTypeFromRule,
-  InferMessageIdsTypeFromRule,
   createRule,
   getParserServices,
+  InferMessageIdsTypeFromRule,
+  InferOptionsTypeFromRule,
 } from '../util';
 
 export type Options = InferOptionsTypeFromRule<typeof baseRule>;
@@ -42,6 +43,10 @@ export default createRule<Options, MessageIds>({
             type: 'boolean',
             default: false,
           },
+          allowIndexSignaturePropertyAccess: {
+            type: 'boolean',
+            default: false,
+          },
         },
         additionalProperties: false,
       },
@@ -53,32 +58,41 @@ export default createRule<Options, MessageIds>({
     {
       allowPrivateClassPropertyAccess: false,
       allowProtectedClassPropertyAccess: false,
+      allowIndexSignaturePropertyAccess: false,
       allowKeywords: true,
       allowPattern: '',
     },
   ],
   create(context, [options]) {
     const rules = baseRule.create(context);
+
+    const { program, esTreeNodeToTSNodeMap } = getParserServices(context);
+    const typeChecker = program.getTypeChecker();
+
     const allowPrivateClassPropertyAccess =
       options.allowPrivateClassPropertyAccess;
     const allowProtectedClassPropertyAccess =
       options.allowProtectedClassPropertyAccess;
-
-    const parserServices = getParserServices(context);
-    const typeChecker = parserServices.program.getTypeChecker();
+    const allowIndexSignaturePropertyAccess =
+      (options.allowIndexSignaturePropertyAccess ?? false) ||
+      tsutils.isCompilerOptionEnabled(
+        program.getCompilerOptions(),
+        'noPropertyAccessFromIndexSignature',
+      );
 
     return {
       MemberExpression(node: TSESTree.MemberExpression): void {
         if (
           (allowPrivateClassPropertyAccess ||
-            allowProtectedClassPropertyAccess) &&
+            allowProtectedClassPropertyAccess ||
+            allowIndexSignaturePropertyAccess) &&
           node.computed
         ) {
-          // for perf reasons - only fetch the symbol if we have to
-          const objectSymbol = typeChecker.getSymbolAtLocation(
-            parserServices.esTreeNodeToTSNodeMap.get(node.property),
+          // for perf reasons - only fetch symbols if we have to
+          const propertySymbol = typeChecker.getSymbolAtLocation(
+            esTreeNodeToTSNodeMap.get(node.property),
           );
-          const modifierKind = objectSymbol?.getDeclarations()?.[0]
+          const modifierKind = propertySymbol?.getDeclarations()?.[0]
             ?.modifiers?.[0].kind;
           if (
             (allowPrivateClassPropertyAccess &&
@@ -88,6 +102,21 @@ export default createRule<Options, MessageIds>({
           ) {
             return;
           }
+          if (
+            propertySymbol === undefined &&
+            allowIndexSignaturePropertyAccess
+          ) {
+            const objectType = typeChecker.getTypeAtLocation(
+              esTreeNodeToTSNodeMap.get(node.object),
+            );
+            const indexType = typeChecker.getIndexTypeOfType(
+              objectType,
+              ts.IndexKind.String,
+            );
+            if (indexType != undefined) {
+              return;
+            }
+          }
         }
         rules.MemberExpression(node);
       },

Diff for: packages/eslint-plugin/src/rules/no-shadow.ts

@@ -126,6 +126,84 @@ export default util.createRule<Options, MessageIds>({
       return util.isFunctionType(id.parent);
     }
 
+    function isGenericOfStaticMethod(
+      variable: TSESLint.Scope.Variable,
+    ): boolean {
+      if (!('isTypeVariable' in variable)) {
+        // this shouldn't happen...
+        return false;
+      }
+
+      if (!variable.isTypeVariable) {
+        return false;
+      }
+
+      if (variable.identifiers.length === 0) {
+        return false;
+      }
+
+      const typeParameter = variable.identifiers[0].parent;
+      if (typeParameter?.type !== AST_NODE_TYPES.TSTypeParameter) {
+        return false;
+      }
+      const typeParameterDecl = typeParameter.parent;
+      if (
+        typeParameterDecl?.type !== AST_NODE_TYPES.TSTypeParameterDeclaration
+      ) {
+        return false;
+      }
+      const functionExpr = typeParameterDecl.parent;
+      if (
+        !functionExpr ||
+        (functionExpr.type !== AST_NODE_TYPES.FunctionExpression &&
+          functionExpr.type !== AST_NODE_TYPES.TSEmptyBodyFunctionExpression)
+      ) {
+        return false;
+      }
+      const methodDefinition = functionExpr.parent;
+      if (methodDefinition?.type !== AST_NODE_TYPES.MethodDefinition) {
+        return false;
+      }
+      return methodDefinition.static;
+    }
+
+    function isGenericOfClassDecl(variable: TSESLint.Scope.Variable): boolean {
+      if (!('isTypeVariable' in variable)) {
+        // this shouldn't happen...
+        return false;
+      }
+
+      if (!variable.isTypeVariable) {
+        return false;
+      }
+
+      if (variable.identifiers.length === 0) {
+        return false;
+      }
+
+      const typeParameter = variable.identifiers[0].parent;
+      if (typeParameter?.type !== AST_NODE_TYPES.TSTypeParameter) {
+        return false;
+      }
+      const typeParameterDecl = typeParameter.parent;
+      if (
+        typeParameterDecl?.type !== AST_NODE_TYPES.TSTypeParameterDeclaration
+      ) {
+        return false;
+      }
+      const classDecl = typeParameterDecl.parent;
+      return classDecl?.type === AST_NODE_TYPES.ClassDeclaration;
+    }
+
+    function isGenericOfAStaticMethodShadow(
+      variable: TSESLint.Scope.Variable,
+      shadowed: TSESLint.Scope.Variable,
+    ): boolean {
+      return (
+        isGenericOfStaticMethod(variable) && isGenericOfClassDecl(shadowed)
+      );
+    }
+
     /**
      * Check if variable name is allowed.
      * @param variable The variable to check.
@@ -321,6 +399,13 @@ export default util.createRule<Options, MessageIds>({
           continue;
         }
 
+        // ignore static class method generic shadowing class generic
+        // this is impossible for the scope analyser to understand
+        // so we have to handle this manually in this rule
+        if (isGenericOfAStaticMethodShadow(variable, shadowed)) {
+          continue;
+        }
+
         const isESLintGlobal = 'writeable' in shadowed;
         if (
           (shadowed.identifiers.length > 0 ||

Diff for: packages/eslint-plugin/tests/rules/dot-notation.test.ts

@@ -87,6 +87,18 @@ x['protected_prop'] = 123;
       `,
       options: [{ allowProtectedClassPropertyAccess: true }],
     },
+    {
+      code: `
+class X {
+  prop: string;
+  [key: string]: number;
+}
+
+const x = new X();
+x['hello'] = 3;
+      `,
+      options: [{ allowIndexSignaturePropertyAccess: true }],
+    },
   ],
   invalid: [
     {
@@ -287,5 +299,27 @@ x.protected_prop = 123;
       `,
       errors: [{ messageId: 'useDot' }],
     },
+    {
+      code: `
+class X {
+  prop: string;
+  [key: string]: number;
+}
+
+const x = new X();
+x['prop'] = 'hello';
+      `,
+      options: [{ allowIndexSignaturePropertyAccess: true }],
+      errors: [{ messageId: 'useDot' }],
+      output: `
+class X {
+  prop: string;
+  [key: string]: number;
+}
+
+const x = new X();
+x.prop = 'hello';
+      `,
+    },
   ],
 });

Diff for: packages/eslint-plugin/tests/rules/no-shadow.test.ts

@@ -159,6 +159,19 @@ type Fn = (Foo: string) => typeof Foo;
       `,
       options: [{ ignoreFunctionTypeParameterNameValueShadow: false }],
     },
+    `
+export class Wrapper<Wrapped> {
+  private constructor(private readonly wrapped: Wrapped) {}
+
+  unwrap(): Wrapped {
+    return this.wrapped;
+  }
+
+  static create<Wrapped>(wrapped: Wrapped) {
+    return new Wrapper<Wrapped>(wrapped);
+  }
+}
+    `,
   ],
   invalid: [
     {

Diff for: packages/eslint-plugin/typings/eslint-rules.d.ts

@@ -713,6 +713,7 @@ declare module 'eslint/lib/rules/dot-notation' {
         allowPattern?: string;
         allowPrivateClassPropertyAccess?: boolean;
         allowProtectedClassPropertyAccess?: boolean;
+        allowIndexSignaturePropertyAccess?: boolean;
       },
     ],
     {