-
Notifications
You must be signed in to change notification settings - Fork 653
feat(rome_js_analyzer): implement rule noRedeclaration, no-redeclare #4053
Conversation
✅ Deploy Preview for docs-rometools canceled.Built without sensitive environment variables
|
type Options = (); | ||
|
||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let mut duplicates = Duplicates::default(); |
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.
Nit: Not sure there is value hiding that this is a HashMap here.
let mut duplicates = HashMap::default();
seems clearer to me.
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
@edvardchen could you please comment on #3990 so I can assign that to you? |
let model = ctx.model(); | ||
let bindings = model.scope(ctx.query().syntax()).bindings(); | ||
for binding in bindings { | ||
let name = binding.tree().text(); |
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.
Nit: you could use binding.tree().name_token().ok()?.text_trimmed()
to use an &str
as key and avoid allocating a String
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs
Outdated
Show resolved
Hide resolved
This PR is stale because it has been open 14 days with no activity. |
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.
It seems we addressed all the comments. We can rebase the branch, generate the files again and merge it! 🚀
@edvardchen Thanks for your contribution :) In order to merge your contribution, you have to rebase your branch on main in order to solve the current conflicts. |
feat(rome_js_analyze): implement rule no_redeclaration feat(rome_js_analyze): update document about noRedeclaration feat(rome_js_analyze): link the first binding in diagnostic for rule no_redecleration test(rome_js_analyze): add testcases for no_redeclaration fix(rome_js_analyze): fix lint error docs(rome_js_analyze): update document feat(rome_js_analyze): don't handle shadow globals in no_redeclaration feat(rome_js_analyze): create scope for static initialization blocks feat(rome_js_analyze): remove duplicated warnings for rule no_redeclaration chore(rome_js_analyze): rename the union type in no_redeclaration docs(rome_js_analyze): add suggestions for no_redeclaration refactor(rome_js_analyze): iterate all scopes for no_redeclaration fix(rome_js_semantic): fix duplicated patterns
5d67920
to
9c21cbe
Compare
Summary
implement noRedeclaration https://eslint.org/docs/latest/rules/no-redeclare. Fixes #3990
Test Plan
have run
just codegen
just test-lintrule no_redeclaration
just documentation
just check-ready
Documentation
Caveats