add [`unnecessary_vec_drain] lint #9623
Conversation
Co-authored-by: Alexander Eklund <[email protected]>
Co-authored-by: Alexander Eklund <[email protected]>
Co-authored-by: Alexander Eklund <[email protected]>
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #9658) made this pull request unmergeable. Please resolve the merge conflicts. |
| /// ### Example | ||
| /// ```rust | ||
| /// let mut vec: Vec<i32> = Vec::new(); | ||
| /// vec.drain(..); |
There was a problem hiding this comment.
| /// vec.drain(..); | |
| /// vec.drain(..); |
| /// vec.clear(); | ||
| /// ``` | ||
| #[clippy::version = "1.66.0"] | ||
| pub UNNECESSARY_VEC_DRAIN, |
There was a problem hiding this comment.
Please rename it to
| pub UNNECESSARY_VEC_DRAIN, | |
| pub NEEDLESS_VEC_DRAIN, |
| impl LateLintPass<'_> for UnnecessaryVecDrain { | ||
| fn check_stmt<'tcx>(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) { | ||
| if let hir::StmtKind::Semi(semi_expr) = &stmt.kind { | ||
| if let hir::ExprKind::MethodCall(path, rcvr, method_args,drain_span) = &semi_expr.kind |
There was a problem hiding this comment.
We abbreviate receiver with recv usually. Would be great if you could do this also in this lint impl.
| && path.ident.name == sym!(drain) | ||
| { | ||
| let ty = cx.typeck_results().expr_ty(rcvr); | ||
| if let [expr_element] = &**method_args |
There was a problem hiding this comment.
You can pull this into the check above and already match for the single argument in methods_args.
| { | ||
| let ty = cx.typeck_results().expr_ty(rcvr); | ||
| if let [expr_element] = &**method_args | ||
| && is_type_diagnostic_item(cx, ty, sym::Vec) |
There was a problem hiding this comment.
You're combining if let chains with the if_chain macro. Please use if let chains everywhere.
You can also use irrefutable patterns in if let chains, so this can be all in one chain up until here.
| if let Int(start,_) = lit.node | ||
| && path_seg.ident.name == sym!(len) | ||
| && owner_rcvr == owner_expr | ||
| && start == 0 |
There was a problem hiding this comment.
NIT: The order of these checks are a bit weird. This check should be done right after matching it to ExprKind::Lit.
| LL | vec.drain(0..vec.len()); | ||
| | ^^^^^^^^^^^^^^^^^^^ help: consider calling `clear()`: `vec.clear()` |
There was a problem hiding this comment.
This suggestion won't work. The code after applying this suggestion would be vec.vec.clear(). You'll have to fix the span your using when producing this suggestion to also include vec.
flip1995
added
S-waiting-on-author
fixes #9339
detects usage of drain with RangeFull and dropped iterator
will suggest to change to
changelog: New lint: [
unnecessary_vec_drain]#9623