From 26cb792bbaae2079bd6b438fc883346bd436501f Mon Sep 17 00:00:00 2001 From: Lana Abdulla Date: Mon, 10 Oct 2022 19:49:08 +0900 Subject: [PATCH 1/3] add unnecessary_vec_drain lint Co-authored-by: Alexander Eklund --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_style.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/unnecessary_vec_drain.rs | 97 +++++++++++++++++++++++ src/docs.rs | 1 + src/docs/unnecessary_vec_drain.txt | 16 ++++ tests/ui/range_plus_minus_one.fixed | 1 + tests/ui/range_plus_minus_one.rs | 1 + tests/ui/range_plus_minus_one.stderr | 18 ++--- tests/ui/unnecessary_vec_drain.rs | 12 +++ tests/ui/unnecessary_vec_drain.stderr | 16 ++++ 13 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 clippy_lints/src/unnecessary_vec_drain.rs create mode 100644 src/docs/unnecessary_vec_drain.txt create mode 100644 tests/ui/unnecessary_vec_drain.rs create mode 100644 tests/ui/unnecessary_vec_drain.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index bfeb009727b5..545814a1d54a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4296,6 +4296,7 @@ Released 2018-09-13 [`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by [`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 +[`unnecessary_vec_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_vec_drain [`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 642d7070fb6b..b10f35b1ae17 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -345,6 +345,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS), LintId::of(unnamed_address::VTABLE_ADDRESS_COMPARISONS), LintId::of(unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS), + LintId::of(unnecessary_vec_drain::UNNECESSARY_VEC_DRAIN), LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(unused_unit::UNUSED_UNIT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 08eb6c2b39bc..d9c6d0ed9e72 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -581,6 +581,7 @@ store.register_lints(&[ unnamed_address::VTABLE_ADDRESS_COMPARISONS, unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS, unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS, + unnecessary_vec_drain::UNNECESSARY_VEC_DRAIN, unnecessary_wraps::UNNECESSARY_WRAPS, unnested_or_patterns::UNNESTED_OR_PATTERNS, unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index 6d5483bc7010..6ec71a579393 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -117,6 +117,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME), LintId::of(unit_types::LET_UNIT_VALUE), LintId::of(unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS), + LintId::of(unnecessary_vec_drain::UNNECESSARY_VEC_DRAIN), LintId::of(unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(unused_unit::UNUSED_UNIT), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e5d26ac22a74..0bdecb0deebe 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -379,6 +379,7 @@ mod unit_types; mod unnamed_address; mod unnecessary_owned_empty_strings; mod unnecessary_self_imports; +mod unnecessary_vec_drain; mod unnecessary_wraps; mod unnested_or_patterns; mod unsafe_removed_from_name; @@ -906,6 +907,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(bool_to_int_with_if::BoolToIntWithIf)); store.register_late_pass(|_| Box::new(box_default::BoxDefault)); store.register_late_pass(|_| Box::new(implicit_saturating_add::ImplicitSaturatingAdd)); + store.register_late_pass(|_| Box::new(unnecessary_vec_drain::UnnecessaryVecDrain)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/unnecessary_vec_drain.rs b/clippy_lints/src/unnecessary_vec_drain.rs new file mode 100644 index 000000000000..7a6b7f98ca36 --- /dev/null +++ b/clippy_lints/src/unnecessary_vec_drain.rs @@ -0,0 +1,97 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; +use rustc_ast::LitKind::Int; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::{LangItem, Stmt}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `vec.drain(..)` with RangeFull that is not bound to a value `let`. + /// + /// ### Why is this bad? + /// This creates an iterator that is dropped immediately. + /// + /// `vec.clear()` also shows clearer intention. + /// + /// ### Example + /// ```rust + /// let mut vec: Vec = Vec::new(); + // vec.drain(..); + /// ``` + /// Use instead: + /// ```rust + /// let mut vec: Vec = Vec::new(); + // vec.clear(); + /// ``` + #[clippy::version = "1.66.0"] + pub UNNECESSARY_VEC_DRAIN, + style, + "unnecessary `vec.drain(..)`" +} +declare_lint_pass!(UnnecessaryVecDrain => [UNNECESSARY_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 + && path.ident.name == sym!(drain) + { + let ty = cx.typeck_results().expr_ty(rcvr); + if let [expr_element] = &**method_args + && is_type_diagnostic_item(cx, ty, sym::Vec) + { + let ty = cx.typeck_results().expr_ty(expr_element); + if is_type_lang_item(cx, ty, LangItem::RangeFull) + { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + UNNECESSARY_VEC_DRAIN, + semi_expr.span, + "unnecessary iterator `Drain` is created and dropped immedietly", + "consider calling `clear()`", + format!("{}.clear()", snippet_with_applicability(cx, rcvr.span, "", &mut applicability)), + applicability + ); + } + if let hir::ExprKind::Struct(_, expr_fields, _) = &expr_element.kind + && is_type_lang_item(cx, ty, LangItem::Range) + { + if_chain! { + if let hir::ExprKind::Lit(lit) = &expr_fields[0].expr.kind; + if let hir::ExprKind::MethodCall(path_seg, expr, _, _) = expr_fields[1].expr.kind; + + if let hir::hir_id::HirId{owner: owner_expr,..} = &expr.hir_id; + if let hir::hir_id::HirId{owner: owner_rcvr,..} = &rcvr.hir_id; + + then { + if let Int(start,_) = lit.node + && path_seg.ident.name == sym!(len) + && owner_rcvr == owner_expr + && start == 0 + { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + UNNECESSARY_VEC_DRAIN, + *drain_span, + "unnecessary iterator `Drain` is created and dropped immediately", + "consider calling `clear()`", + format!("{}.clear()", snippet_with_applicability(cx, rcvr.span, "", &mut applicability)), + applicability + ); + + } + } + } + } + } + } + } + } +} diff --git a/src/docs.rs b/src/docs.rs index 1b727e1a1004..c41697c43517 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -542,6 +542,7 @@ docs! { "unnecessary_sort_by", "unnecessary_to_owned", "unnecessary_unwrap", + "unnecessary_vec_drain", "unnecessary_wraps", "unneeded_field_pattern", "unneeded_wildcard_pattern", diff --git a/src/docs/unnecessary_vec_drain.txt b/src/docs/unnecessary_vec_drain.txt new file mode 100644 index 000000000000..8ee6b9dd2371 --- /dev/null +++ b/src/docs/unnecessary_vec_drain.txt @@ -0,0 +1,16 @@ +### What it does +Checks for usage of `vec.drain(..)` with RangeFull that is not bound to a value `let`. + +### Why is this bad? +This creates an iterator that is dropped immediately. + +`vec.clear()` also shows clearer intention. + +### Example +``` +let mut vec: Vec = Vec::new(); +``` +Use instead: +``` +let mut vec: Vec = Vec::new(); +``` \ No newline at end of file diff --git a/tests/ui/range_plus_minus_one.fixed b/tests/ui/range_plus_minus_one.fixed index a16a3e54d45e..e4d2f47c16ea 100644 --- a/tests/ui/range_plus_minus_one.fixed +++ b/tests/ui/range_plus_minus_one.fixed @@ -2,6 +2,7 @@ #![allow(unused_parens)] #![allow(clippy::iter_with_drain)] +#![allow(clippy::unnecessary_vec_drain)] fn f() -> usize { 42 } diff --git a/tests/ui/range_plus_minus_one.rs b/tests/ui/range_plus_minus_one.rs index bd6cb4d21be5..f468d432305e 100644 --- a/tests/ui/range_plus_minus_one.rs +++ b/tests/ui/range_plus_minus_one.rs @@ -2,6 +2,7 @@ #![allow(unused_parens)] #![allow(clippy::iter_with_drain)] +#![allow(clippy::unnecessary_vec_drain)] fn f() -> usize { 42 } diff --git a/tests/ui/range_plus_minus_one.stderr b/tests/ui/range_plus_minus_one.stderr index 0223696243b2..7550a85fddd1 100644 --- a/tests/ui/range_plus_minus_one.stderr +++ b/tests/ui/range_plus_minus_one.stderr @@ -1,5 +1,5 @@ error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:31:14 + --> $DIR/range_plus_minus_one.rs:32:14 | LL | for _ in 0..3 + 1 {} | ^^^^^^^^ help: use: `0..=3` @@ -7,25 +7,25 @@ LL | for _ in 0..3 + 1 {} = note: `-D clippy::range-plus-one` implied by `-D warnings` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:34:14 + --> $DIR/range_plus_minus_one.rs:35:14 | LL | for _ in 0..1 + 5 {} | ^^^^^^^^ help: use: `0..=5` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:37:14 + --> $DIR/range_plus_minus_one.rs:38:14 | LL | for _ in 1..1 + 1 {} | ^^^^^^^^ help: use: `1..=1` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:43:14 + --> $DIR/range_plus_minus_one.rs:44:14 | LL | for _ in 0..(1 + f()) {} | ^^^^^^^^^^^^ help: use: `0..=f()` error: an exclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:47:13 + --> $DIR/range_plus_minus_one.rs:48:13 | LL | let _ = ..=11 - 1; | ^^^^^^^^^ help: use: `..11` @@ -33,25 +33,25 @@ LL | let _ = ..=11 - 1; = note: `-D clippy::range-minus-one` implied by `-D warnings` error: an exclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:48:13 + --> $DIR/range_plus_minus_one.rs:49:13 | LL | let _ = ..=(11 - 1); | ^^^^^^^^^^^ help: use: `..11` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:49:13 + --> $DIR/range_plus_minus_one.rs:50:13 | LL | let _ = (1..11 + 1); | ^^^^^^^^^^^ help: use: `(1..=11)` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:50:13 + --> $DIR/range_plus_minus_one.rs:51:13 | LL | let _ = (f() + 1)..(f() + 1); | ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())` error: an inclusive range would be more readable - --> $DIR/range_plus_minus_one.rs:54:14 + --> $DIR/range_plus_minus_one.rs:55:14 | LL | for _ in 1..ONE + ONE {} | ^^^^^^^^^^^^ help: use: `1..=ONE` diff --git a/tests/ui/unnecessary_vec_drain.rs b/tests/ui/unnecessary_vec_drain.rs new file mode 100644 index 000000000000..c9c44125e94a --- /dev/null +++ b/tests/ui/unnecessary_vec_drain.rs @@ -0,0 +1,12 @@ +#![allow(unused)] +#![warn(clippy::unnecessary_vec_drain)] + +fn main() { + let mut vec: Vec = Vec::new(); + //Lint + vec.drain(..); + vec.drain(0..vec.len()); + + // Dont Lint + let iter = vec.drain(..); +} diff --git a/tests/ui/unnecessary_vec_drain.stderr b/tests/ui/unnecessary_vec_drain.stderr new file mode 100644 index 000000000000..20f64cd7e1f9 --- /dev/null +++ b/tests/ui/unnecessary_vec_drain.stderr @@ -0,0 +1,16 @@ +error: unnecessary iterator `Drain` is created and dropped immedietly + --> $DIR/unnecessary_vec_drain.rs:7:5 + | +LL | vec.drain(..); + | ^^^^^^^^^^^^^ help: consider calling `clear()`: `vec.clear()` + | + = note: `-D clippy::unnecessary-vec-drain` implied by `-D warnings` + +error: unnecessary iterator `Drain` is created and dropped immediately + --> $DIR/unnecessary_vec_drain.rs:8:9 + | +LL | vec.drain(0..vec.len()); + | ^^^^^^^^^^^^^^^^^^^ help: consider calling `clear()`: `vec.clear()` + +error: aborting due to 2 previous errors + From 913e819d91825b5a7e991f10f3487c90b15dd4c6 Mon Sep 17 00:00:00 2001 From: Lana Abdulla Date: Mon, 10 Oct 2022 19:49:08 +0900 Subject: [PATCH 2/3] add unnecessary_vec_drain lint Co-authored-by: Alexander Eklund --- clippy_lints/src/unnecessary_vec_drain.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/unnecessary_vec_drain.rs b/clippy_lints/src/unnecessary_vec_drain.rs index 7a6b7f98ca36..eae32857fa76 100644 --- a/clippy_lints/src/unnecessary_vec_drain.rs +++ b/clippy_lints/src/unnecessary_vec_drain.rs @@ -64,9 +64,9 @@ impl LateLintPass<'_> for UnnecessaryVecDrain { { if_chain! { if let hir::ExprKind::Lit(lit) = &expr_fields[0].expr.kind; - if let hir::ExprKind::MethodCall(path_seg, expr, _, _) = expr_fields[1].expr.kind; + if let hir::ExprKind::MethodCall(path_seg, vec_expr, _, _) = expr_fields[1].expr.kind; - if let hir::hir_id::HirId{owner: owner_expr,..} = &expr.hir_id; + if let hir::hir_id::HirId{owner: owner_expr,..} = &vec_expr.hir_id; if let hir::hir_id::HirId{owner: owner_rcvr,..} = &rcvr.hir_id; then { @@ -85,7 +85,6 @@ impl LateLintPass<'_> for UnnecessaryVecDrain { format!("{}.clear()", snippet_with_applicability(cx, rcvr.span, "", &mut applicability)), applicability ); - } } } From 5add94907b4a1a6522fcf2d97fc13c0c39244332 Mon Sep 17 00:00:00 2001 From: Lana Abdulla Date: Mon, 10 Oct 2022 19:49:08 +0900 Subject: [PATCH 3/3] add unnecessary_vec_drain lint Co-authored-by: Alexander Eklund --- clippy_lints/src/unnecessary_vec_drain.rs | 4 ++-- src/docs/unnecessary_vec_drain.txt | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/unnecessary_vec_drain.rs b/clippy_lints/src/unnecessary_vec_drain.rs index eae32857fa76..9535e479753e 100644 --- a/clippy_lints/src/unnecessary_vec_drain.rs +++ b/clippy_lints/src/unnecessary_vec_drain.rs @@ -21,12 +21,12 @@ declare_clippy_lint! { /// ### Example /// ```rust /// let mut vec: Vec = Vec::new(); - // vec.drain(..); + /// vec.drain(..); /// ``` /// Use instead: /// ```rust /// let mut vec: Vec = Vec::new(); - // vec.clear(); + /// vec.clear(); /// ``` #[clippy::version = "1.66.0"] pub UNNECESSARY_VEC_DRAIN, diff --git a/src/docs/unnecessary_vec_drain.txt b/src/docs/unnecessary_vec_drain.txt index 8ee6b9dd2371..b65d4d75d82e 100644 --- a/src/docs/unnecessary_vec_drain.txt +++ b/src/docs/unnecessary_vec_drain.txt @@ -9,8 +9,10 @@ This creates an iterator that is dropped immediately. ### Example ``` let mut vec: Vec = Vec::new(); + vec.drain(..); ``` Use instead: ``` let mut vec: Vec = Vec::new(); +vec.clear(); ``` \ No newline at end of file