fix(linter/no-eval): resolve this binding for functions returned from an IIFE by shulaoda · Pull Request #23643 · oxc-project/oxc

shulaoda · 2026-06-19T11:27:16Z

What

is_callee (in ast_util.rs) is used only by is_default_this_binding, which is used only by the no-eval rule to decide whether this in this.eval(...) refers to the global object.

Two coupled bugs in that path are fixed here:

is_callee always returned false. Its closure parameter was named node, shadowing the outer node, so node.kind().span() referred to the parent CallExpression rather than the node under test. callee.span().contains_inclusive(...) then compared the callee span against the whole call-expression span — impossible — so it never reported an actual callee.
The traversal in is_default_this_binding never stepped above the call. After the is_callee check, the loop did current_node = parent (the ReturnStatement / arrow) instead of continuing from above the call site (ESLint does node = func.parent / node = parent.parent), so it always bottomed out at return true. This masked fix (1) entirely.

Effect

With both fixed, no-eval no longer false-positives on a function returned from an immediately-invoked function and assigned to a member, which matches ESLint:

// foo.cjs, { allowIndirect: false }
obj.foo = (function() { return function() { this.eval('foo'); }; })()

The returned function's this is bound to obj, not the global object, so this.eval is not indirect eval. Previously this was reported; now it is not.

Test

Added a no-eval pass case for the above. The whole no-eval suite still passes. (A buggy is_callee falls through the Some(func) if is_callee(...) guard to return true, re-flagging the case, so the rule test covers both fixes.)

codspeed-hq · 2026-06-19T11:35:25Z

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 66 skipped benchmarks¹

_{Comparing shulaoda:06-19-fix_is_callee_closure_shadowing (9db2f54) with main (bca7ce5)}

66 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

camc314

can we add a test case via a rule for this on?

how is this function used?

shulaoda · 2026-06-19T12:17:36Z

Good call — done, and it pointed to a better fix.

is_callee is used only by is_default_this_binding (in ast_util.rs), which is used only by the no-eval rule (to decide whether this in this.eval(...) is the global object).

The isolated is_callee fix turned out to have no observable effect, because is_default_this_binding had a second bug right next to it: after the is_callee check it did current_node = parent instead of stepping above the call (ESLint does node = func.parent / node = parent.parent), so it always bottomed out at return true. Fixing only one of the two does nothing.

I've fixed both and added a rule test. For example, under { allowIndirect: false } in a CommonJS file:

obj.foo = (function() { return function() { this.eval('foo'); }; })()

is now correctly not reported (the returned function's this is bound to obj, matching ESLint), whereas before it was a false positive. The full no-eval suite still passes.

I've updated the PR title/description accordingly.

… an IIFE

camc314

thanks!

Copilot

Pull request overview

This pull request fixes the no-eval linter rule’s detection of whether this.eval(...) is an indirect eval by correcting how the rule determines if this is bound to the global object, specifically for functions returned from an IIFE and then assigned as an object member.

Changes:

Fix ast_util::is_callee to correctly test whether a node is the callee of a CallExpression (by using the tested node’s span rather than the parent call’s span).
Fix ast_util::is_default_this_binding traversal to continue from above the IIFE call site (matching ESLint’s approach), including the arrow-expression-body shape.
Add no-eval pass tests covering IIFE-returned functions assigned to a member (function IIFE and arrow IIFE variants).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
crates/oxc_linter/src/rules/eslint/no_eval.rs	Adds pass cases ensuring `this.eval` inside an IIFE-returned function assigned to a member is not treated as indirect eval.
crates/oxc_linter/src/ast_util.rs	Fixes `is_callee` span logic and corrects `is_default_this_binding` traversal to step above the call site (including arrow IIFE patterns).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9db2f540cb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-02T19:33:08Z

+                    Some(func) if is_callee(func, semantic) => {
+                        current_node = outermost_paren_parent(func, semantic).unwrap();


Reject async/generator IIFEs before following returns

When the enclosing IIFE is async or a generator, calling it does not produce the function from the return statement; it produces a Promise or iterator. This branch still jumps to the outer call, so in CommonJS with allowIndirect: false, obj.foo = (function*() { return function() { this.eval('x'); }; })() or the async-function variant is treated as assigning the inner function to obj.foo and no-eval stops reporting, even though that function is not bound to obj and can still run with default this. Please keep returning true for async/generator enclosing functions.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-02T19:33:08Z

+                if !expr.expression
+                    || expr_stmt.expression.span() != current_node.span()
+                    || !is_callee(arrow_func, semantic)


Reject async concise arrows before following their body

For a concise async arrow IIFE, the call stores a Promise, not the function expression in the arrow body. With this path, obj.foo = (async () => function() { this.eval('x'); })() is now classified as a member assignment of the returned function and the rule suppresses the diagnostic in CommonJS with allowIndirect: false, although the nested function is not actually bound to obj. Please include expr.r#async in the rejection before jumping to the call expression.

Useful? React with 👍 / 👎.

…ed from an IIFE (#23643) ## What `is_callee` (in `ast_util.rs`) is used only by `is_default_this_binding`, which is used only by the `no-eval` rule to decide whether `this` in `this.eval(...)` refers to the global object. Two coupled bugs in that path are fixed here: 1. **`is_callee` always returned `false`.** Its closure parameter was named `node`, shadowing the outer `node`, so `node.kind().span()` referred to the parent `CallExpression` rather than the node under test. `callee.span().contains_inclusive(...)` then compared the callee span against the whole call-expression span — impossible — so it never reported an actual callee. 2. **The traversal in `is_default_this_binding` never stepped above the call.** After the `is_callee` check, the loop did `current_node = parent` (the `ReturnStatement` / arrow) instead of continuing from above the call site (ESLint does `node = func.parent` / `node = parent.parent`), so it always bottomed out at `return true`. This masked fix (1) entirely. ## Effect With both fixed, `no-eval` no longer false-positives on a function returned from an immediately-invoked function and assigned to a member, which matches ESLint: ```js // foo.cjs, { allowIndirect: false } obj.foo = (function() { return function() { this.eval('foo'); }; })() ``` The returned function's `this` is bound to `obj`, not the global object, so `this.eval` is not indirect eval. Previously this was reported; now it is not. ## Test Added a `no-eval` `pass` case for the above. The whole `no-eval` suite still passes. (A buggy `is_callee` falls through the `Some(func) if is_callee(...)` guard to `return true`, re-flagging the case, so the rule test covers both fixes.) --------- Co-authored-by: Cameron <[email protected]>

# Oxlint ### 🚀 Features - 7db7a29 allocator: Add `ReplaceWith` trait (#24012) (overlookmotel) - a2c97f3 linter/unicorn: Implement `explicit-timer-delay` rule (#23612) (Mikhail Baev) - 85735cb linter/unicorn: Implement `no-confusing-array-with` rule (#23638) (Shekhu☺️) - cb4fbb9 linter/eslint: Implement no-unreachable-loop rule (#23975) (Todor Andonov) - dc32112 linter/eslint/no-constant-binary-expression: Check relational comparisons (#24088) (camc314) - 439c344 linter/jsdoc: Added missing options to `jsdoc/require-param` rule (#23364) (kapobajza) - 62af717 linter/unicorn/filename-case: Add `lowercase` and `screamingSnakeCase` (#24045) (Boshen) - d963967 linter/unicorn/no-array-sort: Add `allowAfterSpread` option (#24043) (Boshen) - 0a75682 linter: Add per-rule timings for type-aware linting (#22488) (camchenry) - 743e222 linter/react: Add `disallowedValues` option for `forbid-dom-props` rule (#23970) (Mikhail Baev) ### 🐛 Bug Fixes - 7b80010 linter: Use direct binding symbol ids (#24216) (camc314) - 8f94b49 linter/import/no-duplicates: Don't flag a type-only import beside a side-effect import (#24030) (Boshen) - d8c3fee linter/react/rules-of-hooks: Flag `useEffectEvent` escapes (#23764) (Rayan Salhab) - 0a7312b linter/no-deprecated-functions: Map `require.requireActual` to `jest.requireActual` (#23627) (Jerry Zhao) - d9e3ab3 linter/eslint/no-useless-return: Handle switch case continuation (#23984) (camc314) - 0b25582 ast: Type binding node `typeAnnotation` as `TSTypeAnnotation | null` (#23113) (Boshen) - 122d112 linter/eslint/no-restricted-imports: Flag dynamic import() expressions (#24029) (Boshen) - 59b6b83 linter: Avoid `OnceLock` re-entry on cyclic `export *` re-exports (#23632) (Jerry Zhao) - dd09af0 linter/import/namespace: Avoid panic on destructuring of an unresolvable namespace re-export (#23626) (Jerry Zhao) - bdb51c7 linter/jest/prefer-ending-with-an-expect: Validate config patterns (#24122) (camc314) - e383843 linter/unicorn/prefer-modern-dom-apis: Skip fixer for non identifier arguments (#23630) (Jerry Zhao) - 0ac4c83 linter: Detect circular config extends (#24115) (camc314) - bae1edf linter/import/namespace: Check namespace imports after named imports (#24094) (camc314) - cd8fdfe linter/eslint/no-eval: Recognize Array.from family thisArg (#24091) (camc314) - 851ee43 linter/eslint/no-eval: Resolve this binding for functions returned from an IIFE (#23643) (Jerry Zhao) - 002ab35 linter/unicorn: Avoid prefer-array-find rest destructuring false positive (#23654) (ColemanDunn) - 01c8775 linter/unicorn/filename-case: Keep digits attached in screamingSnakeCase (#24056) (Boshen) - f256941 linter: Recognize `@effect/vitest` as a vitest import source (#24025) (Boshen) - 73eeb1d linter/import/extensions: Honor per-extension `never` for explicit extensions (#24031) (Boshen) - d4ebe1f linter: Reject non-object oxlint config files (#24026) (Boshen) - 45d607d linter/react/forbid-component-props: Make allow/disallow lists optional in schema (#24024) (Boshen) - 54076ad linter/unicorn/no-array-for-each: Suggest entries loop for index callbacks (#24004) (camc314) - d057736 linter/jsdoc: Avoid param root underflow (#23945) (camc314) - 29c76bf linter/unicorn/prefer-at: Skip object numeric-key access (#23909) (Gaurav Dubey) ### ⚡ Performance - 657a8fc linter/oxc/bad-array-method-on-arguments: Only run on member expressions instead of all identifiers (#24164) (camchenry) - 073d9e7 linter/eslint/prefer-rest-params: Run on functions instead of all identifiers (#24163) (camchenry) - e5a4162 linter/jest/no-confusing-set-timeout: Early exit fast path (#24092) (camc314) - bca7ce5 linter: Only run react-perf rules on JSX attribute nodes (#24083) (camchenry) - 6881bf6 linter: Compute `apply_overrides` rule set lazily (#23648) (Jerry Zhao) - 911c106 linter/eslint/no-obj-calls: Use resolved reference instead of scope walk (#23895) (Marius Schulz) - dc8fd9a linter/unicorn/prefer-dom-node-text-content: Change dispatch to run only on less common node types (#23897) (Connor Shea) - fdbd34d linter/eslint/no-useless-call: Fast-path static callees (#24077) (camc314) - b1be114 linter/import/extensions: Skip empty config and borrow extensions (#24075) (camc314) - 4781b2d linter/eslint/no-obj-calls: Use direct global matches (#24076) (camc314) - e6cee89 linter: Avoid node-chain allocation for non-Jest calls (#23907) (Yagiz Nizipli) - 30dc517 linter/typescript/no-restricted-types: O(1) banned-type lookups (#23827) (Yagiz Nizipli) ### 📚 Documentation - 6ca9125 linter/typescript: Clarify consistent-type-imports behavior (#23972) (camc314) # Oxfmt ### 🚀 Features - 4f4313e formatter_css: Update oxc-css-parser 0.0.5 (#24120) (leaysgur) - 0ccd8a1 formatter_graphql: Update oxc-graphql-parser 0.0.5 (#24106) (leaysgur) - 89ec3d9 formatter_core: Add literal line and root indention primitives (#24051) (leaysgur) - 213a96b formatter_core: Add no-expand-parent for multiline text (#24050) (leaysgur) - 0e5bcc9 formatter_graphql: Update oxc-graphql-parser 0.0.4 (#24039) (leaysgur) - e0b35a1 formatter_css: Update `[email protected]` (#23974) (leaysgur) ### 🐛 Bug Fixes - 1fe6546 formatter: Omit unneeded `;` for type members with `no-semi` (#24212) (leaysgur) - 0ad7316 formatter: Print space for `ForStatement`.`update` only if exists (#24211) (leaysgur) - 3abbed5 formatter: Print `;` before jsdoc type-cast parens with no-semi (#24208) (leaysgur) - 9af3833 formatter_css: Make scss formatter consistent (#24207) (leaysgur) - 46d7194 formatter_css: Use fill IR for `@forward` members (#24206) (leaysgur) - e31038f formatter_css: Keep comment inside sass config list (#24205) (leaysgur) - d3b9591 formatter: Add parens around `await/yield` with `<T>` (#24202) (leaysgur) - 2121a55 oxfmt: Reuse tinypool process during the same LSP process (#24197) (leaysgur) - 9bf4b4a formatter_css: Align CSS output to Prettier 3.9.1 (#24100) (leaysgur) - cd2452e formatter_css: Align SCSS output to Prettier 3.9.1 (#24097) (leaysgur) - 4ee8745 formatter_css: Keep selector value contain line-break without breaking line (#24055) (leaysgur) - e1ece97 formatter_graphql: Break `implements` list by print-width (#23997) (leaysgur) - 0a6b16c formatter_json: Preserve key and literal value for json-stringify (#23996) (leaysgur) - 903ab6e formatter_css: Preserve newlines in css-in-js selector list (#23992) (leaysgur) - ea5d095 oxfmt: Update `--migrate prettier` (#23963) (leaysgur) ### ⚡ Performance - 468e1e3 formatter_core: Make printer queues cursor-based (#24098) (Boshen) - c59f2fe rust: Return impl ExactSizeIterator from slice-backed accessors (#24144) (Boshen) - c292fb2 formatter: Inline fits element dispatcher (#23982) (camc314) Co-authored-by: Boshen <[email protected]>

shulaoda requested a review from camc314 as a code owner June 19, 2026 11:27

camc314 requested changes Jun 19, 2026

View reviewed changes

shulaoda marked this pull request as draft June 19, 2026 12:02

shulaoda force-pushed the 06-19-fix_is_callee_closure_shadowing branch from 237e84e to 7c7de6d Compare June 19, 2026 12:17

shulaoda changed the title ~~fix(linter): correct closure shadowing that made is_callee always return false~~ fix(linter/no-eval): resolve this binding for functions returned from an IIFE Jun 19, 2026

camc314 added the A-linter Area - Linter label Jun 19, 2026

camc314 self-assigned this Jun 19, 2026

shulaoda and others added 2 commits July 2, 2026 20:15

fix(linter/no-eval): resolve this binding for functions returned from…

9994a75

… an IIFE

fix(linter/no-eval): handle concise arrow IIFEs

9db2f54

camc314 force-pushed the 06-19-fix_is_callee_closure_shadowing branch from 7c7de6d to 9db2f54 Compare July 2, 2026 19:22

camc314 approved these changes Jul 2, 2026

View reviewed changes

camc314 marked this pull request as ready for review July 2, 2026 19:26

Copilot AI review requested due to automatic review settings July 2, 2026 19:26

Copilot started reviewing on behalf of camc314 July 2, 2026 19:26 View session

camc314 merged commit 851ee43 into oxc-project:main Jul 2, 2026
28 checks passed

Copilot AI reviewed Jul 2, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 2, 2026

View reviewed changes

oxc-guard Bot mentioned this pull request Jul 6, 2026

release(apps): oxlint v1.73.0 && oxfmt v0.58.0 #24219

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(linter/no-eval): resolve this binding for functions returned from an IIFE#23643

fix(linter/no-eval): resolve this binding for functions returned from an IIFE#23643
camc314 merged 2 commits into
oxc-project:mainfrom
shulaoda:06-19-fix_is_callee_closure_shadowing

shulaoda commented Jun 19, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

camc314 left a comment

Uh oh!

shulaoda commented Jun 19, 2026

Uh oh!

camc314 left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 2, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		Some(func) if is_callee(func, semantic) => {
		current_node = outermost_paren_parent(func, semantic).unwrap();

Uh oh!

Uh oh!

Conversation

shulaoda commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Effect

Test

Uh oh!

codspeed-hq Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

camc314 left a comment

Choose a reason for hiding this comment

Uh oh!

shulaoda commented Jun 19, 2026

Uh oh!

camc314 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shulaoda commented Jun 19, 2026 •

edited

Loading

codspeed-hq Bot commented Jun 19, 2026 •

edited

Loading