Skip to content

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6399,6 +6399,7 @@ Released 2018-09-13
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
[`unnecessary_semicolon`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
[`unnecessary_split_off`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_split_off
[`unnecessary_struct_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct_initialization
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_semicolon::UNNECESSARY_SEMICOLON_INFO,
crate::unnecessary_split_off::UNNECESSARY_SPLIT_OFF_INFO,
crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
crate::unneeded_struct_pattern::UNNEEDED_STRUCT_PATTERN_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ mod unnecessary_map_on_constructor;
mod unnecessary_owned_empty_strings;
mod unnecessary_self_imports;
mod unnecessary_semicolon;
mod unnecessary_split_off;
mod unnecessary_struct_initialization;
mod unnecessary_wraps;
mod unneeded_struct_pattern;
Expand Down Expand Up @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap));
store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix));
store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
store.register_late_pass(|_| Box::new(unnecessary_split_off::UnnecessarySplitOff));
// add lints here, do not remove this comment, it's used in `new_lint`
}
55 changes: 55 additions & 0 deletions clippy_lints/src/unnecessary_split_off.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::sym;
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
// FIXME: sym::split_off does not exist, but this still triggers the lint to use it.
&& path.ident.name.as_str() == "split_off"
{
let ty = cx.typeck_results().expr_ty(value);
if clippy_utils::ty::is_type_diagnostic_item(cx, ty, sym::Vec) {
let &[arg] = args else {
return;
};
if clippy_utils::is_integer_literal(arg, 0) || clippy_utils::is_integer_const(cx, arg, 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter should be enough. Each literal is also a const

span_lint_and_sugg(
cx,
UNNECESSARY_SPLIT_OFF,
*span,
"unnecessary `split_off(0)`",
"use",
"drain(..).collect()".to_string(),
Applicability::MachineApplicable,
);
}
}
}
}
}
28 changes: 28 additions & 0 deletions tests/ui/unnecessary_split_off.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//@no-rustfix
#![warn(clippy::unnecessary_split_off)]
#![allow(unused)]

struct A;
impl A {
fn split_off(&mut self, _: usize) {}
}

const ZERO: usize = 0;

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);

let vec4: Vec<_> = vec1.split_off(ZERO);
//~^ unnecessary_split_off

let vec5: Vec<_> = vec1.split_off(const { 0 });
//~^ unnecessary_split_off

Copy link
Contributor

Choose a reason for hiding this comment

The 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 mut a = A;
a.split_off(0);
}
23 changes: 23 additions & 0 deletions tests/ui/unnecessary_split_off.stderr
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

Loading