diff --git a/CHANGELOG.md b/CHANGELOG.md index da29804fad00..605866c44d0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4298,6 +4298,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 758f99360648..c8890f502179 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -347,6 +347,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 72715eddbb91..e71e8ef54429 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -583,6 +583,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 8e1390167dc8..ce65f510138e 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -118,6 +118,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 ebb0f14fef52..bf87f4b3761a 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; @@ -908,6 +909,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..9535e479753e --- /dev/null +++ b/clippy_lints/src/unnecessary_vec_drain.rs @@ -0,0 +1,96 @@ +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, vec_expr, _, _) = expr_fields[1].expr.kind; + + 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 { + 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 c75341860047..aa6ca624c9e9 100644 --- a/src/docs.rs +++ b/src/docs.rs @@ -544,6 +544,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..b65d4d75d82e --- /dev/null +++ b/src/docs/unnecessary_vec_drain.txt @@ -0,0 +1,18 @@ +### 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(); + vec.drain(..); +``` +Use instead: +``` +let mut vec: Vec = Vec::new(); +vec.clear(); +``` \ 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 +