+
Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyzer): add noPositiveTabindex rule #3336

Merged
merged 21 commits into from
Oct 11, 2022

Conversation

kaioduarte
Copy link
Contributor

@kaioduarte kaioduarte commented Oct 5, 2022

Summary

Closes #3304

Test Plan

image

image

@netlify
Copy link

netlify bot commented Oct 5, 2022

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 2e2ea17
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/634468c9f9aeb90008f18b83
😎 Deploy Preview https://deploy-preview-3336--docs-rometools.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Contributor Author

@kaioduarte kaioduarte left a 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!

@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch from 50e5749 to 88fed27 Compare October 5, 2022 21:18
@kaioduarte kaioduarte marked this pull request as ready for review October 5, 2022 22:19
@kaioduarte kaioduarte requested a review from a team October 5, 2022 22:19
@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch 2 times, most recently from f380389 to 85bd430 Compare October 6, 2022 08:11
@ematipico ematipico added the A-Linter Area: linter label Oct 6, 2022
@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch from 85bd430 to c328cd5 Compare October 9, 2022 12:40
Copy link
Contributor

@ematipico ematipico left a 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

@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch from e6b61b5 to 29a7f19 Compare October 10, 2022 10:07
@kaioduarte
Copy link
Contributor Author

sorry, forgot to update doc files 😞

@ematipico
Copy link
Contributor

sorry, forgot to update doc files 😞

No worries. To fix the CI issue, you need to run cargo codegen-configuration

@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch from c849448 to 7995651 Compare October 10, 2022 10:27
@kaioduarte
Copy link
Contributor Author

There was a change right after my last rebase that I missed, hopefully it will be green now 🤞🏻💚

@kaioduarte kaioduarte force-pushed the feat/no-positive-tabindex branch from 7995651 to 2e2ea17 Compare October 10, 2022 18:47
@ematipico ematipico changed the title feat(rome_analyzer): add noPositiveTabindex rule feat(rome_analyzer): add noPositiveTabindex rule Oct 11, 2022
@ematipico ematipico merged commit a4cc3b9 into rome:main Oct 11, 2022
@ematipico ematipico changed the title feat(rome_analyzer): add noPositiveTabindex rule feat(rome_js_analyzer): add noPositiveTabindex rule Oct 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

noPositiveTabIndex
3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载