Skip to content

Commit 7ad8bdf

Browse files
JamieMasonJamieMason
committed
feat: preserve parentheses needed due to operator precedence
Closes #37
1 parent 2a5f613 commit 7ad8bdf

File tree

2 files changed

+241
-1
lines changed

2 files changed

+241
-1
lines changed

‎src/writer.ts‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,77 @@ export class Writer {
5555
const BODY = fn.body;
5656
const RETURN_TYPE = fn.returnType ? fn.returnType : '';
5757
const PARAMS = fn.params.join(', ');
58-
return `${ASYNC}${GENERIC}(${PARAMS})${RETURN_TYPE} => ${BODY}`;
58+
const arrowFunction = `${ASYNC}${GENERIC}(${PARAMS})${RETURN_TYPE} => ${BODY}`;
59+
60+
// Check if parentheses are needed due to operator precedence
61+
if (this.needsParentheses(node)) {
62+
return `(${arrowFunction})`;
63+
}
64+
65+
return arrowFunction;
5966
}
6067

6168
writeArrowConstant(node: TSESTree.FunctionDeclaration): string {
6269
const fn = this.getFunctionDescriptor(node);
6370
return `const ${fn.name} = ${this.writeArrowFunction(node)}`;
6471
}
6572

73+
needsParentheses(node: AnyFunction): boolean {
74+
const parent = node.parent;
75+
76+
if (!parent) return false;
77+
78+
// If the function is the right operand of a binary expression or logical expression
79+
if ((parent.type === AST_NODE_TYPES.BinaryExpression || parent.type === AST_NODE_TYPES.LogicalExpression) &&
80+
parent.right === node) {
81+
return true;
82+
}
83+
84+
// If the function is the left operand of most binary expressions (except assignment-like)
85+
if ((parent.type === AST_NODE_TYPES.BinaryExpression || parent.type === AST_NODE_TYPES.LogicalExpression) &&
86+
parent.left === node) {
87+
// Don't add parentheses for assignment-like operators
88+
if (parent.operator === 'in' || parent.operator === 'instanceof') {
89+
return true;
90+
}
91+
// For other operators, we typically need parentheses on the left side too
92+
return ![AST_NODE_TYPES.AssignmentExpression].includes(parent.type);
93+
}
94+
95+
// If the function is the test of a conditional expression (ternary ? part)
96+
if (parent.type === AST_NODE_TYPES.ConditionalExpression && parent.test === node) {
97+
return true;
98+
}
99+
100+
// If the function is the consequent of a conditional expression (ternary middle part)
101+
// This doesn't need parentheses according to issue #37
102+
if (parent.type === AST_NODE_TYPES.ConditionalExpression && parent.consequent === node) {
103+
return false;
104+
}
105+
106+
// Don't add parentheses for these contexts (as mentioned in issue #37):
107+
// - Right side of assignment =, +=, -=, etc.
108+
// - Right side of arrow function =>
109+
// - Right side of ternary :
110+
// - yield, yield*
111+
// - spread ...
112+
// - comma operator
113+
if (parent.type === AST_NODE_TYPES.AssignmentExpression ||
114+
parent.type === AST_NODE_TYPES.ArrowFunctionExpression ||
115+
parent.type === AST_NODE_TYPES.YieldExpression ||
116+
parent.type === AST_NODE_TYPES.SpreadElement ||
117+
parent.type === AST_NODE_TYPES.SequenceExpression) {
118+
return false;
119+
}
120+
121+
// Right side of ternary doesn't need parentheses
122+
if (parent.type === AST_NODE_TYPES.ConditionalExpression && parent.alternate === node) {
123+
return false;
124+
}
125+
126+
return false;
127+
}
128+
66129
getFunctionDescriptor(node: AnyFunction) {
67130
return {
68131
body: this.getBodySource(node),

‎test/rule.spec.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,3 +624,180 @@ const obj = {
624624
});
625625
});
626626
});
627+
628+
describe('issue #37 - arrow function precedence in operator expressions', () => {
629+
describe('when function expressions need parentheses due to operator precedence', () => {
630+
ruleTester.run('prefer-arrow-functions', rule, {
631+
valid: [],
632+
invalid: [
633+
// Cases that should be transformed but need parentheses for correct precedence
634+
{
635+
code: `const result = f || function() { return 'default'; };`,
636+
output: `const result = f || (() => 'default');`,
637+
},
638+
{
639+
code: `const result = f && function() { return 'value'; };`,
640+
output: `const result = f && (() => 'value');`,
641+
},
642+
{
643+
code: `const result = f + function() { return 1; };`,
644+
output: `const result = f + (() => 1);`,
645+
},
646+
{
647+
code: `const result = f - function() { return 1; };`,
648+
output: `const result = f - (() => 1);`,
649+
},
650+
{
651+
code: `const result = f * function() { return 2; };`,
652+
output: `const result = f * (() => 2);`,
653+
},
654+
{
655+
code: `const result = f / function() { return 2; };`,
656+
output: `const result = f / (() => 2);`,
657+
},
658+
{
659+
code: `const result = f % function() { return 3; };`,
660+
output: `const result = f % (() => 3);`,
661+
},
662+
{
663+
code: `const result = f ** function() { return 2; };`,
664+
output: `const result = f ** (() => 2);`,
665+
},
666+
{
667+
code: `const result = f | function() { return 1; };`,
668+
output: `const result = f | (() => 1);`,
669+
},
670+
{
671+
code: `const result = f & function() { return 1; };`,
672+
output: `const result = f & (() => 1);`,
673+
},
674+
{
675+
code: `const result = f ^ function() { return 1; };`,
676+
output: `const result = f ^ (() => 1);`,
677+
},
678+
{
679+
code: `const result = f << function() { return 1; };`,
680+
output: `const result = f << (() => 1);`,
681+
},
682+
{
683+
code: `const result = f >> function() { return 1; };`,
684+
output: `const result = f >> (() => 1);`,
685+
},
686+
{
687+
code: `const result = f >>> function() { return 1; };`,
688+
output: `const result = f >>> (() => 1);`,
689+
},
690+
{
691+
code: `const result = f < function() { return 1; };`,
692+
output: `const result = f < (() => 1);`,
693+
},
694+
{
695+
code: `const result = f > function() { return 1; };`,
696+
output: `const result = f > (() => 1);`,
697+
},
698+
{
699+
code: `const result = f <= function() { return 1; };`,
700+
output: `const result = f <= (() => 1);`,
701+
},
702+
{
703+
code: `const result = f >= function() { return 1; };`,
704+
output: `const result = f >= (() => 1);`,
705+
},
706+
{
707+
code: `const result = f == function() { return 1; };`,
708+
output: `const result = f == (() => 1);`,
709+
},
710+
{
711+
code: `const result = f != function() { return 1; };`,
712+
output: `const result = f != (() => 1);`,
713+
},
714+
{
715+
code: `const result = f === function() { return 1; };`,
716+
output: `const result = f === (() => 1);`,
717+
},
718+
{
719+
code: `const result = f !== function() { return 1; };`,
720+
output: `const result = f !== (() => 1);`,
721+
},
722+
{
723+
code: `const result = f instanceof function() { return Object; };`,
724+
output: `const result = f instanceof (() => Object);`,
725+
},
726+
{
727+
code: `const result = f in function() { return {}; };`,
728+
output: `const result = f in (() => ({}));`,
729+
},
730+
].map(withErrors(['USE_ARROW_WHEN_FUNCTION'])),
731+
});
732+
});
733+
734+
describe('when function expressions can be safely transformed without parentheses', () => {
735+
ruleTester.run('prefer-arrow-functions', rule, {
736+
valid: [],
737+
invalid: [
738+
// Assignment operators - don't need parentheses
739+
{
740+
code: `let result = function() { return 'value'; };`,
741+
output: `let result = () => 'value';`,
742+
},
743+
{
744+
code: `result += function() { return 1; };`,
745+
output: `result += () => 1;`,
746+
},
747+
{
748+
code: `result -= function() { return 1; };`,
749+
output: `result -= () => 1;`,
750+
},
751+
{
752+
code: `result *= function() { return 2; };`,
753+
output: `result *= () => 2;`,
754+
},
755+
{
756+
code: `result /= function() { return 2; };`,
757+
output: `result /= () => 2;`,
758+
},
759+
// Arrow function as right operand of arrow function
760+
{
761+
code: `const fn = () => function() { return 'nested'; };`,
762+
output: `const fn = () => () => 'nested';`,
763+
},
764+
// Ternary operators - right side doesn't need parentheses
765+
{
766+
code: `const result = condition ? 'yes' : function() { return 'no'; };`,
767+
output: `const result = condition ? 'yes' : () => 'no';`,
768+
},
769+
{
770+
code: `const result = condition ? function() { return 'yes'; } : 'no';`,
771+
output: `const result = condition ? () => 'yes' : 'no';`,
772+
},
773+
// Comma operator - doesn't need parentheses
774+
{
775+
code: `const result = (doSomething(), function() { return 'value'; });`,
776+
output: `const result = (doSomething(), () => 'value');`,
777+
},
778+
{
779+
code: `const result = (function() { return 'first'; }, doSomething());`,
780+
output: `const result = (() => 'first', doSomething());`,
781+
},
782+
{
783+
code: `const result = (doSomething(), function() { return 'second'; });`,
784+
output: `const result = (doSomething(), () => 'second');`,
785+
},
786+
// Spread operator - doesn't need parentheses
787+
{
788+
code: `const result = [...items, function() { return 'last'; }];`,
789+
output: `const result = [...items, () => 'last'];`,
790+
},
791+
// Yield - doesn't need parentheses
792+
{
793+
code: `function* gen() { yield function() { return 'value'; }; }`,
794+
output: `function* gen() { yield () => 'value'; }`,
795+
},
796+
{
797+
code: `function* gen() { yield* function() { return [1, 2, 3]; }; }`,
798+
output: `function* gen() { yield* () => [1, 2, 3]; }`,
799+
},
800+
].map(withErrors(['USE_ARROW_WHEN_FUNCTION'])),
801+
});
802+
});
803+
});

0 commit comments

Comments
 (0)