Skip to content

fix(prefer-in-document): check that a node has arguments before trying to access properties on them #165

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

Merged
merged 2 commits into from
Apr 24, 2021

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Apr 23, 2021

What:

It seems that prefer-in-document crashes in situations where it expected nodes to have an argument but they actually don't.

Why:
The rule assumes that if it's found the node its looking for, then the node has to have arguments and so makes property access keys in the first item in the args array without actually checking if its defined.

How:

I've added checks for the argument length before attempts to access said arguments.

Checklist:

  • Documentation N/A
  • Tests
  • Ready to be merged

@benmonro
Copy link
Member

@all-contributors please add @G-Rath for tests, code, bugs

@allcontributors
Copy link
Contributor

@benmonro

I've put up a pull request to add @G-Rath! 🎉

@benmonro
Copy link
Member

@G-Rath thanks for finding & fixing this!

@codecov
Copy link

codecov bot commented Apr 23, 2021

Codecov Report

Merging #165 (228e760) into master (0ae17db) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #165   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          381       383    +2     
  Branches        69        69           
=========================================
+ Hits           381       383    +2     
Impacted Files Coverage Δ
src/rules/prefer-in-document.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ae17db...228e760. Read the comment docs.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 24, 2021

@benmonro I've got some more fixes of this nature, which I'll push into this PR.

@benmonro
Copy link
Member

@G-Rath sounds good just let me know when to merge

@G-Rath G-Rath force-pushed the fix-length-check branch from 3f00a90 to 228e760 Compare April 24, 2021 01:41
@G-Rath
Copy link
Collaborator Author

G-Rath commented Apr 24, 2021

@benmonro this one should be good to go now.

@G-Rath G-Rath changed the title fix(prefer-in-document): handle toHaveLength without arguments fix(prefer-in-document): check that a node has arguments before trying to access properties on them Apr 24, 2021
@benmonro benmonro merged commit eb1bf68 into testing-library:master Apr 24, 2021
@G-Rath G-Rath deleted the fix-length-check branch April 24, 2021 01:52
@github-actions
Copy link

🎉 This PR is included in version 3.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants