Skip to content

test(extractor): verify exported arrow function funcStack tracking in extractSpreadForOfWalk #1359

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.

Sign up for GitHub

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

Merged
carlos-alm merged 2 commits into from
Jun 6, 2026
Merged

test(extractor): verify exported arrow function funcStack tracking in extractSpreadForOfWalk #1359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f32cb83
test(extractor): verify exported arrow function funcStack tracking in…
carlos-alm Jun 6, 2026
f01c339
fix: remove duplicate paramBindings in SerializedExtractorOutput and …
carlos-alm Jun 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/domain/wasm-worker-protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type {
Export,
Import,
LanguageId,
ParamBinding,
TypeMapEntry,
} from '../types.js';

Expand Down Expand Up @@ -75,7 +74,6 @@ export interface SerializedExtractorOutput {
newExpressions?: readonly string[];
returnTypeMap?: Array<[string, TypeMapEntry]>;
callAssignments?: CallAssignment[];
paramBindings?: ParamBinding[];
}

export interface WorkerParseResponseOk {
Expand Down
25 changes: 25 additions & 0 deletions tests/parsers/javascript.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -908,4 +908,29 @@ describe('JavaScript parser', () => {
);
});
});

describe('Phase 8.3e: extractSpreadForOfWalk — exported arrow function funcStack (#1354)', () => {
it('tracks plain const arrow function on funcStack for for-of loop', () => {
const symbols = parseJS(`const f = (arr) => { for (const x of arr) x(); };`);
expect(symbols.forOfBindings).toContainEqual(expect.objectContaining({ enclosingFunc: 'f' }));
});

it('tracks exported const arrow function on funcStack for for-of loop', () => {
const symbols = parseJS(`export const f = (arr) => { for (const x of arr) x(); };`);
expect(symbols.forOfBindings).toContainEqual(expect.objectContaining({ enclosingFunc: 'f' }));
});

it('records correct varName and sourceName for exported arrow for-of', () => {
const symbols = parseJS(
`export const handleItems = (items) => { for (const cb of items) cb(); };`,
);
expect(symbols.forOfBindings).toContainEqual(
expect.objectContaining({
varName: 'cb',
sourceName: 'items',
enclosingFunc: 'handleItems',
}),
);
});
});
Comment on lines +923 to +935
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Jun 6, 2026

Choose a reason for hiding this comment

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

P2 process shadows a BUILTIN_GLOBALS entry, obscuring test intent

process appears on line 88 of src/extractors/javascript.ts inside BUILTIN_GLOBALS. A future reader of this test may reasonably assume it was chosen to verify how a built-in-named function interacts with the BUILTIN_GLOBALS guard — but BUILTIN_GLOBALS is never checked for enclosingFunc, so the name carries no semantic weight here. Using a clearly made-up name like handleItems or processItems would communicate that the test is purely about the export-wrapping path, not about built-in shadowing.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Jun 6, 2026

Choose a reason for hiding this comment

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

Fixed — renamed process to handleItems in commit f01c339. The test now uses a clearly made-up name that conveys it's purely about the export-wrapping code path, not built-in shadowing.

});
Loading