Skip to content

fix(eslint-plugin): normalize whitespace in allowList variable matching for multiline expressions#10337

Open
mixelburg wants to merge 1 commit intoTanStack:mainfrom
mixelburg:fix/exhaustive-deps-allowlist-multiline
Open

fix(eslint-plugin): normalize whitespace in allowList variable matching for multiline expressions#10337
mixelburg wants to merge 1 commit intoTanStack:mainfrom
mixelburg:fix/exhaustive-deps-allowlist-multiline

Conversation

@mixelburg
Copy link

@mixelburg mixelburg commented Mar 26, 2026

Summary

Fixes #10334

Root Cause

When a member expression spans multiple lines in source code, returns the text verbatim including the newline and indentation:

queryFn: () => ignored
  .run() // getText returns 'ignored\n  .run'

computeRefPath splits on '.' to extract the root identifier, so the root becomes 'ignored\n ' (with trailing whitespace) instead of 'ignored'. This never matches the allowlisted variable name.

Fix

Extend normalizeChain to collapse whitespace in addition to stripping optional-chaining operators and non-null assertions. This makes ignored\n .runignored.run before the path/root extraction.

Changes

  • exhaustive-deps.utils.ts: add .replace(/\s+/g, '') to normalizeChain
  • exhaustive-deps.test.ts: add regression test for the multiline case

All 1531 existing tests pass.

Summary by CodeRabbit

  • Tests

    • Added test coverage for allowlisted variables with multi-line member access in the exhaustive-deps rule.
  • Bug Fixes

    • Improved dependency chain normalization to correctly handle multi-line formatting when validating allowlisted variable references.

When a member expression spans multiple lines (e.g. `ignored\n    .run()`),
`sourceCode.getText()` preserves the newline. The root segment extracted
by splitting on `.'` then becomes `'ignored\n    '`, which never matches
the allowlisted variable name `'ignored'`.

Fix: extend `normalizeChain` to also collapse all whitespace, so
multi-line chains produce the same identifier path as single-line ones.

Fixes TanStack#10334
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c44f0dc0-5926-4e29-8079-bf50003cff21

📥 Commits

Reviewing files that changed from the base of the PR and between 1047cdc and 84adca5.

📒 Files selected for processing (2)
  • packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts
  • packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts

📝 Walkthrough

Walkthrough

Added a test case validating that allowlisted variables are not flagged as missing dependencies regardless of formatting. Modified the normalizeChain utility function to strip whitespace from normalized dependency chains, fixing canonicalization for multi-line member accesses.

Changes

Cohort / File(s) Summary
Test Coverage Addition
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts
Added new test case under allowlist.variables validating that allowlisted variables remain valid when member access spans multiple lines, addressing formatting-dependent false positives.
Normalization Logic Update
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts
Enhanced normalizeChain to remove all whitespace characters from normalized text via .replace(/\s+/g, ''), ensuring dependency chain comparisons are insensitive to line breaks and spacing variations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 With whiskers twitching, I've hopped through the code,
Where whitespace was blocking the allowlist road,
One regex to fix it, one test to be sure,
Now linebreaks won't trick us—the fix is pure! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: normalizing whitespace in allowlist variable matching for multiline expressions, which directly corresponds to the primary code changes.
Description check ✅ Passed The description provides a clear root cause analysis, explains the fix, lists the changes, and notes test status; it adequately fills the template sections for changes and release impact.
Linked Issues check ✅ Passed The code changes fully address the requirements in #10334: multiline member expressions are now properly normalized, allowing allowlisted variables to be correctly matched regardless of formatting.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the whitespace normalization issue described in #10334; no unrelated modifications were introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[eslint-plugin] exhaustive-deps reports error even if variable is set in allowList

1 participant