-
-
Notifications
You must be signed in to change notification settings - Fork 471
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
feat(linter): support useGuardForIn #4104
Conversation
CodSpeed Performance ReportMerging #4104 will not alter performanceComparing Summary
|
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.
The rule must be called useGuardForIn
6cb63b9
to
649c7d2
Compare
sorry i miss the rule name convention for biome, i update the lint name now, please review again when you free~ @ematipico |
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.
The code might be good, but unfortunately the documentation is nearly absent, and the diagnostics should provide more information to the user.
crates/biome_js_analyze/tests/specs/nursery/useGuardForIn/invalid.js.snap
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/tests/specs/nursery/useGuardForIn/invalid.js.snap
Outdated
Show resolved
Hide resolved
47242b3
to
0c0be67
Compare
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.
Shouldn't we add cases where we have a for..in
loop with Object.hasOwn(foo, key)
, for example?
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.
updated~ thanks for review
2fca630
to
f6d18bb
Compare
8e936f3
to
2cfc0ee
Compare
i solve the conflict at my local, maybe the mr can be merged later? |
2cfc0ee
to
06b5bbd
Compare
06b5bbd
to
3dc2528
Compare
Summary
closes: #3933
Test Plan