-
Notifications
You must be signed in to change notification settings - Fork 653
feat(rome_js_analyzer): add noPositiveTabindex
rule
#3336
Conversation
✅ Deploy Preview for docs-rometools ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
I'm opening a draft PR so we can discuss the implementation.
While implementing the rule, I noticed that the helper inner_string_text
couldn't strip the quotes on JsxSelfClosingElement
. Can you confirm this is working as intended, or is it a bug?
The same also happens when getting the value for JsNumberLiteralExpression
and JsStringLiteralExpression
on React.createElement
.
Any help is appreciated!
crates/rome_js_analyze/tests/specs/nursery/noPositiveTabindex/invalidJsx.jsx
Outdated
Show resolved
Hide resolved
50e5749
to
88fed27
Compare
f380389
to
85bd430
Compare
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
85bd430
to
c328cd5
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.
The implementation is way better now! I left few more suggestions, and we should fix the incorrectness of the diagnostic.
Also, it seems there are some conflicts to solve
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/tests/specs/nursery/noPositiveTabindex/invalidJsx.jsx.snap
Show resolved
Hide resolved
crates/rome_js_analyze/src/semantic_analyzers/nursery/no_positive_tabindex.rs
Outdated
Show resolved
Hide resolved
e6b61b5
to
29a7f19
Compare
sorry, forgot to update doc files 😞 |
No worries. To fix the CI issue, you need to run |
c849448
to
7995651
Compare
There was a change right after my last rebase that I missed, hopefully it will be green now 🤞🏻💚 |
…ive_tabindex.rs Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
…ive_tabindex.rs Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
…ive_tabindex.rs Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
…ive_tabindex.rs Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
7995651
to
2e2ea17
Compare
noPositiveTabindex
rule
noPositiveTabindex
rulenoPositiveTabindex
rule
Summary
Closes #3304
Test Plan