-
-
Notifications
You must be signed in to change notification settings - Fork 503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(lint): add rule useAtIndex
#4120
Conversation
CodSpeed Performance ReportMerging #4120 will improve performances by 7.04%Comparing Summary
Benchmarks breakdown
|
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.
Thanks for your contribution! I left my first comments. I will make a second pass once these comments are addressed.
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
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.
Once the comments are addressed I think we can merge your contribution :)
) -> Option<(AnyJsExpression, Vec<AnyJsExpression>)> { | ||
let mut right_list = vec![]; |
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.
We could avoid allocating a Vec
by using std::iter::successors.
We could clone the iterator at call site if we need to iterate over it several times.
This is a refactor that we can delay to a future PR.
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.
Sorry. I didn't know the appropriate way to use successors
while simultaneously obtaining the final expression
.
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
@GunseiKPaseri Looks good to me. Could you merge main in your PR and solve conflicts? This will allow us to merge your PR :) |
Summary
Implement useAtIndex(unicorn/prefer-at)
closes #3943
Making some slight changes.
X.slice(-1, unknown)[0]
foo[foo.length - a - b]
lodash
checkAllIndexAccess
,getLastElementFunctions
)Test Plan