From 9ec542873cde1332baf2a0ed33088ba98eba25cf Mon Sep 17 00:00:00 2001 From: bluthej Date: Thu, 6 Apr 2023 07:49:59 +0200 Subject: [PATCH] Add 5 other container types and start testing --- clippy_lints/src/methods/clear_with_drain.rs | 33 +++++++++++++---- clippy_lints/src/methods/mod.rs | 11 +++--- tests/ui/clear_with_drain.fixed | 25 ++++++++++++- tests/ui/clear_with_drain.rs | 25 ++++++++++++- tests/ui/clear_with_drain.stderr | 38 ++++++++++++++++---- 5 files changed, 112 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/methods/clear_with_drain.rs b/clippy_lints/src/methods/clear_with_drain.rs index 24496bd46..4f5b7762e 100644 --- a/clippy_lints/src/methods/clear_with_drain.rs +++ b/clippy_lints/src/methods/clear_with_drain.rs @@ -2,6 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_range_full; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; +use rustc_hir as hir; use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::LateContext; use rustc_span::symbol::sym; @@ -9,17 +10,37 @@ 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)) +const ACCEPTABLE_TYPES_WITH_ARG: [rustc_span::Symbol; 3] = [sym::String, 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)) +} + +fn suggest(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span) { + if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def() + && let Some(ty_name) = cx.tcx.get_diagnostic_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 257bc4ecc..c25f1b60a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -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 9c4dc010c..dd02cc914 100644 --- a/tests/ui/clear_with_drain.fixed +++ b/tests/ui/clear_with_drain.fixed @@ -2,6 +2,11 @@ #![allow(unused)] #![warn(clippy::clear_with_drain)] +use std::collections::BinaryHeap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; + fn range() { let mut v = vec![1, 2, 3]; let iter = v.drain(0..v.len()); // Yay @@ -83,4 +88,22 @@ fn partial_drains() { let w: Vec = v.drain(1..v.len() - 1).collect(); // Yay } -fn main() {} +fn main() { + let mut deque: VecDeque<_> = [1, 2, 3].into(); + deque.clear(); + + let mut set = HashSet::from([1, 2, 3]); + set.clear(); + + let mut a = HashMap::new(); + a.insert(1, "a"); + a.insert(2, "b"); + a.clear(); + + let mut heap = BinaryHeap::from([1, 3]); + heap.clear(); + + // Not firing for now because `String` is not reckognized by `is_type_diagnostic_item` + let mut s = String::from("α is alpha, β is beta"); + s.drain(..); +} diff --git a/tests/ui/clear_with_drain.rs b/tests/ui/clear_with_drain.rs index f00dbab23..af2fe503d 100644 --- a/tests/ui/clear_with_drain.rs +++ b/tests/ui/clear_with_drain.rs @@ -2,6 +2,11 @@ #![allow(unused)] #![warn(clippy::clear_with_drain)] +use std::collections::BinaryHeap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; + fn range() { let mut v = vec![1, 2, 3]; let iter = v.drain(0..v.len()); // Yay @@ -83,4 +88,22 @@ fn partial_drains() { let w: Vec = v.drain(1..v.len() - 1).collect(); // Yay } -fn main() {} +fn main() { + let mut deque: VecDeque<_> = [1, 2, 3].into(); + deque.drain(..); + + let mut set = HashSet::from([1, 2, 3]); + set.drain(); + + let mut a = HashMap::new(); + a.insert(1, "a"); + a.insert(2, "b"); + a.drain(); + + let mut heap = BinaryHeap::from([1, 3]); + heap.drain(); + + // Not firing for now because `String` is not reckognized by `is_type_diagnostic_item` + let mut s = String::from("α is alpha, β is beta"); + s.drain(..); +} diff --git a/tests/ui/clear_with_drain.stderr b/tests/ui/clear_with_drain.stderr index c88aa1a23..2c0cc846d 100644 --- a/tests/ui/clear_with_drain.stderr +++ b/tests/ui/clear_with_drain.stderr @@ -1,5 +1,5 @@ error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:17:7 + --> $DIR/clear_with_drain.rs:22:7 | LL | v.drain(0..v.len()); // Nay | ^^^^^^^^^^^^^^^^^ help: try: `clear()` @@ -7,34 +7,58 @@ LL | v.drain(0..v.len()); // Nay = 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:25:7 | LL | v.drain(usize::MIN..v.len()); // Nay | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:35:7 + --> $DIR/clear_with_drain.rs:40:7 | LL | v.drain(0..); // Nay | ^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:38:7 + --> $DIR/clear_with_drain.rs:43:7 | LL | v.drain(usize::MIN..); // Nay | ^^^^^^^^^^^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:52:7 + --> $DIR/clear_with_drain.rs:57:7 | LL | v.drain(..); // Nay | ^^^^^^^^^ help: try: `clear()` error: `drain` used to clear a `Vec` - --> $DIR/clear_with_drain.rs:66:7 + --> $DIR/clear_with_drain.rs:71:7 | LL | v.drain(..v.len()); // Nay | ^^^^^^^^^^^^^^^^ help: try: `clear()` -error: aborting due to 6 previous errors +error: `drain` used to clear a `VecDeque` + --> $DIR/clear_with_drain.rs:93:11 + | +LL | deque.drain(..); + | ^^^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `HashSet` + --> $DIR/clear_with_drain.rs:96:9 + | +LL | set.drain(); + | ^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `HashMap` + --> $DIR/clear_with_drain.rs:101:7 + | +LL | a.drain(); + | ^^^^^^^ help: try: `clear()` + +error: `drain` used to clear a `BinaryHeap` + --> $DIR/clear_with_drain.rs:104:10 + | +LL | heap.drain(); + | ^^^^^^^ help: try: `clear()` + +error: aborting due to 10 previous errors