fix(eslint-plugin): [consistent-type-assertions] wrap object return v…

…alue with parentheses (#6885)

* fix(eslint-plugin): [consistent-type-assertions] wrap object return value with parentheses

* use getWrappingFixer

* fix

* fix

* fix

* fix

* update getOperatorPrecedence

* use precedence to wrap node in parens

* Fix up post-merge

* lint --fix

* remove comment

* Remove post-merge artifact

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

packages/eslint-plugin/src/rules/consistent-type-assertions.ts

@@ -1,7 +1,9 @@
 import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
+import * as ts from 'typescript';
 
 import * as util from '../util';
+import { getWrappedCode } from '../util/getWrappedCode';
 
 // intentionally mirroring the options
 export type MessageIds =
@@ -82,6 +84,7 @@ export default util.createRule<Options, MessageIds>({
   ],
   create(context, [options]) {
     const sourceCode = context.getSourceCode();
+    const parserServices = util.getParserServices(context, true);
 
     function isConst(node: TSESTree.TypeNode): boolean {
       if (node.type !== AST_NODE_TYPES.TSTypeReference) {
@@ -125,7 +128,6 @@ export default util.createRule<Options, MessageIds>({
       if (isConst(node.typeAnnotation) && messageId === 'never') {
         return;
       }
-
       context.report({
         node,
         messageId,
@@ -135,16 +137,43 @@ export default util.createRule<Options, MessageIds>({
             : {},
         fix:
           messageId === 'as'
-            ? (fixer): TSESLint.RuleFix[] => [
-                fixer.replaceText(
-                  node,
-                  getTextWithParentheses(node.expression),
-                ),
-                fixer.insertTextAfter(
+            ? (fixer): TSESLint.RuleFix => {
+                const tsNode = parserServices.esTreeNodeToTSNodeMap.get(
+                  node as TSESTree.TSTypeAssertion,
+                );
+
+                /**
+                 * AsExpression has lower precedence than TypeAssertionExpression,
+                 * so we don't need to wrap expression and typeAnnotation in parens.
+                 */
+                const expressionCode = sourceCode.getText(node.expression);
+                const typeAnnotationCode = sourceCode.getText(
+                  node.typeAnnotation,
+                );
+
+                const asPrecedence = util.getOperatorPrecedence(
+                  ts.SyntaxKind.AsExpression,
+                  ts.SyntaxKind.Unknown,
+                );
+                const parentPrecedence = util.getOperatorPrecedence(
+                  tsNode.parent.kind,
+                  ts.isBinaryExpression(tsNode.parent)
+                    ? tsNode.parent.operatorToken.kind
+                    : ts.SyntaxKind.Unknown,
+                  ts.isNewExpression(tsNode.parent)
+                    ? tsNode.parent.arguments != null &&
+                        tsNode.parent.arguments.length > 0
+                    : undefined,
+                );
+
+                const text = `${expressionCode} as ${typeAnnotationCode}`;
+                return fixer.replaceText(
                   node,
-                  ` as ${getTextWithParentheses(node.typeAnnotation)}`,
-                ),
-              ]
+                  util.isParenthesized(node, sourceCode)
+                    ? text
+                    : getWrappedCode(text, asPrecedence, parentPrecedence),
+                );
+              }
             : undefined,
       });
     }

packages/eslint-plugin/src/util/getOperatorPrecedence.ts

@@ -370,6 +370,7 @@ export function getOperatorPrecedence(
       return OperatorPrecedence.Member;
 
     case SyntaxKind.AsExpression:
+    case SyntaxKind.SatisfiesExpression:
       return OperatorPrecedence.Relational;
 
     case SyntaxKind.ThisKeyword:

packages/eslint-plugin/src/util/getWrappedCode.ts

@@ -0,0 +1,9 @@
+import type { OperatorPrecedence } from './getOperatorPrecedence';
+
+export function getWrappedCode(
+  text: string,
+  nodePrecedence: OperatorPrecedence,
+  parentPrecedence: OperatorPrecedence,
+): string {
+  return nodePrecedence > parentPrecedence ? text : `(${text})`;
+}

packages/eslint-plugin/src/util/getWrappingFixer.ts

@@ -35,10 +35,15 @@ export function getWrappingFixer(
     const innerCodes = innerNodes.map(innerNode => {
       let code = sourceCode.getText(innerNode);
 
-      // check the inner expression's precedence
-      if (!isStrongPrecedenceNode(innerNode)) {
-        // the code we are adding might have stronger precedence than our wrapped node
-        // let's wrap our node in parens in case it has a weaker precedence than the code we are wrapping it in
+      /**
+       * Wrap our node in parens to prevent the following cases:
+       * - It has a weaker precedence than the code we are wrapping it in
+       * - It's gotten mistaken as block statement instead of object expression
+       */
+      if (
+        !isStrongPrecedenceNode(innerNode) ||
+        isObjectExpressionInOneLineReturn(node, innerNode)
+      ) {
         code = `(${code})`;
       }
 
@@ -73,12 +78,15 @@ export function isStrongPrecedenceNode(innerNode: TSESTree.Node): boolean {
   return (
     innerNode.type === AST_NODE_TYPES.Literal ||
     innerNode.type === AST_NODE_TYPES.Identifier ||
+    innerNode.type === AST_NODE_TYPES.TSTypeReference ||
+    innerNode.type === AST_NODE_TYPES.TSTypeOperator ||
     innerNode.type === AST_NODE_TYPES.ArrayExpression ||
     innerNode.type === AST_NODE_TYPES.ObjectExpression ||
     innerNode.type === AST_NODE_TYPES.MemberExpression ||
     innerNode.type === AST_NODE_TYPES.CallExpression ||
     innerNode.type === AST_NODE_TYPES.NewExpression ||
-    innerNode.type === AST_NODE_TYPES.TaggedTemplateExpression
+    innerNode.type === AST_NODE_TYPES.TaggedTemplateExpression ||
+    innerNode.type === AST_NODE_TYPES.TSInstantiationExpression
   );
 }
 
@@ -205,3 +213,17 @@ function isLeftHandSide(node: TSESTree.Node): boolean {
 
   return false;
 }
+
+/**
+ * Checks if a node's parent is arrow function expression and a inner node is object expression
+ */
+function isObjectExpressionInOneLineReturn(
+  node: TSESTree.Node,
+  innerNode: TSESTree.Node,
+): boolean {
+  return (
+    node.parent?.type === AST_NODE_TYPES.ArrowFunctionExpression &&
+    node.parent.body === node &&
+    innerNode.type === AST_NODE_TYPES.ObjectExpression
+  );
+}

packages/eslint-plugin/tests/rules/consistent-type-assertions.test.ts

@@ -22,7 +22,11 @@ const x = <A>!'string';
 const x = <A>a + b;
 const x = <(A)>a + (b);
 const x = <Foo>(new Generic<string>());
-const x = (new (<Foo>Generic<string>)());`;
+const x = new (<Foo>Generic<string>)();
+const x = new (<Foo>Generic<string>)('string');
+const x = () => <Foo>{ bar: 5 };
+const x = () => <Foo>({ bar: 5 });
+const x = () => <Foo>bar;`;
 
 const ANGLE_BRACKET_TESTS = `${ANGLE_BRACKET_TESTS_EXCEPT_CONST_CASE}
 const x = <const>{ key: 'value' };
@@ -32,12 +36,16 @@ const AS_TESTS_EXCEPT_CONST_CASE = `
 const x = new Generic<int>() as Foo;
 const x = b as A;
 const x = [1] as readonly number[];
-const x = ('string') as a | b;
+const x = 'string' as a | b;
 const x = !'string' as A;
-const x = a as A + b;
-const x = a as (A) + (b);
-const x = (new Generic<string>()) as Foo;
-const x = (new (Generic<string> as Foo)());`;
+const x = (a as A) + b;
+const x = (a as A) + (b);
+const x = new Generic<string>() as Foo;
+const x = new (Generic<string> as Foo)();
+const x = new (Generic<string> as Foo)('string');
+const x = () => ({ bar: 5 } as Foo);
+const x = () => ({ bar: 5 } as Foo);
+const x = () => (bar as Foo);`;
 
 const AS_TESTS = `${AS_TESTS_EXCEPT_CONST_CASE}
 const x = { key: 'value' } as const;
@@ -206,6 +214,22 @@ ruleTester.run('consistent-type-assertions', rule, {
           messageId: 'angle-bracket',
           line: 11,
         },
+        {
+          messageId: 'angle-bracket',
+          line: 12,
+        },
+        {
+          messageId: 'angle-bracket',
+          line: 13,
+        },
+        {
+          messageId: 'angle-bracket',
+          line: 14,
+        },
+        {
+          messageId: 'angle-bracket',
+          line: 15,
+        },
       ],
     }),
     ...batchedSingleLineTests<MessageIds, Options>({
@@ -256,6 +280,22 @@ ruleTester.run('consistent-type-assertions', rule, {
           messageId: 'as',
           line: 11,
         },
+        {
+          messageId: 'as',
+          line: 12,
+        },
+        {
+          messageId: 'as',
+          line: 13,
+        },
+        {
+          messageId: 'as',
+          line: 14,
+        },
+        {
+          messageId: 'as',
+          line: 15,
+        },
       ],
       output: AS_TESTS,
     }),
@@ -303,6 +343,22 @@ ruleTester.run('consistent-type-assertions', rule, {
           messageId: 'never',
           line: 10,
         },
+        {
+          messageId: 'never',
+          line: 11,
+        },
+        {
+          messageId: 'never',
+          line: 12,
+        },
+        {
+          messageId: 'never',
+          line: 13,
+        },
+        {
+          messageId: 'never',
+          line: 14,
+        },
       ],
     }),
     ...batchedSingleLineTests<MessageIds, Options>({
@@ -349,6 +405,22 @@ ruleTester.run('consistent-type-assertions', rule, {
           messageId: 'never',
           line: 10,
         },
+        {
+          messageId: 'never',
+          line: 11,
+        },
+        {
+          messageId: 'never',
+          line: 12,
+        },
+        {
+          messageId: 'never',
+          line: 13,
+        },
+        {
+          messageId: 'never',
+          line: 14,
+        },
       ],
     }),
     ...batchedSingleLineTests<MessageIds, Options>({

packages/eslint-plugin/tests/util/getWrappedCode.test.ts

@@ -0,0 +1,91 @@
+import { RuleTester } from '@typescript-eslint/rule-tester';
+import type { TSESTree } from '@typescript-eslint/utils';
+import * as ts from 'typescript';
+
+import * as util from '../../src/util';
+import { getWrappedCode } from '../../src/util/getWrappedCode';
+import { getFixturesRootDir } from '../RuleTester';
+
+const rootPath = getFixturesRootDir();
+const ruleTester = new RuleTester({
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    tsconfigRootDir: rootPath,
+    project: './tsconfig.json',
+  },
+});
+
+const removeFunctionRule = util.createRule({
+  name: 'remove-function',
+  defaultOptions: [],
+  meta: {
+    type: 'suggestion',
+    fixable: 'code',
+    docs: {
+      description:
+        'Remove function with first arg remaining in random places for test purposes.',
+    },
+    messages: {
+      removeFunction: 'Please remove this function',
+    },
+    schema: [],
+  },
+
+  create(context) {
+    const sourceCode = context.getSourceCode();
+    const parserServices = util.getParserServices(context, true);
+
+    const report = (node: TSESTree.CallExpression): void => {
+      context.report({
+        node,
+        messageId: 'removeFunction',
+        fix: fixer => {
+          const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
+          const tsArgumentNode = tsNode.arguments[0];
+
+          const nodePrecedence = util.getOperatorPrecedence(
+            tsArgumentNode.kind,
+            ts.isBinaryExpression(tsArgumentNode)
+              ? tsArgumentNode.operatorToken.kind
+              : ts.SyntaxKind.Unknown,
+          );
+          const parentPrecedence = util.getOperatorPrecedence(
+            tsNode.parent.kind,
+            ts.isBinaryExpression(tsNode.parent)
+              ? tsNode.parent.operatorToken.kind
+              : ts.SyntaxKind.Unknown,
+          );
+
+          const text = sourceCode.getText(node.arguments[0]);
+          return fixer.replaceText(
+            node,
+            getWrappedCode(text, nodePrecedence, parentPrecedence),
+          );
+        },
+      });
+    };
+
+    return {
+      'CallExpression[callee.name="fn"]': report,
+    };
+  },
+});
+
+ruleTester.run('getWrappedCode - removeFunctionRule', removeFunctionRule, {
+  valid: [],
+  invalid: [
+    // should add parens when the first argument node has lower precedence than the parent node of the CallExpression
+    {
+      code: '() => fn({ x: "wrapObject" })',
+      errors: [{ messageId: 'removeFunction' }],
+      output: '() => ({ x: "wrapObject" })',
+    },
+
+    // shouldn't add parens when not necessary
+    {
+      code: 'const a = fn({ x: "wrapObject" })',
+      errors: [{ messageId: 'removeFunction' }],
+      output: 'const a = { x: "wrapObject" }',
+    },
+  ],
+});