feat(rome_js_analyze): noSelfCompare rule
#4031
Conversation
✅ Deploy Preview for docs-rometools ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@kaioduarte could you please comment on the issue? Doing so, I can assign the issue to you. This will help showing that the issue is being assigned already |
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
a2a5e41 to
53ed581
Compare
| ``` | ||
|
|
||
| ``` | ||
| noSelfCompare.js:26:1 lint/nursery/noSelfCompare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ |
There was a problem hiding this comment.
I am not a huge fan of these cases where the expression is very complex.
Not only the expression can have side effects, it is also very hard to known what "self" is, to say that we are comparing something with itself.
Makes sense to short circuit the comparison in problematic nodes like function calls?
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
Outdated
Show resolved
Hide resolved
| /// Verifies that both nodes are equal by checking their descendants (nodes included) kinds | ||
| /// and tokens (same kind and inner token text). |
There was a problem hiding this comment.
should we mark these as equal nodes?
a.b.c === a?.b.cthey have the same meaning in the end
There was a problem hiding this comment.
Doesn't the first one throw? and the second no?
If we remove this expression, a function can stop throwing.
Maybe this is a good thing, but we are changing behaviour.
There was a problem hiding this comment.
Yeah, in that sense they are different, but not if we only consider the identifiers chain. Anyway, maybe it's fine to keep it as is.
8b76a4d to
8162104
Compare
Summary
Closes #3982.
Test Plan
cargo test -p rome_js_analyze -- no_self_compare