-
Notifications
You must be signed in to change notification settings - Fork 40
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
fix(prefer-in-document): handle toHaveLength
without any arguments and with trailing commas
#276
fix(prefer-in-document): handle toHaveLength
without any arguments and with trailing commas
#276
Conversation
for (const argument of Array.from(matcherArguments)) { | ||
const sourceCode = context.getSourceCode(); | ||
const token = sourceCode.getTokenAfter(argument); | ||
if (token.value === "," && token.type === "Punctuator") { | ||
// Remove commas if toHaveLength had more than one argument or a trailing comma | ||
operations.push(fixer.replaceText(token, "")); | ||
} | ||
operations.push(fixer.remove(argument)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better fix for this should be to use the starting location of the first argument and then the ending location of the last argument - that way we don't have to actually care about the commas at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might me right, I thought about it also (couldn't tried since I don't know the working mechanism of the eslint very well),
but I felt like comments would create problems if we handled it that way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problems with comments are not something we can really avoid - I'm happy to pick up getting the PR landed if you've not got the bandwidth :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would be great, thanks 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok so I was thinking about the handing for another situation (ESLint fixers are a bit mystic because of all the different ways code can be shaped 😂) - what you've got is good; it also turns out we're not handling trailing commas properly in eslint-plugin-jest
either so thanks for being this to my attention!
Codecov Report
@@ Coverage Diff @@
## main #276 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 15 15
Lines 575 581 +6
Branches 165 167 +2
=========================================
+ Hits 575 581 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
toHaveLength
without any arguments and with trailing commas
🎉 This PR is included in version 4.0.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What:
Why:
Eslint plugin crush stack trace in VSCode:
Even after handling calling toHaveLength without any arguments, it won't work since toHaveLength expected argument is required. Thus, toHaveLength without any arguments considered as toHaveLength zero.
2.
toHaveLength(0)
andtoHaveLength(1)
on a RTL query replaced to propertoBeInTheDocument
's. Logic is already applied to work with multiple arguments but, if there were multiple arguments after 0 or 1, commas aren't removed.How:
(matcherArguments.length === 0 || matcherArguments[0].value === 0)
Checklist:
Fixes #277