fix(linter): fix false negatives in typescript/no-require-imports#19589
fix(linter): fix false negatives in typescript/no-require-imports#19589camc314 merged 6 commits intooxc-project:mainfrom
Conversation
- Flag `require()` with dynamic arguments (e.g. `require(variable)`, `require(path.join(...))`) — previously only string/template literals were matched via `is_require_call()`. - Flag `require()` inside class methods named `require` — same root cause: dynamic arg `module` was not a string literal so the call was silently skipped. - Fix double-diagnostics when `allow` config is set: string/template literal mismatches previously emitted on the argument span *and* the call expression span; now consistently emits once on the call span. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes false negatives in the typescript/no-require-imports linter rule where dynamic require() calls and require calls inside class methods named require were not being flagged. The root cause was the use of CallExpression::is_require_call() which only matches when the argument is a string or template literal.
Changes:
- Replaced
is_require_call()check with direct callee identifier validation to catch allrequire(...)calls regardless of argument type - Fixed duplicate diagnostics when
allowconfig is set by only emitting on call expression span - Added comprehensive test coverage for dynamic arguments and class method scenarios
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/typescript/no_require_imports.rs | Refactored require call detection logic to check callee identifier directly instead of using is_require_call(), reordered scope checks for efficiency, and simplified allow pattern checking to eliminate duplicate diagnostics |
| crates/oxc_linter/src/snapshots/typescript_no_require_imports.snap | Removed duplicate diagnostic entries (previously emitted on both argument and call spans) and added new diagnostics for the fixed false negative cases |
Merging this PR will not alter performance
Comparing Footnotes
|
… args
Ensures the two existing guards correctly suppress diagnostics when
`require` is locally defined and called with a non-literal argument:
- root scope: `const require = createRequire(); require(someModule)`
- nested scope: `function foo() { let require = bazz; require(someModule) }`
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Oxlint ### 🚀 Features - 46177dd linter: Implement unicorn/prefer-module (#19603) (camc314) - 42f78bb linter: Implement unicorn/prefer-ternary (#19605) (camc314) ### 🐛 Bug Fixes - 43df857 react/exhaustive-deps: Normalize .current callback deps (#19610) (camc314) - 574f48f linter/no-throw-literal: Close warning block (#19612) (camc314) - 79fe3b4 linter/prefer-mock-return-shorthand: Avoid unsafe autofixes for call-like returns (#19581) (camc314) - 85045e8 linter: Check protected members in explicit-module-boundary-types (#19594) (camc314) - e38115e linter: Catch missing return type on exported arrow/function expressions (#19587) (Peter Wagenet) - 419d3fd linter: Fix false negatives in typescript/no-require-imports (#19589) (Peter Wagenet) - 7958b56 linter: Fix syntax error reporting in some output formatters. (#19590) (connorshea) - 024f51c linter: Add help text to more eslint diagnostics (#19591) (Anthony Amaro) - a8489a1 linter: Warning `eslint/no-throw-literal` rule to be deprecated, better use `typescript/only-throw-error` (#19593) (Said Atrahouch) - 50fc70d linter/type-aware: Use correct span for disable directives (#19576) (camc314) - 421a99c linter: Add help guidance to eslint diagnostic messages (#19562) (Anthony Amaro) - e81364a linter: Add help text to eslint rule diagnostics (#19560) (Anthony Amaro) - 89b58d0 linter: Add help text to more eslint rule diagnostics (#19561) (Anthony Amaro) - 74f7833 linter/jest/prefer-mock-return-shorthand: Preserve typed arrow returns (#19556) (camc314) - bdd6f34 linter: Restrict prefer-import-in-mock to mock calls (#19555) (camc314) ### 📚 Documentation - a331993 linter: Improve docs for `eslint/radix` rule. (#19611) (connorshea) ### 🛡️ Security - c67f9dc linter: Update ajv version. (#19613) (connorshea) # Oxfmt ### 🚀 Features - 984dc07 oxfmt: Strip `"experimental"SortXxx` prefix (#19567) (leaysgur) ### 🐛 Bug Fixes - d7b63a4 oxfmt: Update API types for `sortPackageJsonOptions` (#19569) (leaysgur) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Summary
The
typescript/no-require-importsrule had two false negatives:require()with dynamic arguments was not flagged:require()inside a class method namedrequirewas not flagged:Both cases share the same root cause: the rule used
CallExpression::is_require_call()to detectrequirecalls, which only matches when the argument is aStringLiteralorTemplateLiteral. Calls with dynamic arguments were silently skipped.Fix: Replace
is_require_call()with a direct callee identifier check. The rule now flags anyCallExpressionwhere the callee is the unqualified identifierrequireresolving to the global, regardless of argument type.Also fixed: double-diagnostics when the
allowconfig option is set. Non-matching string/template literal arguments previously emitted a diagnostic on the argument span and then fell through to emit a second diagnostic on the call expression span. Now a single diagnostic is emitted on the call expression span. The same issue was present in theTSImportEqualsDeclarationpath and is fixed there too.Test plan
failcases:require(someVariable),require(path.join(dir, file)),require(module)andrequire('foo')inside a class method namedrequirecargo test -p oxc_linter --lib no_require_importspasses🤖 Generated with Claude Code