Add lint manual_split_once
This commit is contained in:
parent
8cf6dae0ca
commit
a7f376fbd3
14 changed files with 459 additions and 26 deletions
|
|
@ -773,6 +773,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
methods::MANUAL_FILTER_MAP,
|
||||
methods::MANUAL_FIND_MAP,
|
||||
methods::MANUAL_SATURATING_ARITHMETIC,
|
||||
methods::MANUAL_SPLIT_ONCE,
|
||||
methods::MANUAL_STR_REPEAT,
|
||||
methods::MAP_COLLECT_RESULT_UNIT,
|
||||
methods::MAP_FLATTEN,
|
||||
|
|
@ -1319,6 +1320,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(methods::MANUAL_FILTER_MAP),
|
||||
LintId::of(methods::MANUAL_FIND_MAP),
|
||||
LintId::of(methods::MANUAL_SATURATING_ARITHMETIC),
|
||||
LintId::of(methods::MANUAL_SPLIT_ONCE),
|
||||
LintId::of(methods::MANUAL_STR_REPEAT),
|
||||
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
|
||||
LintId::of(methods::MAP_IDENTITY),
|
||||
|
|
@ -1617,6 +1619,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(methods::ITER_COUNT),
|
||||
LintId::of(methods::MANUAL_FILTER_MAP),
|
||||
LintId::of(methods::MANUAL_FIND_MAP),
|
||||
LintId::of(methods::MANUAL_SPLIT_ONCE),
|
||||
LintId::of(methods::MAP_IDENTITY),
|
||||
LintId::of(methods::OPTION_AS_REF_DEREF),
|
||||
LintId::of(methods::OPTION_FILTER_MAP),
|
||||
|
|
|
|||
213
clippy_lints/src/methods/manual_split_once.rs
Normal file
213
clippy_lints/src/methods/manual_split_once.rs
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
use clippy_utils::consts::{constant, Constant};
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::snippet_with_context;
|
||||
use clippy_utils::{is_diag_item_method, match_def_path, paths};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, HirId, LangItem, Node, QPath};
|
||||
use rustc_lint::LateContext;
|
||||
use rustc_middle::ty::{self, adjustment::Adjust};
|
||||
use rustc_span::{symbol::sym, Span, SyntaxContext};
|
||||
|
||||
use super::MANUAL_SPLIT_ONCE;
|
||||
|
||||
pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, self_arg: &Expr<'_>, pat_arg: &Expr<'_>) {
|
||||
if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() {
|
||||
return;
|
||||
}
|
||||
|
||||
let ctxt = expr.span.ctxt();
|
||||
let usage = match parse_iter_usage(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id)) {
|
||||
Some(x) => x,
|
||||
None => return,
|
||||
};
|
||||
let (method_name, msg) = if method_name == "splitn" {
|
||||
("split_once", "manual implementation of `split_once`")
|
||||
} else {
|
||||
("rsplit_once", "manual implementation of `rsplit_once`")
|
||||
};
|
||||
|
||||
let mut app = Applicability::MachineApplicable;
|
||||
let self_snip = snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0;
|
||||
let pat_snip = snippet_with_context(cx, pat_arg.span, ctxt, "..", &mut app).0;
|
||||
|
||||
match usage.kind {
|
||||
IterUsageKind::NextTuple => {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MANUAL_SPLIT_ONCE,
|
||||
usage.span,
|
||||
msg,
|
||||
"try this",
|
||||
format!("{}.{}({})", self_snip, method_name, pat_snip),
|
||||
app,
|
||||
);
|
||||
},
|
||||
IterUsageKind::Next => {
|
||||
let self_deref = {
|
||||
let adjust = cx.typeck_results().expr_adjustments(self_arg);
|
||||
if adjust.is_empty() {
|
||||
String::new()
|
||||
} else if cx.typeck_results().expr_ty(self_arg).is_box()
|
||||
|| adjust
|
||||
.iter()
|
||||
.any(|a| matches!(a.kind, Adjust::Deref(Some(_))) || a.target.is_box())
|
||||
{
|
||||
format!("&{}", "*".repeat(adjust.len() - 1))
|
||||
} else {
|
||||
"*".repeat(adjust.len() - 2)
|
||||
}
|
||||
};
|
||||
let sugg = if usage.unwrap_kind.is_some() {
|
||||
format!(
|
||||
"{}.{}({}).map_or({}{}, |x| x.0)",
|
||||
&self_snip, method_name, pat_snip, self_deref, &self_snip
|
||||
)
|
||||
} else {
|
||||
format!(
|
||||
"Some({}.{}({}).map_or({}{}, |x| x.0))",
|
||||
&self_snip, method_name, pat_snip, self_deref, &self_snip
|
||||
)
|
||||
};
|
||||
|
||||
span_lint_and_sugg(cx, MANUAL_SPLIT_ONCE, usage.span, msg, "try this", sugg, app);
|
||||
},
|
||||
IterUsageKind::Second => {
|
||||
let access_str = match usage.unwrap_kind {
|
||||
Some(UnwrapKind::Unwrap) => ".unwrap().1",
|
||||
Some(UnwrapKind::QuestionMark) => "?.1",
|
||||
None => ".map(|x| x.1)",
|
||||
};
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MANUAL_SPLIT_ONCE,
|
||||
usage.span,
|
||||
msg,
|
||||
"try this",
|
||||
format!("{}.{}({}){}", self_snip, method_name, pat_snip, access_str),
|
||||
app,
|
||||
);
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
enum IterUsageKind {
|
||||
Next,
|
||||
Second,
|
||||
NextTuple,
|
||||
}
|
||||
|
||||
enum UnwrapKind {
|
||||
Unwrap,
|
||||
QuestionMark,
|
||||
}
|
||||
|
||||
struct IterUsage {
|
||||
kind: IterUsageKind,
|
||||
unwrap_kind: Option<UnwrapKind>,
|
||||
span: Span,
|
||||
}
|
||||
|
||||
fn parse_iter_usage(
|
||||
cx: &LateContext<'tcx>,
|
||||
ctxt: SyntaxContext,
|
||||
mut iter: impl Iterator<Item = (HirId, Node<'tcx>)>,
|
||||
) -> Option<IterUsage> {
|
||||
let (kind, span) = match iter.next() {
|
||||
Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => {
|
||||
let (name, args) = if let ExprKind::MethodCall(name, _, [_, args @ ..], _) = e.kind {
|
||||
(name, args)
|
||||
} else {
|
||||
return None;
|
||||
};
|
||||
let did = cx.typeck_results().type_dependent_def_id(e.hir_id)?;
|
||||
let iter_id = cx.tcx.get_diagnostic_item(sym::Iterator)?;
|
||||
|
||||
match (&*name.ident.as_str(), args) {
|
||||
("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => (IterUsageKind::Next, e.span),
|
||||
("next_tuple", []) => {
|
||||
if_chain! {
|
||||
if match_def_path(cx, did, &paths::ITERTOOLS_NEXT_TUPLE);
|
||||
if let ty::Adt(adt_def, subs) = cx.typeck_results().expr_ty(e).kind();
|
||||
if cx.tcx.is_diagnostic_item(sym::option_type, adt_def.did);
|
||||
if let ty::Tuple(subs) = subs.type_at(0).kind();
|
||||
if subs.len() == 2;
|
||||
then {
|
||||
return Some(IterUsage { kind: IterUsageKind::NextTuple, span: e.span, unwrap_kind: None });
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
},
|
||||
("nth" | "skip", [idx_expr]) if cx.tcx.trait_of_item(did) == Some(iter_id) => {
|
||||
if let Some((Constant::Int(idx), _)) = constant(cx, cx.typeck_results(), idx_expr) {
|
||||
let span = if name.ident.as_str() == "nth" {
|
||||
e.span
|
||||
} else {
|
||||
if_chain! {
|
||||
if let Some((_, Node::Expr(next_expr))) = iter.next();
|
||||
if let ExprKind::MethodCall(next_name, _, [_], _) = next_expr.kind;
|
||||
if next_name.ident.name == sym::next;
|
||||
if next_expr.span.ctxt() == ctxt;
|
||||
if let Some(next_id) = cx.typeck_results().type_dependent_def_id(next_expr.hir_id);
|
||||
if cx.tcx.trait_of_item(next_id) == Some(iter_id);
|
||||
then {
|
||||
next_expr.span
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
};
|
||||
match idx {
|
||||
0 => (IterUsageKind::Next, span),
|
||||
1 => (IterUsageKind::Second, span),
|
||||
_ => return None,
|
||||
}
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
},
|
||||
_ => return None,
|
||||
}
|
||||
},
|
||||
_ => return None,
|
||||
};
|
||||
|
||||
let (unwrap_kind, span) = if let Some((_, Node::Expr(e))) = iter.next() {
|
||||
match e.kind {
|
||||
ExprKind::Call(
|
||||
Expr {
|
||||
kind: ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, _)),
|
||||
..
|
||||
},
|
||||
_,
|
||||
) => {
|
||||
let parent_span = e.span.parent().unwrap();
|
||||
if parent_span.ctxt() == ctxt {
|
||||
(Some(UnwrapKind::QuestionMark), parent_span)
|
||||
} else {
|
||||
(None, span)
|
||||
}
|
||||
},
|
||||
_ if e.span.ctxt() != ctxt => (None, span),
|
||||
ExprKind::MethodCall(name, _, [_], _)
|
||||
if name.ident.name == sym::unwrap
|
||||
&& cx
|
||||
.typeck_results()
|
||||
.type_dependent_def_id(e.hir_id)
|
||||
.map_or(false, |id| is_diag_item_method(cx, id, sym::option_type)) =>
|
||||
{
|
||||
(Some(UnwrapKind::Unwrap), e.span)
|
||||
},
|
||||
_ => (None, span),
|
||||
}
|
||||
} else {
|
||||
(None, span)
|
||||
};
|
||||
|
||||
Some(IterUsage {
|
||||
kind,
|
||||
unwrap_kind,
|
||||
span,
|
||||
})
|
||||
}
|
||||
|
|
@ -33,6 +33,7 @@ mod iter_nth_zero;
|
|||
mod iter_skip_next;
|
||||
mod iterator_step_by_zero;
|
||||
mod manual_saturating_arithmetic;
|
||||
mod manual_split_once;
|
||||
mod manual_str_repeat;
|
||||
mod map_collect_result_unit;
|
||||
mod map_flatten;
|
||||
|
|
@ -64,6 +65,7 @@ mod wrong_self_convention;
|
|||
mod zst_offset;
|
||||
|
||||
use bind_instead_of_map::BindInsteadOfMap;
|
||||
use clippy_utils::consts::{constant, Constant};
|
||||
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
|
||||
use clippy_utils::ty::{contains_adt_constructor, contains_ty, implements_trait, is_copy, is_type_diagnostic_item};
|
||||
use clippy_utils::{contains_return, get_trait_def_id, in_macro, iter_input_pats, meets_msrv, msrvs, paths, return_ty};
|
||||
|
|
@ -1771,6 +1773,31 @@ declare_clippy_lint! {
|
|||
"manual implementation of `str::repeat`"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for usages of `splitn(2, _)`
|
||||
///
|
||||
/// **Why is this bad?** `split_once` is clearer.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust
|
||||
/// // Bad
|
||||
/// let some_str = "name=value";
|
||||
/// let mut iter = some_str.splitn(2, '=');
|
||||
/// let name = iter.next().unwrap();
|
||||
/// let value = iter.next().unwrap_or("");
|
||||
///
|
||||
/// // Good
|
||||
/// let some_str = "name=value";
|
||||
/// let (name, value) = some_str.split_once('=').unwrap_or((some_str, ""));
|
||||
/// ```
|
||||
pub MANUAL_SPLIT_ONCE,
|
||||
complexity,
|
||||
"replace `.splitn(2, pat)` with `.split_once(pat)`"
|
||||
}
|
||||
|
||||
pub struct Methods {
|
||||
avoid_breaking_exported_api: bool,
|
||||
msrv: Option<RustcVersion>,
|
||||
|
|
@ -1848,7 +1875,8 @@ impl_lint_pass!(Methods => [
|
|||
IMPLICIT_CLONE,
|
||||
SUSPICIOUS_SPLITN,
|
||||
MANUAL_STR_REPEAT,
|
||||
EXTEND_WITH_DRAIN
|
||||
EXTEND_WITH_DRAIN,
|
||||
MANUAL_SPLIT_ONCE
|
||||
]);
|
||||
|
||||
/// Extracts a method call name, args, and `Span` of the method name.
|
||||
|
|
@ -2176,8 +2204,18 @@ 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);
|
||||
("splitn" | "rsplitn", [count_arg, pat_arg]) => {
|
||||
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
|
||||
suspicious_splitn::check(cx, name, expr, recv, count);
|
||||
if count == 2 && meets_msrv(msrv, &msrvs::STR_SPLIT_ONCE) {
|
||||
manual_split_once::check(cx, name, expr, recv, pat_arg);
|
||||
}
|
||||
}
|
||||
},
|
||||
("splitn_mut" | "rsplitn_mut", [count_arg, _]) => {
|
||||
if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
|
||||
suspicious_splitn::check(cx, name, expr, recv, count);
|
||||
}
|
||||
},
|
||||
("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg),
|
||||
("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
use clippy_utils::consts::{constant, Constant};
|
||||
use clippy_utils::diagnostics::span_lint_and_note;
|
||||
use if_chain::if_chain;
|
||||
use rustc_ast::LitKind;
|
||||
|
|
@ -8,15 +7,8 @@ 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<'_>,
|
||||
) {
|
||||
pub(super) fn check(cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, self_arg: &Expr<'_>, count: u128) {
|
||||
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);
|
||||
|
|
@ -24,9 +16,9 @@ pub(super) fn check(
|
|||
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([]))
|
||||
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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ define_Conf! {
|
|||
///
|
||||
/// Suppress lints whenever the suggested change would cause breakage for other crates.
|
||||
(avoid_breaking_exported_api: bool = true),
|
||||
/// Lint: MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE.
|
||||
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE.
|
||||
///
|
||||
/// The minimum rust version that the project supports
|
||||
(msrv: Option<String> = None),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue