Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

49 changes: 39 additions & 10 deletions packages/eslint-plugin/src/rules/consistent-type-assertions.ts
Original file line number Diff line number Diff line change
@@ -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 =
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -125,7 +128,6 @@ export default util.createRule<Options, MessageIds>({
if (isConst(node.typeAnnotation) && messageId === 'never') {
return;
}

context.report({
node,
messageId,
Expand All @@ -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,
});
}
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/getOperatorPrecedence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ export function getOperatorPrecedence(
return OperatorPrecedence.Member;

case SyntaxKind.AsExpression:
case SyntaxKind.SatisfiesExpression:
return OperatorPrecedence.Relational;

case SyntaxKind.ThisKeyword:
Expand Down
9 changes: 9 additions & 0 deletions packages/eslint-plugin/src/util/getWrappedCode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { OperatorPrecedence } from './getOperatorPrecedence';

export function getWrappedCode(
text: string,
nodePrecedence: OperatorPrecedence,
parentPrecedence: OperatorPrecedence,
): string {
return nodePrecedence > parentPrecedence ? text : `(${text})`;
}
32 changes: 27 additions & 5 deletions packages/eslint-plugin/src/util/getWrappingFixer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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})`;
}

Expand Down Expand Up @@ -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
);
}

Expand Down Expand Up @@ -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
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
Expand All @@ -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;
Expand Down Expand Up @@ -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>({
Expand Down Expand Up @@ -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,
}),
Expand Down Expand Up @@ -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>({
Expand Down Expand Up @@ -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>({
Expand Down
91 changes: 91 additions & 0 deletions packages/eslint-plugin/tests/util/getWrappedCode.test.ts
Original file line number Diff line number Diff line change
@@ -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" }',
},
],
});