diff --git a/clippy_lints/src/collection_is_never_read.rs b/clippy_lints/src/collection_is_never_read.rs index 10f2bef268a2..15a5b7dd2748 100644 --- a/clippy_lints/src/collection_is_never_read.rs +++ b/clippy_lints/src/collection_is_never_read.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; use clippy_utils::visitors::for_each_expr_with_closures; use clippy_utils::{get_enclosing_block, get_parent_node, path_to_local_id}; use core::ops::ControlFlow; -use rustc_hir::{Block, ExprKind, HirId, Local, Node, PatKind}; +use rustc_hir::{Block, ExprKind, HirId, LangItem, Local, Node, PatKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::sym; @@ -44,7 +44,8 @@ declare_clippy_lint! { } declare_lint_pass!(CollectionIsNeverRead => [COLLECTION_IS_NEVER_READ]); -static COLLECTIONS: [Symbol; 10] = [ +// Add `String` here when it is added to diagnostic items +static COLLECTIONS: [Symbol; 9] = [ sym::BTreeMap, sym::BTreeSet, sym::BinaryHeap, @@ -52,7 +53,6 @@ static COLLECTIONS: [Symbol; 10] = [ sym::HashSet, sym::LinkedList, sym::Option, - sym::String, sym::Vec, sym::VecDeque, ]; @@ -60,8 +60,7 @@ static COLLECTIONS: [Symbol; 10] = [ impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead { fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) { // Look for local variables whose type is a container. Search surrounding bock for read access. - let ty = cx.typeck_results().pat_ty(local.pat); - if COLLECTIONS.iter().any(|&sym| is_type_diagnostic_item(cx, ty, sym)) + if match_acceptable_type(cx, local, &COLLECTIONS) && let PatKind::Binding(_, local_id, _, _) = local.pat.kind && let Some(enclosing_block) = get_enclosing_block(cx, local.hir_id) && has_no_read_access(cx, local_id, enclosing_block) @@ -71,6 +70,13 @@ impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead { } } +fn match_acceptable_type(cx: &LateContext<'_>, local: &Local<'_>, collections: &[rustc_span::Symbol]) -> bool { + let ty = cx.typeck_results().pat_ty(local.pat); + collections.iter().any(|&sym| is_type_diagnostic_item(cx, ty, sym)) + // String type is a lang item but not a diagnostic item for now so we need a separate check + || is_type_lang_item(cx, ty, LangItem::String) +} + fn has_no_read_access<'tcx>(cx: &LateContext<'tcx>, id: HirId, block: &'tcx Block<'tcx>) -> bool { let mut has_access = false; let mut has_read_access = false; diff --git a/clippy_lints/src/methods/clear_with_drain.rs b/clippy_lints/src/methods/clear_with_drain.rs index 24496bd4689f..67ad58d5a8c6 100644 --- a/clippy_lints/src/methods/clear_with_drain.rs +++ b/clippy_lints/src/methods/clear_with_drain.rs @@ -1,25 +1,50 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_range_full; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_hir as hir; +use rustc_hir::{Expr, ExprKind, LangItem, QPath}; use rustc_lint::LateContext; use rustc_span::symbol::sym; use rustc_span::Span; use super::CLEAR_WITH_DRAIN; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: &Expr<'_>) { - let ty = cx.typeck_results().expr_ty(recv); - if is_type_diagnostic_item(cx, ty, sym::Vec) - && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind - && is_range_full(cx, arg, Some(container_path)) +// Add `String` here when it is added to diagnostic items +const ACCEPTABLE_TYPES_WITH_ARG: [rustc_span::Symbol; 2] = [sym::Vec, sym::VecDeque]; + +const ACCEPTABLE_TYPES_WITHOUT_ARG: [rustc_span::Symbol; 3] = [sym::BinaryHeap, sym::HashMap, sym::HashSet]; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: Option<&Expr<'_>>) { + if let Some(arg) = arg { + if match_acceptable_type(cx, recv, &ACCEPTABLE_TYPES_WITH_ARG) + && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind + && is_range_full(cx, arg, Some(container_path)) + { + suggest(cx, expr, recv, span); + } + } else if match_acceptable_type(cx, recv, &ACCEPTABLE_TYPES_WITHOUT_ARG) { + suggest(cx, expr, recv, span); + } +} + +fn match_acceptable_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>, types: &[rustc_span::Symbol]) -> bool { + let expr_ty = cx.typeck_results().expr_ty(expr).peel_refs(); + types.iter().any(|&ty| is_type_diagnostic_item(cx, expr_ty, ty)) + // String type is a lang item but not a diagnostic item for now so we need a separate check + || is_type_lang_item(cx, expr_ty, LangItem::String) +} + +fn suggest(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span) { + if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def() + // Use `opt_item_name` while `String` is not a diagnostic item + && let Some(ty_name) = cx.tcx.opt_item_name(adt.did()) { span_lint_and_sugg( cx, CLEAR_WITH_DRAIN, span.with_hi(expr.span.hi()), - "`drain` used to clear a `Vec`", + &format!("`drain` used to clear a `{ty_name}`"), "try", "clear()".to_string(), Applicability::MachineApplicable, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 257bc4eccc30..64bf55ba24c9 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3193,7 +3193,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `.drain(..)` for the sole purpose of clearing a `Vec`. + /// Checks for usage of `.drain(..)` for the sole purpose of clearing a container. /// /// ### Why is this bad? /// This creates an unnecessary iterator that is dropped immediately. @@ -3213,7 +3213,7 @@ declare_clippy_lint! { #[clippy::version = "1.69.0"] pub CLEAR_WITH_DRAIN, nursery, - "calling `drain` in order to `clear` a `Vec`" + "calling `drain` in order to `clear` a container" } pub struct Methods { @@ -3589,12 +3589,13 @@ impl Methods { Some(("bytes", recv2, [], _, _)) => bytes_count_to_len::check(cx, expr, recv, recv2), _ => {}, }, - ("drain", [arg]) => { - if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.hir().get_parent(expr.hir_id) - && matches!(kind, StmtKind::Semi(_)) + ("drain", ..) => { + if let Node::Stmt(Stmt { hir_id: _, kind, .. }) = cx.tcx.hir().get_parent(expr.hir_id) + && matches!(kind, StmtKind::Semi(_)) + && args.len() <= 1 { - clear_with_drain::check(cx, expr, recv, span, arg); - } else { + clear_with_drain::check(cx, expr, recv, span, args.first()); + } else if let [arg] = args { iter_with_drain::check(cx, expr, recv, span, arg); } }, diff --git a/tests/ui/clear_with_drain.fixed b/tests/ui/clear_with_drain.fixed index 9c4dc010ca7f..2d9545eeed19 100644 --- a/tests/ui/clear_with_drain.fixed +++ b/tests/ui/clear_with_drain.fixed @@ -2,85 +2,357 @@ #![allow(unused)] #![warn(clippy::clear_with_drain)] -fn range() { +use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque}; + +fn vec_range() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(0..v.len()); // Yay + let iter = v.drain(0..v.len()); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - let n = v.drain(0..v.len()).count(); // Yay + let n = v.drain(0..v.len()).count(); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let iter = v.drain(usize::MIN..v.len()); // Yay + let iter = v.drain(usize::MIN..v.len()); let n = iter.count(); + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); } -fn range_from() { +fn vec_range_from() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(0..); // Yay + let iter = v.drain(0..); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let mut iter = v.drain(0..); // Yay + let mut iter = v.drain(0..); let next = iter.next(); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - let next = v.drain(usize::MIN..).next(); // Yay + let next = v.drain(usize::MIN..).next(); + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); } -fn range_full() { +fn vec_range_full() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(..); // Yay + let iter = v.drain(..); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - // Yay for x in v.drain(..) { let y = format!("x = {x}"); } + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); } -fn range_to() { +fn vec_range_to() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(..v.len()); // Yay + let iter = v.drain(..v.len()); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let iter = v.drain(..v.len()); // Yay + let iter = v.drain(..v.len()); for x in iter { let y = format!("x = {x}"); } + // Do lint let mut v = vec![1, 2, 3]; - v.clear(); // Nay + v.clear(); } -fn partial_drains() { +fn vec_partial_drains() { + // Do not lint any of these because the ranges are not full + let mut v = vec![1, 2, 3]; - v.drain(1..); // Yay + v.drain(1..); let mut v = vec![1, 2, 3]; - v.drain(1..).max(); // Yay + v.drain(1..).max(); let mut v = vec![1, 2, 3]; - v.drain(..v.len() - 1); // Yay + v.drain(..v.len() - 1); let mut v = vec![1, 2, 3]; - v.drain(..v.len() - 1).min(); // Yay + v.drain(..v.len() - 1).min(); let mut v = vec![1, 2, 3]; - v.drain(1..v.len() - 1); // Yay + v.drain(1..v.len() - 1); let mut v = vec![1, 2, 3]; - let w: Vec = v.drain(1..v.len() - 1).collect(); // Yay + let w: Vec = v.drain(1..v.len() - 1).collect(); +} + +fn vec_deque_range() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(0..deque.len()); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + let n = deque.drain(0..deque.len()).count(); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(usize::MIN..deque.len()); + let n = iter.count(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); +} + +fn vec_deque_range_from() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(0..); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let mut iter = deque.drain(0..); + let next = iter.next(); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + let next = deque.drain(usize::MIN..).next(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); +} + +fn vec_deque_range_full() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + for x in deque.drain(..) { + let y = format!("x = {x}"); + } + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); +} + +fn vec_deque_range_to() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..deque.len()); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..deque.len()); + for x in iter { + let y = format!("x = {x}"); + } + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.clear(); +} + +fn vec_deque_partial_drains() { + // Do not lint any of these because the ranges are not full + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..); + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..).max(); + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..deque.len() - 1); + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..deque.len() - 1).min(); + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..deque.len() - 1); + let mut deque = VecDeque::from([1, 2, 3]); + let w: Vec = deque.drain(1..deque.len() - 1).collect(); +} + +fn string_range() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(0..s.len()); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + let n = s.drain(0..s.len()).count(); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let iter = s.drain(usize::MIN..s.len()); + let n = iter.count(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); +} + +fn string_range_from() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(0..); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let mut iter = s.drain(0..); + let next = iter.next(); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + let next = s.drain(usize::MIN..).next(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); +} + +fn string_range_full() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(..); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + for x in s.drain(..) { + let y = format!("x = {x}"); + } + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); +} + +fn string_range_to() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(..s.len()); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let iter = s.drain(..s.len()); + for x in iter { + let y = format!("x = {x}"); + } + + // Do lint + let mut s = String::from("Hello, world!"); + s.clear(); +} + +fn string_partial_drains() { + // Do not lint any of these because the ranges are not full + + let mut s = String::from("Hello, world!"); + s.drain(1..); + let mut s = String::from("Hello, world!"); + s.drain(1..).max(); + + let mut s = String::from("Hello, world!"); + s.drain(..s.len() - 1); + let mut s = String::from("Hello, world!"); + s.drain(..s.len() - 1).min(); + + let mut s = String::from("Hello, world!"); + s.drain(1..s.len() - 1); + let mut s = String::from("Hello, world!"); + let w: String = s.drain(1..s.len() - 1).collect(); +} + +fn hash_set() { + // Do not lint because iterator is assigned + let mut set = HashSet::from([1, 2, 3]); + let iter = set.drain(); + + // Do not lint because iterator is assigned and used + let mut set = HashSet::from([1, 2, 3]); + let mut iter = set.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut set = HashSet::from([1, 2, 3]); + let next = set.drain().next(); + + // Do lint + let mut set = HashSet::from([1, 2, 3]); + set.clear(); +} + +fn hash_map() { + // Do not lint because iterator is assigned + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let iter = map.drain(); + + // Do not lint because iterator is assigned and used + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let mut iter = map.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let next = map.drain().next(); + + // Do lint + let mut map = HashMap::from([(1, "a"), (2, "b")]); + map.clear(); +} + +fn binary_heap() { + // Do not lint because iterator is assigned + let mut heap = BinaryHeap::from([1, 2]); + let iter = heap.drain(); + + // Do not lint because iterator is assigned and used + let mut heap = BinaryHeap::from([1, 2]); + let mut iter = heap.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut heap = BinaryHeap::from([1, 2]); + let next = heap.drain().next(); + + // Do lint + let mut heap = BinaryHeap::from([1, 2]); + heap.clear(); } fn main() {} diff --git a/tests/ui/clear_with_drain.rs b/tests/ui/clear_with_drain.rs index f00dbab234cc..4d60ee46e186 100644 --- a/tests/ui/clear_with_drain.rs +++ b/tests/ui/clear_with_drain.rs @@ -2,85 +2,357 @@ #![allow(unused)] #![warn(clippy::clear_with_drain)] -fn range() { +use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque}; + +fn vec_range() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(0..v.len()); // Yay + let iter = v.drain(0..v.len()); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - let n = v.drain(0..v.len()).count(); // Yay + let n = v.drain(0..v.len()).count(); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let iter = v.drain(usize::MIN..v.len()); // Yay + let iter = v.drain(usize::MIN..v.len()); let n = iter.count(); + // Do lint let mut v = vec![1, 2, 3]; - v.drain(0..v.len()); // Nay + v.drain(0..v.len()); + // Do lint let mut v = vec![1, 2, 3]; - v.drain(usize::MIN..v.len()); // Nay + v.drain(usize::MIN..v.len()); } -fn range_from() { +fn vec_range_from() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(0..); // Yay + let iter = v.drain(0..); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let mut iter = v.drain(0..); // Yay + let mut iter = v.drain(0..); let next = iter.next(); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - let next = v.drain(usize::MIN..).next(); // Yay + let next = v.drain(usize::MIN..).next(); + // Do lint let mut v = vec![1, 2, 3]; - v.drain(0..); // Nay + v.drain(0..); + // Do lint let mut v = vec![1, 2, 3]; - v.drain(usize::MIN..); // Nay + v.drain(usize::MIN..); } -fn range_full() { +fn vec_range_full() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(..); // Yay + let iter = v.drain(..); + // Do not lint because iterator is used let mut v = vec![1, 2, 3]; - // Yay for x in v.drain(..) { let y = format!("x = {x}"); } + // Do lint let mut v = vec![1, 2, 3]; - v.drain(..); // Nay + v.drain(..); } -fn range_to() { +fn vec_range_to() { + // Do not lint because iterator is assigned let mut v = vec![1, 2, 3]; - let iter = v.drain(..v.len()); // Yay + let iter = v.drain(..v.len()); + // Do not lint because iterator is assigned and used let mut v = vec![1, 2, 3]; - let iter = v.drain(..v.len()); // Yay + let iter = v.drain(..v.len()); for x in iter { let y = format!("x = {x}"); } + // Do lint let mut v = vec![1, 2, 3]; - v.drain(..v.len()); // Nay + v.drain(..v.len()); } -fn partial_drains() { +fn vec_partial_drains() { + // Do not lint any of these because the ranges are not full + let mut v = vec![1, 2, 3]; - v.drain(1..); // Yay + v.drain(1..); let mut v = vec![1, 2, 3]; - v.drain(1..).max(); // Yay + v.drain(1..).max(); let mut v = vec![1, 2, 3]; - v.drain(..v.len() - 1); // Yay + v.drain(..v.len() - 1); let mut v = vec![1, 2, 3]; - v.drain(..v.len() - 1).min(); // Yay + v.drain(..v.len() - 1).min(); let mut v = vec![1, 2, 3]; - v.drain(1..v.len() - 1); // Yay + v.drain(1..v.len() - 1); let mut v = vec![1, 2, 3]; - let w: Vec = v.drain(1..v.len() - 1).collect(); // Yay + let w: Vec = v.drain(1..v.len() - 1).collect(); +} + +fn vec_deque_range() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(0..deque.len()); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + let n = deque.drain(0..deque.len()).count(); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(usize::MIN..deque.len()); + let n = iter.count(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(0..deque.len()); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(usize::MIN..deque.len()); +} + +fn vec_deque_range_from() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(0..); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let mut iter = deque.drain(0..); + let next = iter.next(); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + let next = deque.drain(usize::MIN..).next(); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(0..); + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(usize::MIN..); +} + +fn vec_deque_range_full() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..); + + // Do not lint because iterator is used + let mut deque = VecDeque::from([1, 2, 3]); + for x in deque.drain(..) { + let y = format!("x = {x}"); + } + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..); +} + +fn vec_deque_range_to() { + // Do not lint because iterator is assigned + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..deque.len()); + + // Do not lint because iterator is assigned and used + let mut deque = VecDeque::from([1, 2, 3]); + let iter = deque.drain(..deque.len()); + for x in iter { + let y = format!("x = {x}"); + } + + // Do lint + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..deque.len()); +} + +fn vec_deque_partial_drains() { + // Do not lint any of these because the ranges are not full + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..); + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..).max(); + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..deque.len() - 1); + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(..deque.len() - 1).min(); + + let mut deque = VecDeque::from([1, 2, 3]); + deque.drain(1..deque.len() - 1); + let mut deque = VecDeque::from([1, 2, 3]); + let w: Vec = deque.drain(1..deque.len() - 1).collect(); +} + +fn string_range() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(0..s.len()); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + let n = s.drain(0..s.len()).count(); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let iter = s.drain(usize::MIN..s.len()); + let n = iter.count(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(0..s.len()); + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(usize::MIN..s.len()); +} + +fn string_range_from() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(0..); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let mut iter = s.drain(0..); + let next = iter.next(); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + let next = s.drain(usize::MIN..).next(); + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(0..); + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(usize::MIN..); +} + +fn string_range_full() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(..); + + // Do not lint because iterator is used + let mut s = String::from("Hello, world!"); + for x in s.drain(..) { + let y = format!("x = {x}"); + } + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(..); +} + +fn string_range_to() { + // Do not lint because iterator is assigned + let mut s = String::from("Hello, world!"); + let iter = s.drain(..s.len()); + + // Do not lint because iterator is assigned and used + let mut s = String::from("Hello, world!"); + let iter = s.drain(..s.len()); + for x in iter { + let y = format!("x = {x}"); + } + + // Do lint + let mut s = String::from("Hello, world!"); + s.drain(..s.len()); +} + +fn string_partial_drains() { + // Do not lint any of these because the ranges are not full + + let mut s = String::from("Hello, world!"); + s.drain(1..); + let mut s = String::from("Hello, world!"); + s.drain(1..).max(); + + let mut s = String::from("Hello, world!"); + s.drain(..s.len() - 1); + let mut s = String::from("Hello, world!"); + s.drain(..s.len() - 1).min(); + + let mut s = String::from("Hello, world!"); + s.drain(1..s.len() - 1); + let mut s = String::from("Hello, world!"); + let w: String = s.drain(1..s.len() - 1).collect(); +} + +fn hash_set() { + // Do not lint because iterator is assigned + let mut set = HashSet::from([1, 2, 3]); + let iter = set.drain(); + + // Do not lint because iterator is assigned and used + let mut set = HashSet::from([1, 2, 3]); + let mut iter = set.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut set = HashSet::from([1, 2, 3]); + let next = set.drain().next(); + + // Do lint + let mut set = HashSet::from([1, 2, 3]); + set.drain(); +} + +fn hash_map() { + // Do not lint because iterator is assigned + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let iter = map.drain(); + + // Do not lint because iterator is assigned and used + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let mut iter = map.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut map = HashMap::from([(1, "a"), (2, "b")]); + let next = map.drain().next(); + + // Do lint + let mut map = HashMap::from([(1, "a"), (2, "b")]); + map.drain(); +} + +fn binary_heap() { + // Do not lint because iterator is assigned + let mut heap = BinaryHeap::from([1, 2]); + let iter = heap.drain(); + + // Do not lint because iterator is assigned and used + let mut heap = BinaryHeap::from([1, 2]); + let mut iter = heap.drain(); + let next = iter.next(); + + // Do not lint because iterator is used + let mut heap = BinaryHeap::from([1, 2]); + let next = heap.drain().next(); + + // Do lint + let mut heap = BinaryHeap::from([1, 2]); + heap.drain(); } fn main() {} diff --git a/tests/ui/clear_with_drain.stderr b/tests/ui/clear_with_drain.stderr index c88aa1a23cb6..20158da1121b 100644 --- a/tests/ui/clear_with_drain.stderr +++ b/tests/ui/clear_with_drain.stderr @@ -1,40 +1,130 @@ error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:17:7 + --> $DIR/clear_with_drain.rs:23:7 | -LL | v.drain(0..v.len()); // Nay +LL | v.drain(0..v.len()); | ^^^^^^^^^^^^^^^^^ help: try: `clear()` | = note: `-D clippy::clear-with-drain` implied by `-D warnings` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:20:7 + --> $DIR/clear_with_drain.rs:27:7 | -LL | v.drain(usize::MIN..v.len()); // Nay +LL | v.drain(usize::MIN..v.len()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:35:7 + --> $DIR/clear_with_drain.rs:46:7 | -LL | v.drain(0..); // Nay +LL | v.drain(0..); | ^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:38:7 + --> $DIR/clear_with_drain.rs:50:7 | -LL | v.drain(usize::MIN..); // Nay +LL | v.drain(usize::MIN..); | ^^^^^^^^^^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:52:7 + --> $DIR/clear_with_drain.rs:66:7 | -LL | v.drain(..); // Nay +LL | v.drain(..); | ^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:66:7 + --> $DIR/clear_with_drain.rs:83:7 + | +LL | v.drain(..v.len()); + | ^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:121:11 + | +LL | deque.drain(0..deque.len()); + | ^^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:125:11 + | +LL | deque.drain(usize::MIN..deque.len()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:144:11 + | +LL | deque.drain(0..); + | ^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:148:11 + | +LL | deque.drain(usize::MIN..); + | ^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:164:11 + | +LL | deque.drain(..); + | ^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:181:11 + | +LL | deque.drain(..deque.len()); + | ^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:219:7 + | +LL | s.drain(0..s.len()); + | ^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:223:7 + | +LL | s.drain(usize::MIN..s.len()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:242:7 + | +LL | s.drain(0..); + | ^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:246:7 + | +LL | s.drain(usize::MIN..); + | ^^^^^^^^^^^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:262:7 | -LL | v.drain(..v.len()); // Nay +LL | s.drain(..); + | ^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `String` + --> $DIR/clear_with_drain.rs:279:7 + | +LL | s.drain(..s.len()); | ^^^^^^^^^^^^^^^^ help: try: `clear()` -error: aborting due to 6 previous errors +error: `drain` used to clear a `HashSet` + --> $DIR/clear_with_drain.rs:317:9 + | +LL | set.drain(); + | ^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `HashMap` + --> $DIR/clear_with_drain.rs:336:9 + | +LL | map.drain(); + | ^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `BinaryHeap` + --> $DIR/clear_with_drain.rs:355:10 + | +LL | heap.drain(); + | ^^^^^^^ help: try: `clear()` + +error: aborting due to 21 previous errors diff --git a/tests/ui/collection_is_never_read.rs b/tests/ui/collection_is_never_read.rs index 068a49486cf8..ca20031bfbef 100644 --- a/tests/ui/collection_is_never_read.rs +++ b/tests/ui/collection_is_never_read.rs @@ -163,3 +163,23 @@ fn function_argument() { let x = vec![1, 2, 3]; // Ok foo(&x); } + +fn string() { + // Do lint (write without read) + let mut s = String::new(); + s.push_str("Hello, World!"); + + // Do not lint (read without write) + let mut s = String::from("Hello, World!"); + let _ = s.len(); + + // Do not lint (write and read) + let mut s = String::from("Hello, World!"); + s.push_str("foo, bar"); + let _ = s.len(); + + // Do lint the first line, but not the second + let mut s = String::from("Hello, World!"); + let t = String::from("foo, bar"); + s = t; +} diff --git a/tests/ui/collection_is_never_read.stderr b/tests/ui/collection_is_never_read.stderr index 7654b74be3d1..f5dea96116f8 100644 --- a/tests/ui/collection_is_never_read.stderr +++ b/tests/ui/collection_is_never_read.stderr @@ -48,5 +48,17 @@ error: collection is never read LL | let x = vec![1, 2, 3]; // WARNING | ^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 8 previous errors +error: collection is never read + --> $DIR/collection_is_never_read.rs:169:5 + | +LL | let mut s = String::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: collection is never read + --> $DIR/collection_is_never_read.rs:182:5 + | +LL | let mut s = String::from("Hello, World!"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 10 previous errors