Skip to content

Commit f941a81

Browse files
JamieMasonJamieMason
committed
feat: handle nested functions
Closes #39
1 parent 7ad8bdf commit f941a81

File tree

3 files changed

+246
-10
lines changed

3 files changed

+246
-10
lines changed

‎src/guard.ts‎

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,63 @@ export class Guard {
9393
return this.containsToken('Keyword', 'this', node);
9494
}
9595

96+
containsThisDirectly(node: TSESTree.Node): boolean {
97+
// Check if 'this' is used directly in the current function, excluding nested functions
98+
const visit = (currentNode: TSESTree.Node | null | undefined, insideNestedFunction: boolean): boolean => {
99+
if (!currentNode || typeof currentNode !== 'object' || !currentNode.type) {
100+
return false;
101+
}
102+
103+
// If we find a 'this' expression and we're not inside a nested function, return true
104+
if (currentNode.type === AST_NODE_TYPES.ThisExpression && !insideNestedFunction) {
105+
return true;
106+
}
107+
108+
// Check if current node is a nested function
109+
const isNestedFunction =
110+
currentNode.type === AST_NODE_TYPES.FunctionDeclaration ||
111+
currentNode.type === AST_NODE_TYPES.FunctionExpression ||
112+
currentNode.type === AST_NODE_TYPES.ArrowFunctionExpression;
113+
114+
// If we encounter a nested function, mark that we're inside one for its children
115+
const newInsideNestedFunction = insideNestedFunction || (isNestedFunction && currentNode !== node);
116+
117+
// Skip traversing function body if this is a nested function (not the root)
118+
if (isNestedFunction && currentNode !== node) {
119+
return false;
120+
}
121+
122+
// Recursively check all child nodes
123+
for (const key in currentNode) {
124+
if (key === 'parent' || key === 'range' || key === 'loc') {
125+
continue; // Skip these properties to avoid circular references
126+
}
127+
128+
const child = currentNode[key as keyof TSESTree.Node];
129+
130+
if (child && typeof child === 'object') {
131+
if (Array.isArray(child)) {
132+
for (const item of child) {
133+
if (item && typeof item === 'object' && 'type' in item) {
134+
if (visit(item as TSESTree.Node, newInsideNestedFunction)) {
135+
return true;
136+
}
137+
}
138+
}
139+
} else if ('type' in child) {
140+
if (visit(child as TSESTree.Node, newInsideNestedFunction)) {
141+
return true;
142+
}
143+
}
144+
}
145+
}
146+
147+
return false;
148+
};
149+
150+
return visit(node, false);
151+
}
152+
96153
containsArguments(node: TSESTree.Node): boolean {
97154
return this.containsToken('Identifier', 'arguments', node);
98155
}
@@ -178,7 +235,7 @@ export class Guard {
178235
!this.isGeneratorFunction(fn) &&
179236
!this.isAssertionFunction(fn) &&
180237
!this.isOverloadedFunction(fn) &&
181-
!this.containsThis(fn) &&
238+
!this.containsThisDirectly(fn) &&
182239
!this.containsSuper(fn) &&
183240
!this.containsArguments(fn) &&
184241
!this.containsNewDotTarget(fn);

‎src/writer.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,18 @@ export class Writer {
7676
if (!parent) return false;
7777

7878
// 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) {
79+
if (
80+
(parent.type === AST_NODE_TYPES.BinaryExpression || parent.type === AST_NODE_TYPES.LogicalExpression) &&
81+
parent.right === node
82+
) {
8183
return true;
8284
}
8385

8486
// 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+
if (
88+
(parent.type === AST_NODE_TYPES.BinaryExpression || parent.type === AST_NODE_TYPES.LogicalExpression) &&
89+
parent.left === node
90+
) {
8791
// Don't add parentheses for assignment-like operators
8892
if (parent.operator === 'in' || parent.operator === 'instanceof') {
8993
return true;
@@ -110,11 +114,13 @@ export class Writer {
110114
// - yield, yield*
111115
// - spread ...
112116
// - 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) {
117+
if (
118+
parent.type === AST_NODE_TYPES.AssignmentExpression ||
119+
parent.type === AST_NODE_TYPES.ArrowFunctionExpression ||
120+
parent.type === AST_NODE_TYPES.YieldExpression ||
121+
parent.type === AST_NODE_TYPES.SpreadElement ||
122+
parent.type === AST_NODE_TYPES.SequenceExpression
123+
) {
118124
return false;
119125
}
120126

‎test/rule.spec.ts‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,3 +801,176 @@ describe('issue #37 - arrow function precedence in operator expressions', () =>
801801
});
802802
});
803803
});
804+
805+
describe('issue #39 - outer functions should be transformed even if inner functions use this', () => {
806+
describe('when outer functions should NOT be transformed (they themselves use this)', () => {
807+
ruleTester.run('prefer-arrow-functions', rule, {
808+
valid: [
809+
{
810+
code: `function outer() {
811+
console.log(this);
812+
function inner() {
813+
return this;
814+
}
815+
return inner;
816+
}`,
817+
},
818+
{
819+
code: `function outer() {
820+
const self = this;
821+
function inner() {
822+
return this;
823+
}
824+
return self;
825+
}`,
826+
},
827+
{
828+
code: `const obj = {
829+
method: function() {
830+
console.log(this);
831+
function helper() {
832+
return this;
833+
}
834+
return helper();
835+
}
836+
}`,
837+
},
838+
],
839+
invalid: [],
840+
});
841+
});
842+
843+
describe('when outer functions can be safely transformed (inner functions use this but outer does not)', () => {
844+
ruleTester.run('prefer-arrow-functions', rule, {
845+
valid: [],
846+
invalid: [
847+
{
848+
code: `function outer() {
849+
function inner() {
850+
return this;
851+
}
852+
return inner;
853+
}`,
854+
output: `const outer = () => {
855+
function inner() {
856+
return this;
857+
}
858+
return inner;
859+
};`,
860+
},
861+
{
862+
code: `function outer() {
863+
console.log('outer function');
864+
function inner() {
865+
return this;
866+
}
867+
return inner();
868+
}`,
869+
output: `const outer = () => {
870+
console.log('outer function');
871+
function inner() {
872+
return this;
873+
}
874+
return inner();
875+
};`,
876+
},
877+
{
878+
code: `function outer(param) {
879+
let result;
880+
function inner() {
881+
console.log(this);
882+
return param;
883+
}
884+
result = inner();
885+
return result;
886+
}`,
887+
output: `const outer = (param) => {
888+
let result;
889+
function inner() {
890+
console.log(this);
891+
return param;
892+
}
893+
result = inner();
894+
return result;
895+
};`,
896+
},
897+
{
898+
code: `function outer() {
899+
const helper = function() {
900+
return this;
901+
};
902+
return helper;
903+
}`,
904+
output: `const outer = () => {
905+
const helper = function() {
906+
return this;
907+
};
908+
return helper;
909+
};`,
910+
},
911+
{
912+
code: `function outer() {
913+
return function() {
914+
return this;
915+
};
916+
}`,
917+
output: `const outer = () => function() {
918+
return this;
919+
};`,
920+
},
921+
{
922+
code: `function outer() {
923+
function innerA() {
924+
return this;
925+
}
926+
function innerB() {
927+
console.log(this);
928+
}
929+
return [innerA, innerB];
930+
}`,
931+
output: `const outer = () => {
932+
function innerA() {
933+
return this;
934+
}
935+
function innerB() {
936+
console.log(this);
937+
}
938+
return [innerA, innerB];
939+
};`,
940+
},
941+
{
942+
code: `const obj = {
943+
method: function() {
944+
function helper() {
945+
return this;
946+
}
947+
return helper();
948+
}
949+
}`,
950+
output: `const obj = {
951+
method: () => {
952+
function helper() {
953+
return this;
954+
}
955+
return helper();
956+
}
957+
}`,
958+
},
959+
{
960+
code: `export default function() {
961+
function inner() {
962+
return this;
963+
}
964+
return inner;
965+
}`,
966+
output: `export default () => {
967+
function inner() {
968+
return this;
969+
}
970+
return inner;
971+
};`,
972+
},
973+
].map(withErrors(['USE_ARROW_WHEN_FUNCTION'])),
974+
});
975+
});
976+
});

0 commit comments

Comments
 (0)