Auto merge of #7292 - Jarcho:suspicious_splitn, r=flip1995

Add lint `suspicious_splitn`

fixes: #7245
changelog: Add lint `suspicious_splitn`
This commit is contained in:
bors 2021-05-30 20:32:22 +00:00
commit d1308aecaf
9 changed files with 197 additions and 7 deletions

View file

@ -779,6 +779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
methods::SKIP_WHILE_NEXT,
methods::STRING_EXTEND_CHARS,
methods::SUSPICIOUS_MAP,
methods::SUSPICIOUS_SPLITN,
methods::UNINIT_ASSUMED_INIT,
methods::UNNECESSARY_FILTER_MAP,
methods::UNNECESSARY_FOLD,
@ -1312,6 +1313,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(methods::SKIP_WHILE_NEXT),
LintId::of(methods::STRING_EXTEND_CHARS),
LintId::of(methods::SUSPICIOUS_MAP),
LintId::of(methods::SUSPICIOUS_SPLITN),
LintId::of(methods::UNINIT_ASSUMED_INIT),
LintId::of(methods::UNNECESSARY_FILTER_MAP),
LintId::of(methods::UNNECESSARY_FOLD),
@ -1688,6 +1690,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),
LintId::of(methods::CLONE_DOUBLE_REF),
LintId::of(methods::ITERATOR_STEP_BY_ZERO),
LintId::of(methods::SUSPICIOUS_SPLITN),
LintId::of(methods::UNINIT_ASSUMED_INIT),
LintId::of(methods::ZST_OFFSET),
LintId::of(minmax::MIN_MAX),

View file

@ -48,6 +48,7 @@ mod single_char_push_string;
mod skip_while_next;
mod string_extend_chars;
mod suspicious_map;
mod suspicious_splitn;
mod uninit_assumed_init;
mod unnecessary_filter_map;
mod unnecessary_fold;
@ -1633,6 +1634,36 @@ declare_clippy_lint! {
"replace `.iter().count()` with `.len()`"
}
declare_clippy_lint! {
/// **What it does:** Checks for calls to [`splitn`]
/// (https://doc.rust-lang.org/std/primitive.str.html#method.splitn) and
/// related functions with either zero or one splits.
///
/// **Why is this bad?** These calls don't actually split the value and are
/// likely to be intended as a different number.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // Bad
/// let s = "";
/// for x in s.splitn(1, ":") {
/// // use x
/// }
///
/// // Good
/// let s = "";
/// for x in s.splitn(2, ":") {
/// // use x
/// }
/// ```
pub SUSPICIOUS_SPLITN,
correctness,
"checks for `.splitn(0, ..)` and `.splitn(1, ..)`"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -1705,7 +1736,8 @@ impl_lint_pass!(Methods => [
MAP_COLLECT_RESULT_UNIT,
FROM_ITER_INSTEAD_OF_COLLECT,
INSPECT_FOR_EACH,
IMPLICIT_CLONE
IMPLICIT_CLONE,
SUSPICIOUS_SPLITN
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -2024,6 +2056,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
}
},
("splitn" | "splitn_mut" | "rsplitn" | "rsplitn_mut", [count_arg, _]) => {
suspicious_splitn::check(cx, name, expr, recv, count_arg);
},
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
implicit_clone::check(cx, name, expr, recv, span);

View file

@ -0,0 +1,56 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_note;
use if_chain::if_chain;
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::source_map::Spanned;
use super::SUSPICIOUS_SPLITN;
pub(super) fn check(
cx: &LateContext<'_>,
method_name: &str,
expr: &Expr<'_>,
self_arg: &Expr<'_>,
count_arg: &Expr<'_>,
) {
if_chain! {
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg);
if count <= 1;
if let Some(call_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if let Some(impl_id) = cx.tcx.impl_of_method(call_id);
let lang_items = cx.tcx.lang_items();
if lang_items.slice_impl() == Some(impl_id) || lang_items.str_impl() == Some(impl_id);
then {
// Ignore empty slice and string literals when used with a literal count.
if (matches!(self_arg.kind, ExprKind::Array([]))
|| matches!(self_arg.kind, ExprKind::Lit(Spanned { node: LitKind::Str(s, _), .. }) if s.is_empty())
) && matches!(count_arg.kind, ExprKind::Lit(_))
{
return;
}
let (msg, note_msg) = if count == 0 {
(format!("`{}` called with `0` splits", method_name),
"the resulting iterator will always return `None`")
} else {
(format!("`{}` called with `1` split", method_name),
if lang_items.slice_impl() == Some(impl_id) {
"the resulting iterator will always return the entire slice followed by `None`"
} else {
"the resulting iterator will always return the entire string followed by `None`"
})
};
span_lint_and_note(
cx,
SUSPICIOUS_SPLITN,
expr.span,
&msg,
None,
note_msg,
);
}
}
}