Skip to content

Commit

Permalink
new lint truncate_with_drain
Browse files Browse the repository at this point in the history
  • Loading branch information
Kither12 committed Oct 25, 2024
1 parent 9cf416d commit 9458180
Show file tree
Hide file tree
Showing 9 changed files with 767 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6033,6 +6033,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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html).
You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category.
Expand Down
2 changes: 1 addition & 1 deletion book/src/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your
[Rust](https://github.com/rust-lang/rust) code.

[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

Lints are divided into categories, each with a default [lint
level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,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,
Expand Down
28 changes: 28 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,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;
Expand Down Expand Up @@ -4166,6 +4167,31 @@ declare_clippy_lint! {
"calling `.first().is_some()` or `.first().is_none()` instead of `.is_empty()`"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.drain(x..)` for the sole purpose of truncate 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,
nursery,
"calling `drain` in order to `truncate` a `Vec`"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -4327,6 +4353,7 @@ impl_lint_pass!(Methods => [
NEEDLESS_CHARACTER_ITERATION,
MANUAL_INSPECT,
UNNECESSARY_MIN_OR_MAX,
TRUNCATE_WITH_DRAIN,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4634,6 +4661,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);
Expand Down
100 changes: 100 additions & 0 deletions clippy_lints/src/methods/truncate_with_drain.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
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::{self as rustc_ty};
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(cx: &LateContext<'_>, expr: &Expr<'_>, container_path: Option<&Path<'_>>) -> bool {
let ty = cx.typeck_results().expr_ty(expr);
if let Some(higher::Range { start, end, limits }) = higher::Range::hir(expr) {
let start_is_none_or_min = start.map_or(true, |start| {
if let rustc_ty::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 rustc_ty::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
}
},
});
return !start_is_none_or_min && end_is_none_or_max;
}
false
}

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_open_ended(cx, arg, Some(container_path))
{
suggest(cx, expr, recv, span, arg);
}
}
}

fn match_acceptable_type(cx: &LateContext<'_>, expr: &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, arg: &Expr<'_>) {
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())
{
if let Some(higher::Range { start: Some(start), .. }) = higher::Range::hir(arg) {
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,
);
}
}
}
Loading

0 comments on commit 9458180

Please sign in to comment.