Skip to content

fix(no-node-access): Improve detection logic in no-node-access to resolve imported aliases and setup instances #1033

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.

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

y-hsgw
Copy link
Member

@y-hsgw y-hsgw commented Jun 22, 2025

As suggested in this comment, I’ve implemented a resolveToTestingLibraryFn utility inspired by eslint-plugin-jest’s parseJestFnCall.

Checks

Changes

  • Introduced resolveToTestingLibraryFn, which improves accuracy by accepting a CallExpression node and resolving where it comes into the file — in other words, tracing it back to its original import source. This follows the same principle used in eslint-plugin-jest's parseJestFnCall.
  • I’ve also created an accessors.ts module that parseJestFnCall depends on. It’s largely traced from the original implementation in eslint-plugin-jest.
  • Addressed one of the concerns raised in issue no-node-access rule output false-positive warning after 7.5.0 release #1024:
    • userEvent.setup() instances with custom variable names (not just user) are now recognized and excluded from being flagged incorrectly (e.g., const userEvt = userEvent.setup(); userEvt.click(...) is no longer a false positive).

I understand this PR is a bit large—if it would help with the review process, I'm happy to split it into smaller parts. Please let me know if you'd prefer that.🙇‍♂️

Context

Fixes #1024

@y-hsgw y-hsgw self-assigned this Jun 22, 2025
Copy link

codecov bot commented Jun 22, 2025

Codecov Report

Attention: Patch coverage is 93.57143% with 9 lines in your changes missing coverage. Please review.

Project coverage is 96.15%. Comparing base (1c1497a) to head (f7ce66f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/utils/resolve-to-testing-library-fn.ts 92.40% 5 Missing and 1 partial ⚠️
lib/node-utils/accessors.ts 85.71% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
- Coverage   96.30%   96.15%   -0.16%     
==========================================
  Files          47       49       +2     
  Lines        2542     2676     +134     
  Branches     1051     1107      +56     
==========================================
+ Hits         2448     2573     +125     
- Misses         94      102       +8     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@y-hsgw y-hsgw changed the title fix(no-node-access): Add resolveToTestingLibraryFn Utility for CallExpression Resolution fix(no-node-access): Improve detection logic in no-node-access to resolve imported aliases and setup instances Jun 22, 2025
@Belco90 Belco90 self-requested a review June 26, 2025 05:12
@Belco90 Belco90 added the bug Something isn't working label Jun 26, 2025
@Belco90
Copy link
Member

Belco90 commented Jun 26, 2025

@y-hsgw thanks! I'll try to review this during the week.

Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

I'm happy with the changes in general, but I'd like to think a bit more about the implications regarding the aggressive reporting.

@y-hsgw
Copy link
Member Author

y-hsgw commented Jun 29, 2025

I've added preliminary support for testing-library/utils-module in 6781117.
I hope this helps as a reference for further discussion.
Please feel free to let me know if it doesn't align with your expectations — I'm happy to revert if needed 🙇‍♂️

@Belco90
Copy link
Member

Belco90 commented Jul 3, 2025

I've added preliminary support for testing-library/utils-module in 6781117. I hope this helps as a reference for further discussion. Please feel free to let me know if it doesn't align with your expectations — I'm happy to revert if needed 🙇‍♂️

Thanks, this is great! I think it's fine and shouldn't overlap with the aggressive reporting for now. I'll give it another go and let you know if I can think of any improvement.

In the meantime, codecov is complaining about decreased code coverage so you can add some tests to cover that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-node-access rule output false-positive warning after 7.5.0 release
2 participants