-
Notifications
You must be signed in to change notification settings - Fork 1.8k
new lint: unnecessary_indexing
#14058
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
base: master
Are you sure you want to change the base?
Conversation
/// Use instead: | ||
/// ```no_run | ||
/// let a: &[i32] = &[1]; | ||
/// if let Some(b) = a.first() { |
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.
Note that you can generate even simplier expressions:
- If
a
is a slice ref, use:if let [b, ..] = a
- If
a
is an array, use:if let [b, ..] = &a
- If
a
derefs to a slice, use:if let [b, ..] = &a[..]
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 can add it as an alternate suggestion, which one is more preferable though?
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.
They're in order of preference. a
is the simplest, &a
is needed if a
is [T]
or [T; _]
, and &a[..]
is needed for everything else.
This comment has been minimized.
This comment has been minimized.
Ping @Alexendoo from triage. This is still waiting on a review. |
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 think also some of the points in #12464 still apply such as #12464 (comment)
let receiver = snippet(cx, receiver_span, ".."); | ||
let mut suggestions: Vec<(Span, String)> = vec![]; | ||
let mut message = "consider using `if..let` syntax instead of indexing".to_string(); |
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.
These can be moved into the lint callback
// span of the receiver for the index operation, only Some in the event the indexing is via a direct primitive | ||
index_receiver_span: Option<Span>, |
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 Option
here can be removed since process_indexing
already returns an Option<IndexCheckResult>
Ping @J-ZhengLi from triage. Do you plan to return to working on this? |
yes, sorry I'm in a family vacation😃, I'll get back in a few days |
do not lint after mutable reference is taken check path to local on conditional receiver, check mutable borrow after ref, do not lint on mutable auto borrow fix autoborrow/mutability checks remove unneded `extra_locals` inline if ststements; check locality earlier remove unnecessary impl on IndexCheckResult check for borrows in if block and change inner `Some` based on this
…f condition & lint messages etc.
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Lintcheck changes for 4ddddab
This comment will be updated if you push new changes |
closes: #11858
This is a continuation of #12464 (shout out to @Jacherr for their hard work)
Since they don't have time, I thought I could push this forward by adding another commit on top of it to fix some of the addressed issues.
r? Alexendoo
changelog: add new lint [
unnecessary_indexing
]