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): [no-unnecessary-template-expression] add missing parentheses in autofix #8673

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
24 changes: 9 additions & 15 deletions packages/eslint-plugin/src/rules/no-useless-template-literals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as ts from 'typescript';
import {
createRule,
getConstrainedTypeAtLocation,
getMovedNodeCode,
getParserServices,
getStaticStringValue,
isTypeFlagSet,
Expand Down Expand Up @@ -92,21 +93,14 @@ export default createRule<[], MessageId>({
context.report({
node: node.expressions[0],
messageId: 'noUselessTemplateLiteral',
fix(fixer): TSESLint.RuleFix[] {
const [prevQuasi, nextQuasi] = node.quasis;

// Remove the quasis and backticks.
return [
fixer.removeRange([
prevQuasi.range[1] - 3,
node.expressions[0].range[0],
]),

fixer.removeRange([
node.expressions[0].range[1],
nextQuasi.range[0] + 2,
]),
];
fix(fixer): TSESLint.RuleFix | null {
const wrappingCode = getMovedNodeCode({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) the name wrappingCode is slightly stale with the rename, but I don't have many good ideas for a better name 🙃. Any ideas? codeToReplaceTemplate? Up to you if you want to change this or leave it 🤷‍♂️

sourceCode: context.sourceCode,
nodeToMove: node.expressions[0],
destinationNode: node,
});

return fixer.replaceText(node, wrappingCode);
},
});

Expand Down
33 changes: 31 additions & 2 deletions packages/eslint-plugin/src/util/getWrappingFixer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,33 @@ export function getWrappingFixer(
return fixer.replaceText(node, code);
};
}
/**
* If the node to be moved and the destination node require parentheses, include parentheses in the node to be moved.
* @param sourceCode Source code of current file
* @param nodeToMove Nodes that need to be moved
* @param destinationNode Final destination node with nodeToMove
* @returns If parentheses are required, code for the nodeToMove node is returned with parentheses at both ends of the code.
*/
export function getMovedNodeCode(params: {
sourceCode: Readonly<TSESLint.SourceCode>;
nodeToMove: TSESTree.Node;
destinationNode: TSESTree.Node;
}): string {
const { sourceCode, nodeToMove: existingNode, destinationNode } = params;
const code = sourceCode.getText(existingNode);
if (isStrongPrecedenceNode(existingNode)) {
// Moved node never needs parens
return code;
}

if (!isWeakPrecedenceParent(destinationNode)) {
// Destination would never needs parens, regardless what node moves there
return code;
}

// Parens may be necessary
return `(${code})`;
}

/**
* Check if a node will always have the same precedence if it's parent changes.
Expand All @@ -98,8 +125,10 @@ export function isStrongPrecedenceNode(innerNode: TSESTree.Node): boolean {
* Check if a node's parent could have different precedence if the node changes.
*/
function isWeakPrecedenceParent(node: TSESTree.Node): boolean {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const parent = node.parent!;
const parent = node.parent;
if (!parent) {
return false;
}

if (
parent.type === AST_NODE_TYPES.UpdateExpression ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,5 +637,14 @@ declare const nested: string, interpolation: string;
},
],
},
{
code: "true ? `${'test' || ''}`.trim() : undefined;",
output: "true ? ('test' || '').trim() : undefined;",
errors: [
{
messageId: 'noUselessTemplateLiteral',
},
],
},
],
});