truncate_with_drain

Author: Kither12

CHANGELOG.md

@@ -6037,6 +6037,7 @@ Released 2018-09-13
 [`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
 [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
+[`truncate_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#truncate_with_drain
 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
 [`tuple_array_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions
 [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

clippy_lints/src/declared_lints.rs

@@ -469,6 +469,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::SUSPICIOUS_OPEN_OPTIONS_INFO,
     crate::methods::SUSPICIOUS_SPLITN_INFO,
     crate::methods::SUSPICIOUS_TO_OWNED_INFO,
+    crate::methods::TRUNCATE_WITH_DRAIN_INFO,
     crate::methods::TYPE_ID_ON_BOX_INFO,
     crate::methods::UNINIT_ASSUMED_INIT_INFO,
     crate::methods::UNIT_HASH_INFO,

clippy_lints/src/methods/mod.rs

@@ -109,6 +109,7 @@ mod suspicious_command_arg_space;
 mod suspicious_map;
 mod suspicious_splitn;
 mod suspicious_to_owned;
+mod truncate_with_drain;
 mod type_id_on_box;
 mod uninit_assumed_init;
 mod unit_hash;
@@ -4281,6 +4282,31 @@ declare_clippy_lint! {
     "map of a trivial closure (not dependent on parameter) over a range"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `.drain(x..)` for the sole purpose of truncating a container.
+    ///
+    /// ### Why is this bad?
+    /// This creates an unnecessary iterator that is dropped immediately.
+    ///
+    /// Calling `.truncate(x)` also makes the intent clearer.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let mut v = vec![1, 2, 3];
+    /// v.drain(1..);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let mut v = vec![1, 2, 3];
+    /// v.truncate(1);
+    /// ```
+    #[clippy::version = "1.84.0"]
+    pub TRUNCATE_WITH_DRAIN,
+    style,
+    "calling `drain` in order to truncate a `Vec`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4446,6 +4472,7 @@ impl_lint_pass!(Methods => [
     MAP_ALL_ANY_IDENTITY,
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     UNNECESSARY_MAP_OR,
+    TRUNCATE_WITH_DRAIN,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4762,6 +4789,7 @@ impl Methods {
                         && matches!(kind, StmtKind::Semi(_))
                         && args.len() <= 1
                     {
+                        truncate_with_drain::check(cx, expr, recv, span, args.first());
                         clear_with_drain::check(cx, expr, recv, span, args.first());
                     } else if let [arg] = args {
                         iter_with_drain::check(cx, expr, recv, span, arg);

clippy_lints/src/methods/truncate_with_drain.rs

@@ -0,0 +1,102 @@
+use clippy_utils::consts::{ConstEvalCtxt, mir_to_const};
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::higher;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
+use rustc_ast::ast::RangeLimits;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, LangItem, Path, QPath};
+use rustc_lint::LateContext;
+use rustc_middle::mir::Const;
+use rustc_middle::ty::{Adt, Ty, TypeckResults};
+use rustc_span::Span;
+use rustc_span::symbol::sym;
+
+use super::TRUNCATE_WITH_DRAIN;
+
+// Add `String` here when it is added to diagnostic items
+const ACCEPTABLE_TYPES_WITH_ARG: [rustc_span::Symbol; 2] = [sym::Vec, sym::VecDeque];
+
+pub fn is_range_open_ended<'a>(
+    cx: &LateContext<'a>,
+    range: higher::Range<'_>,
+    ty: Ty<'a>,
+    container_path: Option<&Path<'_>>,
+) -> bool {
+    let higher::Range { start, end, limits } = range;
+    let start_is_none_or_min = start.map_or(true, |start| {
+        if let Adt(_, subst) = ty.kind()
+            && let bnd_ty = subst.type_at(0)
+            && let Some(min_val) = bnd_ty.numeric_min_val(cx.tcx)
+            && let Some(min_const) = mir_to_const(cx.tcx, Const::from_ty_const(min_val, bnd_ty, cx.tcx))
+            && let Some(start_const) = ConstEvalCtxt::new(cx).eval(start)
+        {
+            start_const == min_const
+        } else {
+            false
+        }
+    });
+    let end_is_none_or_max = end.map_or(true, |end| match limits {
+        RangeLimits::Closed => {
+            if let Adt(_, subst) = ty.kind()
+                && let bnd_ty = subst.type_at(0)
+                && let Some(max_val) = bnd_ty.numeric_max_val(cx.tcx)
+                && let Some(max_const) = mir_to_const(cx.tcx, Const::from_ty_const(max_val, bnd_ty, cx.tcx))
+                && let Some(end_const) = ConstEvalCtxt::new(cx).eval(end)
+            {
+                end_const == max_const
+            } else {
+                false
+            }
+        },
+        RangeLimits::HalfOpen => {
+            if let Some(container_path) = container_path
+                && let ExprKind::MethodCall(name, self_arg, [], _) = end.kind
+                && name.ident.name == sym::len
+                && let ExprKind::Path(QPath::Resolved(None, path)) = self_arg.kind
+            {
+                container_path.res == path.res
+            } else {
+                false
+            }
+        },
+    });
+    !start_is_none_or_min && end_is_none_or_max
+}
+
+fn match_acceptable_type(
+    cx: &LateContext<'_>,
+    expr: &Expr<'_>,
+    typeck_results: &TypeckResults<'_>,
+    types: &[rustc_span::Symbol],
+) -> bool {
+    let expr_ty = 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)
+}
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: Option<&Expr<'_>>) {
+    if let Some(arg) = arg {
+        let typeck_results = cx.typeck_results();
+        if match_acceptable_type(cx, recv, typeck_results, &ACCEPTABLE_TYPES_WITH_ARG)
+            && let ExprKind::Path(QPath::Resolved(None, container_path)) = recv.kind
+            && let Some(range) = higher::Range::hir(arg)
+            && let higher::Range { start: Some(start), .. } = range
+            && is_range_open_ended(cx, range, typeck_results.expr_ty(arg), Some(container_path))
+            && let Some(adt) = 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,
+                TRUNCATE_WITH_DRAIN,
+                span.with_hi(expr.span.hi()),
+                format!("`drain` used to truncate a `{ty_name}`"),
+                "try",
+                format!("truncate({})", snippet(cx, start.span, "0")),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}

tests/ui/clear_with_drain.fixed

@@ -86,7 +86,7 @@ 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..);
+    v.truncate(1);
     let mut v = vec![1, 2, 3];
     v.drain(1..).max();
 
@@ -184,7 +184,7 @@ 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..);
+    deque.truncate(1);
     let mut deque = VecDeque::from([1, 2, 3]);
     deque.drain(1..).max();
 
@@ -282,7 +282,7 @@ 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..);
+    s.truncate(1);
     let mut s = String::from("Hello, world!");
     s.drain(1..).max();
 

tests/ui/clear_with_drain.stderr

@@ -37,6 +37,15 @@ error: `drain` used to clear a `Vec`
 LL |     v.drain(..v.len());
    |       ^^^^^^^^^^^^^^^^ help: try: `clear()`
 
+error: `drain` used to truncate a `Vec`
+  --> tests/ui/clear_with_drain.rs:89:7
+   |
+LL |     v.drain(1..);
+   |       ^^^^^^^^^^ help: try: `truncate(1)`
+   |
+   = note: `-D clippy::truncate-with-drain` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::truncate_with_drain)]`
+
 error: `drain` used to clear a `VecDeque`
   --> tests/ui/clear_with_drain.rs:120:11
    |
@@ -73,6 +82,12 @@ error: `drain` used to clear a `VecDeque`
 LL |     deque.drain(..deque.len());
    |           ^^^^^^^^^^^^^^^^^^^^ help: try: `clear()`
 
+error: `drain` used to truncate a `VecDeque`
+  --> tests/ui/clear_with_drain.rs:187:11
+   |
+LL |     deque.drain(1..);
+   |           ^^^^^^^^^^ help: try: `truncate(1)`
+
 error: `drain` used to clear a `String`
   --> tests/ui/clear_with_drain.rs:218:7
    |
@@ -109,6 +124,12 @@ error: `drain` used to clear a `String`
 LL |     s.drain(..s.len());
    |       ^^^^^^^^^^^^^^^^ help: try: `clear()`
 
+error: `drain` used to truncate a `String`
+  --> tests/ui/clear_with_drain.rs:285:7
+   |
+LL |     s.drain(1..);
+   |       ^^^^^^^^^^ help: try: `truncate(1)`
+
 error: `drain` used to clear a `HashSet`
   --> tests/ui/clear_with_drain.rs:316:9
    |
@@ -127,5 +148,5 @@ error: `drain` used to clear a `BinaryHeap`
 LL |     heap.drain();
    |          ^^^^^^^ help: try: `clear()`
 
-error: aborting due to 21 previous errors
+error: aborting due to 24 previous errors