This repository has been archived by the owner on Aug 31, 2023. It is now read-only.
feat(rome_js_analyze): useExponentiationOperator rule
#3848
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaioduarte
requested review from
leops,
ematipico and
xunilrj
as code owners
✅ Deploy Preview for docs-rometools ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
kaioduarte
changed the title
useExponentiation ruleuseExponentiationOperator rule
|
@kaioduarte considering that this is an initial implementation, would it be possible to create an issue where you can track works that need to be done for this rule? |
4 tasks
|
@ematipico here it is #3850 |
MichaReiser
reviewed
Nov 25, 2022
Comment on lines
69
to
132
| let has_math = static_member_expr | ||
| .object() | ||
| .ok()? | ||
| .omit_parentheses() | ||
| .as_reference_identifier()? | ||
| .has_name("Math"); | ||
| let has_pow = static_member_expr | ||
| .member() | ||
| .ok()? | ||
| .as_js_name()? | ||
| .value_token() | ||
| .ok()? | ||
| .token_text_trimmed() | ||
| == "pow"; | ||
|
|
||
| if has_math && has_pow { | ||
| return Some(()); | ||
| } |
There was a problem hiding this comment.
@xunilrj What's your take: Would it be faster to instead use the semantic model to get the global Math symbol and then find all references to it instead of running the rule on each static member expression?
There was a problem hiding this comment.
Yes. But we would need to be sure that the context has Math configured as global.
There was a problem hiding this comment.
@xunilrj do we have any rule that does that check?
There was a problem hiding this comment.
Not exactly. Let us leave the rule as it is, and make this change later as an optimization. Like we did for https://github.com/rome/tools/blob/main/crates/rome_js_analyze/src/semantic_analyzers/correctness/no_arguments.rs
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
kaioduarte
force-pushed
the
feat/use-exponentiation
branch
from
a6799dd
to
513189e
Compare
kaioduarte
commented
Nov 27, 2022
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Show resolved
Hide resolved
kaioduarte
force-pushed
the
feat/use-exponentiation
branch
from
aacc6da
to
87bca70
Compare
MichaReiser
approved these changes
Dec 2, 2022
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
crates/rome_js_analyze/src/analyzers/nursery/use_exponentiation_operator.rs
Outdated
Show resolved
Hide resolved
leops
approved these changes
Dec 5, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds an initial implementation of this ESLint rule prefer-exponentiation-operator. There are some edge cases to be handled. For realistic usage, it looks fine.
Test Plan
cargo test -p rome_js_analyze -- use_exponentiation_operator