chore: add prefer-to-contain rule #174
Conversation
| `expect(a.includes(b)).toEqual("test")`, | ||
| `expect(a.includes(b)).toBe("test")`, | ||
| `expect(a.includes()).toEqual()`, | ||
| `expect(a.includes(b)).toBe(false)`, |
There was a problem hiding this comment.
could we support this, and suggest expect(a).not.toContain(b)?
There was a problem hiding this comment.
Agree on supporting the .not.toContain() syntax. 👍
There was a problem hiding this comment.
Added not.toContain suggestion
katakonst
force-pushed
the
feature/prefer-to-contain
branch
2 times, most recently
from
8227f3e to
14bb2a1
Compare
docs/rules/prefer-to-contain.md
Outdated
| ``` | ||
|
|
||
| ```js | ||
| expect(a).toContain(b); |
rules/prefer-to-contain.js
Outdated
| node.arguments[0].arguments.length === 1 && | ||
| node.arguments[0].arguments[0] | ||
| ) { | ||
| const sourceCode = context.getSourceCode(); |
There was a problem hiding this comment.
could we move all these variables into the the fix closure to avoid the hit of calculating them unless people call fix?
| create(context) { | ||
| return { | ||
| CallExpression(node) { | ||
| if ( |
There was a problem hiding this comment.
this if is really verbose, could you clean it up? Not a bigge (we can do it later), but 15 lines seems excessive
There was a problem hiding this comment.
Broke it in multiple methods
rules/prefer-to-contain.js
Outdated
| CallExpression(node) { | ||
| if ( | ||
| !( | ||
| expectNotCase(node) || |
There was a problem hiding this comment.
what about e.g. expect(a.includes(b)).not.toEqual(true);? might be weird enough that we don't need to support it, I guess 🙂
There was a problem hiding this comment.
It'a a little weird but it's a nice to have. Added it to the rule!
katakonst
force-pushed
the
feature/prefer-to-contain
branch
2 times, most recently
from
8335e71 to
254bca7
Compare
rules/prefer-to-contain.js
Outdated
| return getNegationFixes(node, sourceCode, fixer).concat(fixArr); | ||
| } | ||
|
|
||
| const equalityArgument = isEqualityNegation(node) |
There was a problem hiding this comment.
CI is complaining that this line is not covered by tests. Mind adding a test for the other case (or removing the ternary)?
There was a problem hiding this comment.
It was not used so removed it, the coverage is fine now.
katakonst
force-pushed
the
feature/prefer-to-contain
branch
from
254bca7 to
9358170
Compare
|
🎉 This PR is included in version 21.25.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #100