fix(eslint-plugin): Support more nodes [no-extra-parens] (#465)

packages/eslint-plugin-tslint/package.json

@@ -19,10 +19,11 @@
   },
   "license": "MIT",
   "scripts": {
-    "test": "jest --coverage",
-    "prebuild": "npm run clean",
     "build": "tsc -p tsconfig.build.json",
     "clean": "rimraf dist/",
+    "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
+    "prebuild": "npm run clean",
+    "test": "jest --coverage",
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {

packages/eslint-plugin/package.json

@@ -25,13 +25,14 @@
   "license": "MIT",
   "main": "dist/index.js",
   "scripts": {
+    "build": "tsc -p tsconfig.build.json",
+    "clean": "rimraf dist/",
     "docs": "eslint-docs",
     "docs:check": "eslint-docs check",
-    "test": "jest --coverage",
-    "recommended:update": "ts-node tools/update-recommended.ts",
+    "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
     "prebuild": "npm run clean",
-    "build": "tsc -p tsconfig.build.json",
-    "clean": "rimraf dist/",
+    "recommended:update": "ts-node tools/update-recommended.ts",
+    "test": "jest --coverage",
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {

packages/eslint-plugin/src/rules/no-extra-parens.ts

@@ -1,4 +1,4 @@
-import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/typescript-estree';
+import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree';
 import baseRule from 'eslint/lib/rules/no-extra-parens';
 import * as util from '../util';
 
@@ -22,10 +22,204 @@ export default util.createRule<Options, MessageIds>({
   create(context) {
     const rules = baseRule.create(context);
 
+    function binaryExp(
+      node: TSESTree.BinaryExpression | TSESTree.LogicalExpression,
+    ) {
+      const rule = rules.BinaryExpression as (n: typeof node) => void;
+
+      // makes the rule think it should skip the left or right
+      if (node.left.type === AST_NODE_TYPES.TSAsExpression) {
+        return rule({
+          ...node,
+          left: {
+            ...node.left,
+            type: AST_NODE_TYPES.BinaryExpression as any,
+          },
+        });
+      }
+      if (node.right.type === AST_NODE_TYPES.TSAsExpression) {
+        return rule({
+          ...node,
+          right: {
+            ...node.right,
+            type: AST_NODE_TYPES.BinaryExpression as any,
+          },
+        });
+      }
+
+      return rule(node);
+    }
+    function callExp(node: TSESTree.CallExpression | TSESTree.NewExpression) {
+      const rule = rules.CallExpression as (n: typeof node) => void;
+
+      if (node.callee.type === AST_NODE_TYPES.TSAsExpression) {
+        // reduces the precedence of the node so the rule thinks it needs to be wrapped
+        return rule({
+          ...node,
+          callee: {
+            ...node.callee,
+            type: AST_NODE_TYPES.SequenceExpression as any,
+          },
+        });
+      }
+
+      return rule(node);
+    }
+    function unaryUpdateExpression(
+      node: TSESTree.UnaryExpression | TSESTree.UpdateExpression,
+    ) {
+      const rule = rules.UnaryExpression as (n: typeof node) => void;
+
+      if (node.argument.type === AST_NODE_TYPES.TSAsExpression) {
+        // reduces the precedence of the node so the rule thinks it needs to be wrapped
+        return rule({
+          ...node,
+          argument: {
+            ...node.argument,
+            type: AST_NODE_TYPES.SequenceExpression as any,
+          },
+        });
+      }
+
+      return rule(node);
+    }
+
     return Object.assign({}, rules, {
-      MemberExpression(node: TSESTree.MemberExpression) {
-        if (node.object.type !== AST_NODE_TYPES.TSAsExpression) {
-          return rules.MemberExpression(node);
+      // ArrayExpression
+      ArrowFunctionExpression(node) {
+        if (node.body.type !== AST_NODE_TYPES.TSAsExpression) {
+          return rules.ArrowFunctionExpression(node);
+        }
+      },
+      // AssignmentExpression
+      // AwaitExpression
+      BinaryExpression: binaryExp,
+      CallExpression: callExp,
+      // ClassDeclaration
+      // ClassExpression
+      ConditionalExpression(node) {
+        // reduces the precedence of the node so the rule thinks it needs to be wrapped
+        if (node.test.type === AST_NODE_TYPES.TSAsExpression) {
+          return rules.ConditionalExpression({
+            ...node,
+            test: {
+              ...node.test,
+              type: AST_NODE_TYPES.SequenceExpression as any,
+            },
+          });
+        }
+        if (node.consequent.type === AST_NODE_TYPES.TSAsExpression) {
+          return rules.ConditionalExpression({
+            ...node,
+            consequent: {
+              ...node.consequent,
+              type: AST_NODE_TYPES.SequenceExpression as any,
+            },
+          });
+        }
+        if (node.alternate.type === AST_NODE_TYPES.TSAsExpression) {
+          // reduces the precedence of the node so the rule thinks it needs to be rapped
+          return rules.ConditionalExpression({
+            ...node,
+            alternate: {
+              ...node.alternate,
+              type: AST_NODE_TYPES.SequenceExpression as any,
+            },
+          });
+        }
+        return rules.ConditionalExpression(node);
+      },
+      // DoWhileStatement
+      'ForInStatement, ForOfStatement'(
+        node: TSESTree.ForInStatement | TSESTree.ForOfStatement,
+      ) {
+        if (node.right.type === AST_NODE_TYPES.TSAsExpression) {
+          // makes the rule skip checking of the right
+          return rules['ForInStatement, ForOfStatement']({
+            ...node,
+            type: AST_NODE_TYPES.ForOfStatement as any,
+            right: {
+              ...node.right,
+              type: AST_NODE_TYPES.SequenceExpression as any,
+            },
+          });
+        }
+
+        return rules['ForInStatement, ForOfStatement'](node);
+      },
+      ForStatement(node) {
+        // make the rule skip the piece by removing it entirely
+        if (node.init && node.init.type === AST_NODE_TYPES.TSAsExpression) {
+          return rules.ForStatement({
+            ...node,
+            init: null,
+          });
+        }
+        if (node.test && node.test.type === AST_NODE_TYPES.TSAsExpression) {
+          return rules.ForStatement({
+            ...node,
+            test: null,
+          });
+        }
+        if (node.update && node.update.type === AST_NODE_TYPES.TSAsExpression) {
+          return rules.ForStatement({
+            ...node,
+            update: null,
+          });
+        }
+
+        return rules.ForStatement(node);
+      },
+      // IfStatement
+      LogicalExpression: binaryExp,
+      MemberExpression(node) {
+        if (node.object.type === AST_NODE_TYPES.TSAsExpression) {
+          // reduces the precedence of the node so the rule thinks it needs to be wrapped
+          return rules.MemberExpression({
+            ...node,
+            object: {
+              ...node.object,
+              type: AST_NODE_TYPES.SequenceExpression as any,
+            },
+          });
+        }
+
+        return rules.MemberExpression(node);
+      },
+      NewExpression: callExp,
+      // ObjectExpression
+      // ReturnStatement
+      // SequenceExpression
+      SpreadElement(node) {
+        if (node.argument.type !== AST_NODE_TYPES.TSAsExpression) {
+          return rules.SpreadElement(node);
+        }
+      },
+      SwitchCase(node) {
+        if (node.test.type !== AST_NODE_TYPES.TSAsExpression) {
+          return rules.SwitchCase(node);
+        }
+      },
+      // SwitchStatement
+      ThrowStatement(node) {
+        if (
+          node.argument &&
+          node.argument.type !== AST_NODE_TYPES.TSAsExpression
+        ) {
+          return rules.ThrowStatement(node);
+        }
+      },
+      UnaryExpression: unaryUpdateExpression,
+      UpdateExpression: unaryUpdateExpression,
+      // VariableDeclarator
+      // WhileStatement
+      // WithStatement - i'm not going to even bother implementing this terrible and never used feature
+      YieldExpression(node) {
+        if (
+          node.argument &&
+          node.argument.type !== AST_NODE_TYPES.TSAsExpression
+        ) {
+          return rules.YieldExpression(node);
         }
       },
     });

packages/eslint-plugin/tests/RuleTester.ts

@@ -61,11 +61,68 @@ function getFixturesRootDir() {
   return path.join(process.cwd(), 'tests/fixtures/');
 }
 
+/**
+ * Converts a batch of single line tests into a number of separate test cases.
+ * This makes it easier to write tests which use the same options.
+ *
+ * Why wouldn't you just leave them as one test?
+ * Because it makes the test error messages harder to decipher.
+ * This way each line will fail separately, instead of them all failing together.
+ */
+function batchedSingleLineTests<TOptions extends Readonly<any[]>>(
+  test: ValidTestCase<TOptions>,
+): ValidTestCase<TOptions>[];
+/**
+ * Converts a batch of single line tests into a number of separate test cases.
+ * This makes it easier to write tests which use the same options.
+ *
+ * Why wouldn't you just leave them as one test?
+ * Because it makes the test error messages harder to decipher.
+ * This way each line will fail separately, instead of them all failing together.
+ *
+ * Make sure you have your line numbers correct for error reporting, as it will match
+ * the line numbers up with the split tests!
+ */
+function batchedSingleLineTests<
+  TMessageIds extends string,
+  TOptions extends Readonly<any[]>
+>(
+  test: InvalidTestCase<TMessageIds, TOptions>,
+): InvalidTestCase<TMessageIds, TOptions>[];
+function batchedSingleLineTests<
+  TMessageIds extends string,
+  TOptions extends Readonly<any[]>
+>(
+  options: ValidTestCase<TOptions> | InvalidTestCase<TMessageIds, TOptions>,
+): (ValidTestCase<TOptions> | InvalidTestCase<TMessageIds, TOptions>)[] {
+  // eslint counts lines from 1
+  const lineOffset = options.code[0] === '\n' ? 2 : 1;
+  return options.code
+    .trim()
+    .split('\n')
+    .map((code, i) => {
+      const lineNum = i + lineOffset;
+      const errors =
+        'errors' in options
+          ? options.errors.filter(e => e.line === lineNum)
+          : [];
+      return {
+        ...options,
+        code,
+        errors: errors.map(e => ({
+          ...e,
+          line: 1,
+        })),
+      };
+    });
+}
+
 export {
   RuleTester,
   RunTests,
   TestCaseError,
   InvalidTestCase,
   ValidTestCase,
+  batchedSingleLineTests,
   getFixturesRootDir,
 };