-
Notifications
You must be signed in to change notification settings - Fork 1.8k
WIP: add unnecessary_split_off lint #14814
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| use clippy_utils::diagnostics::span_lint_and_sugg; | ||
| use clippy_utils::{is_integer_const, sym, ty}; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{Expr, ExprKind}; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_session::declare_lint_pass; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Suggests using `drain(..).collect()` when a `split_off(0)` is being called on a `vec`. | ||
| /// ### Why is this bad? | ||
| /// Because splitting implies dividing the vec into two parts, so the modified vector being emptied could be unexpected. | ||
| /// ### Example | ||
| /// ```no_run | ||
| /// let mut vec = vec![1, 2, 3]; | ||
| /// let vec1 = vec.split_off(0); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```no_run | ||
| /// let mut vec = vec![1, 2, 3]; | ||
| /// let vec1 = vec.drain(..).collect(); | ||
| /// ``` | ||
| #[clippy::version = "1.88.0"] | ||
| pub UNNECESSARY_SPLIT_OFF, | ||
| style, | ||
| "unnecessary `split_off(0)`" | ||
| } | ||
| declare_lint_pass!(UnnecessarySplitOff => [UNNECESSARY_SPLIT_OFF]); | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for UnnecessarySplitOff { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { | ||
| if let ExprKind::MethodCall(path, value, args, span) = &expr.kind | ||
| && path.ident.name == sym::split_off | ||
| { | ||
| let ty = cx.typeck_results().expr_ty(value); | ||
| if ty::is_type_diagnostic_item(cx, ty, sym::Vec) { | ||
| let &[arg] = args else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could do this check already when matching the method call: -if let ExprKind::MethodCall(path, value, args, span) = &expr.kind
+if let ExprKind::MethodCall(path, value, [arg], span) = &expr.kind |
||
| return; | ||
| }; | ||
| if is_integer_const(cx, arg, 0) { | ||
|
Comment on lines
+32
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this whole thing could be one big let-chain, which would reduce indentation |
||
| span_lint_and_sugg( | ||
| cx, | ||
| UNNECESSARY_SPLIT_OFF, | ||
| *span, | ||
| "unnecessary `split_off(0)`", | ||
| "use", | ||
| "drain(..).collect()".to_string(), | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| //@no-rustfix | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why? |
||
| #![warn(clippy::unnecessary_split_off)] | ||
| #![allow(unused)] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is added implicitly already, so you don't need to specify it yourself |
||
|
|
||
| struct A; | ||
| impl A { | ||
| fn split_off(&mut self, _: usize) {} | ||
| } | ||
|
|
||
| fn main() { | ||
| let mut vec1 = vec![1, 2, 3]; | ||
|
|
||
| let vec2: Vec<_> = vec1.split_off(0); | ||
| //~^ unnecessary_split_off | ||
|
|
||
| let vec3: Vec<_> = vec1.split_off(1); | ||
|
|
||
| const ZERO: usize = 0; | ||
| let vec4: Vec<_> = vec1.split_off(ZERO); | ||
| //~^ unnecessary_split_off | ||
|
|
||
| let vec5: Vec<_> = vec1.split_off(const { 0 }); | ||
| //~^ unnecessary_split_off | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also like to test let zero = 0;
let vec6 = vec1.split_off(zero); |
||
| let zero = 0; | ||
| let vec6: Vec<_> = vec1.split_off(zero); | ||
| //~^ unnecessary_split_off | ||
|
|
||
| let mut a = A; | ||
| a.split_off(0); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| error: unnecessary `split_off(0)` | ||
| --> tests/ui/unnecessary_split_off.rs:15:29 | ||
| | | ||
| LL | let vec2: Vec<_> = vec1.split_off(0); | ||
| | ^^^^^^^^^^^^ help: use: `drain(..).collect()` | ||
| | | ||
| = note: `-D clippy::unnecessary-split-off` implied by `-D warnings` | ||
| = help: to override `-D warnings` add `#[allow(clippy::unnecessary_split_off)]` | ||
|
|
||
| error: unnecessary `split_off(0)` | ||
| --> tests/ui/unnecessary_split_off.rs:20:29 | ||
| | | ||
| LL | let vec4: Vec<_> = vec1.split_off(ZERO); | ||
| | ^^^^^^^^^^^^^^^ help: use: `drain(..).collect()` | ||
|
|
||
| error: unnecessary `split_off(0)` | ||
| --> tests/ui/unnecessary_split_off.rs:23:29 | ||
| | | ||
| LL | let vec5: Vec<_> = vec1.split_off(const { 0 }); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^ help: use: `drain(..).collect()` | ||
|
|
||
| error: aborting due to 3 previous errors | ||
|
|
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 lint could be in the
methods/group, which already has some boilerplate for parsing method calls