From 6fba89751bfacf59109d37331ec79a8f87ee0aaf Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Mon, 4 Apr 2022 16:30:38 +0100 Subject: [PATCH] Remove overlap between `manual_split_once` and `needless_splitn` Also fixes some incorrect suggestions for rsplitn --- clippy_lints/src/methods/mod.rs | 7 +- clippy_lints/src/methods/str_splitn.rs | 246 ++++++++----------------- tests/ui/crashes/ice-8250.stderr | 10 +- tests/ui/manual_split_once.fixed | 21 +-- tests/ui/manual_split_once.rs | 21 +-- tests/ui/manual_split_once.stderr | 96 +++++----- tests/ui/needless_splitn.fixed | 20 ++ tests/ui/needless_splitn.rs | 20 ++ tests/ui/needless_splitn.stderr | 44 ++++- 9 files changed, 221 insertions(+), 264 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 021337280d1..7047aa4c69b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2516,12 +2516,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("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) { - str_splitn::check_manual_split_once(cx, name, expr, recv, pat_arg); - } - if count >= 2 { - str_splitn::check_needless_splitn(cx, name, expr, recv, pat_arg, count); - } + str_splitn::check(cx, name, expr, recv, pat_arg, count, msrv); } }, ("splitn_mut" | "rsplitn_mut", [count_arg, _]) => { diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs index 8125930b304..6eaa9eb2ef0 100644 --- a/clippy_lints/src/methods/str_splitn.rs +++ b/clippy_lints/src/methods/str_splitn.rs @@ -1,36 +1,77 @@ 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 clippy_utils::{is_diag_item_method, match_def_path, meets_msrv, msrvs, 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_middle::ty; +use rustc_semver::RustcVersion; use rustc_span::{symbol::sym, Span, SyntaxContext}; -use super::MANUAL_SPLIT_ONCE; +use super::{MANUAL_SPLIT_ONCE, NEEDLESS_SPLITN}; -pub(super) fn check_manual_split_once( +pub(super) fn check( cx: &LateContext<'_>, method_name: &str, expr: &Expr<'_>, self_arg: &Expr<'_>, pat_arg: &Expr<'_>, + count: u128, + msrv: Option<&RustcVersion>, ) { - if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() { + if count < 2 || !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() { return; } let ctxt = expr.span.ctxt(); - let (method_name, msg, reverse) = if method_name == "splitn" { - ("split_once", "manual implementation of `split_once`", false) - } else { - ("rsplit_once", "manual implementation of `rsplit_once`", true) + let Some(usage) = parse_iter_usage(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id)) else { return }; + + let needless = match usage.kind { + IterUsageKind::Nth(n) => count > n + 1, + IterUsageKind::NextTuple => count > 2, }; - let usage = match parse_iter_usage(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id), reverse) { - Some(x) => x, - None => return, + + if needless { + let mut app = Applicability::MachineApplicable; + let (r, message) = if method_name == "splitn" { + ("", "unnecessary use of `splitn`") + } else { + ("r", "unnecessary use of `rsplitn`") + }; + + span_lint_and_sugg( + cx, + NEEDLESS_SPLITN, + expr.span, + message, + "try this", + format!( + "{}.{r}split({})", + snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0, + snippet_with_context(cx, pat_arg.span, ctxt, "..", &mut app).0, + ), + app, + ); + } else if count == 2 && meets_msrv(msrv, &msrvs::STR_SPLIT_ONCE) { + check_manual_split_once(cx, method_name, expr, self_arg, pat_arg, &usage); + } +} + +fn check_manual_split_once( + cx: &LateContext<'_>, + method_name: &str, + expr: &Expr<'_>, + self_arg: &Expr<'_>, + pat_arg: &Expr<'_>, + usage: &IterUsage, +) { + let ctxt = expr.span.ctxt(); + let (msg, reverse) = if method_name == "splitn" { + ("manual implementation of `split_once`", false) + } else { + ("manual implementation of `rsplit_once`", true) }; let mut app = Applicability::MachineApplicable; @@ -39,77 +80,36 @@ pub(super) fn check_manual_split_once( let sugg = match usage.kind { IterUsageKind::NextTuple => { - format!("{}.{}({})", self_snip, method_name, pat_snip) - }, - IterUsageKind::RNextTuple => format!("{}.{}({}).map(|(x, y)| (y, x))", self_snip, method_name, pat_snip), - IterUsageKind::Next | IterUsageKind::Second => { - let self_deref = { - let adjust = cx.typeck_results().expr_adjustments(self_arg); - if adjust.len() < 2 { - 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().saturating_sub(1))) - } else { - "*".repeat(adjust.len().saturating_sub(2)) - } - }; - if matches!(usage.kind, IterUsageKind::Next) { - match usage.unwrap_kind { - Some(UnwrapKind::Unwrap) => { - if reverse { - format!("{}.{}({}).unwrap().0", self_snip, method_name, pat_snip) - } else { - format!( - "{}.{}({}).map_or({}{}, |x| x.0)", - self_snip, method_name, pat_snip, self_deref, &self_snip - ) - } - }, - Some(UnwrapKind::QuestionMark) => { - format!( - "{}.{}({}).map_or({}{}, |x| x.0)", - self_snip, method_name, pat_snip, self_deref, &self_snip - ) - }, - None => { - format!( - "Some({}.{}({}).map_or({}{}, |x| x.0))", - &self_snip, method_name, pat_snip, self_deref, &self_snip - ) - }, - } + if reverse { + format!("{self_snip}.rsplit_once({pat_snip}).map(|(x, y)| (y, x))") } else { - match usage.unwrap_kind { - Some(UnwrapKind::Unwrap) => { - if reverse { - // In this case, no better suggestion is offered. - return; - } - format!("{}.{}({}).unwrap().1", self_snip, method_name, pat_snip) - }, - Some(UnwrapKind::QuestionMark) => { - format!("{}.{}({})?.1", self_snip, method_name, pat_snip) - }, - None => { - format!("{}.{}({}).map(|x| x.1)", self_snip, method_name, pat_snip) - }, - } + format!("{self_snip}.split_once({pat_snip})") } }, + IterUsageKind::Nth(1) => { + let (r, field) = if reverse { ("r", 0) } else { ("", 1) }; + + match usage.unwrap_kind { + Some(UnwrapKind::Unwrap) => { + format!("{self_snip}.{r}split_once({pat_snip}).unwrap().{field}") + }, + Some(UnwrapKind::QuestionMark) => { + format!("{self_snip}.{r}split_once({pat_snip})?.{field}") + }, + None => { + format!("{self_snip}.{r}split_once({pat_snip}).map(|x| x.{field})") + }, + } + }, + IterUsageKind::Nth(_) => return, }; span_lint_and_sugg(cx, MANUAL_SPLIT_ONCE, usage.span, msg, "try this", sugg, app); } enum IterUsageKind { - Next, - Second, + Nth(u128), NextTuple, - RNextTuple, } enum UnwrapKind { @@ -128,7 +128,6 @@ fn parse_iter_usage<'tcx>( cx: &LateContext<'tcx>, ctxt: SyntaxContext, mut iter: impl Iterator)>, - reverse: bool, ) -> Option { let (kind, span) = match iter.next() { Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => { @@ -141,13 +140,7 @@ fn parse_iter_usage<'tcx>( 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) => { - if reverse { - (IterUsageKind::Second, e.span) - } else { - (IterUsageKind::Next, e.span) - } - }, + ("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => (IterUsageKind::Nth(0), e.span), ("next_tuple", []) => { return if_chain! { if match_def_path(cx, did, &paths::ITERTOOLS_NEXT_TUPLE); @@ -157,7 +150,7 @@ fn parse_iter_usage<'tcx>( if subs.len() == 2; then { Some(IterUsage { - kind: if reverse { IterUsageKind::RNextTuple } else { IterUsageKind::NextTuple }, + kind: IterUsageKind::NextTuple, span: e.span, unwrap_kind: None }) @@ -185,11 +178,7 @@ fn parse_iter_usage<'tcx>( } } }; - match if reverse { idx ^ 1 } else { idx } { - 0 => (IterUsageKind::Next, span), - 1 => (IterUsageKind::Second, span), - _ => return None, - } + (IterUsageKind::Nth(idx), span) } else { return None; } @@ -238,86 +227,3 @@ fn parse_iter_usage<'tcx>( span, }) } - -use super::NEEDLESS_SPLITN; - -pub(super) fn check_needless_splitn( - cx: &LateContext<'_>, - method_name: &str, - expr: &Expr<'_>, - self_arg: &Expr<'_>, - pat_arg: &Expr<'_>, - count: u128, -) { - if !cx.typeck_results().expr_ty_adjusted(self_arg).peel_refs().is_str() { - return; - } - let ctxt = expr.span.ctxt(); - let mut app = Applicability::MachineApplicable; - let (reverse, message) = if method_name == "splitn" { - (false, "unnecessary use of `splitn`") - } else { - (true, "unnecessary use of `rsplitn`") - }; - if_chain! { - if count >= 2; - if check_iter(cx, ctxt, cx.tcx.hir().parent_iter(expr.hir_id), count); - then { - span_lint_and_sugg( - cx, - NEEDLESS_SPLITN, - expr.span, - message, - "try this", - format!( - "{}.{}({})", - snippet_with_context(cx, self_arg.span, ctxt, "..", &mut app).0, - if reverse {"rsplit"} else {"split"}, - snippet_with_context(cx, pat_arg.span, ctxt, "..", &mut app).0 - ), - app, - ); - } - } -} - -fn check_iter<'tcx>( - cx: &LateContext<'tcx>, - ctxt: SyntaxContext, - mut iter: impl Iterator)>, - count: u128, -) -> bool { - 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 false; - }; - if_chain! { - if let Some(did) = cx.typeck_results().type_dependent_def_id(e.hir_id); - if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator); - then { - match (name.ident.as_str(), args) { - ("next", []) if cx.tcx.trait_of_item(did) == Some(iter_id) => { - return true; - }, - ("next_tuple", []) if count > 2 => { - return true; - }, - ("nth", [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) { - if count > idx + 1 { - return true; - } - } - }, - _ => return false, - } - } - } - }, - _ => return false, - }; - false -} diff --git a/tests/ui/crashes/ice-8250.stderr b/tests/ui/crashes/ice-8250.stderr index 04ea4456656..8ed8f3b3a06 100644 --- a/tests/ui/crashes/ice-8250.stderr +++ b/tests/ui/crashes/ice-8250.stderr @@ -1,11 +1,3 @@ -error: manual implementation of `split_once` - --> $DIR/ice-8250.rs:2:13 - | -LL | let _ = s[1..].splitn(2, '.').next()?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s[1..].split_once('.').map_or(s[1..], |x| x.0)` - | - = note: `-D clippy::manual-split-once` implied by `-D warnings` - error: unnecessary use of `splitn` --> $DIR/ice-8250.rs:2:13 | @@ -14,5 +6,5 @@ LL | let _ = s[1..].splitn(2, '.').next()?; | = note: `-D clippy::needless-splitn` implied by `-D warnings` -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/tests/ui/manual_split_once.fixed b/tests/ui/manual_split_once.fixed index d5113df569a..8621ddc7467 100644 --- a/tests/ui/manual_split_once.fixed +++ b/tests/ui/manual_split_once.fixed @@ -2,7 +2,7 @@ #![feature(custom_inner_attributes)] #![warn(clippy::manual_split_once)] -#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::needless_splitn)] +#![allow(clippy::iter_skip_next, clippy::iter_nth_zero)] extern crate itertools; @@ -10,27 +10,25 @@ extern crate itertools; use itertools::Itertools; fn main() { - let _ = Some("key=value".split_once('=').map_or("key=value", |x| x.0)); let _ = "key=value".splitn(2, '=').nth(2); - let _ = "key=value".split_once('=').map_or("key=value", |x| x.0); - let _ = "key=value".split_once('=').map_or("key=value", |x| x.0); let _ = "key=value".split_once('=').unwrap().1; let _ = "key=value".split_once('=').unwrap().1; let (_, _) = "key=value".split_once('=').unwrap(); let s = String::from("key=value"); - let _ = s.split_once('=').map_or(&*s, |x| x.0); + let _ = s.split_once('=').unwrap().1; let s = Box::::from("key=value"); - let _ = s.split_once('=').map_or(&*s, |x| x.0); + let _ = s.split_once('=').unwrap().1; let s = &"key=value"; - let _ = s.split_once('=').map_or(*s, |x| x.0); + let _ = s.split_once('=').unwrap().1; fn _f(s: &str) -> Option<&str> { - let _ = s.split_once("key=value").map_or(s, |x| x.0); - let _ = s.split_once("key=value")?.1; - let _ = s.split_once("key=value")?.1; + let _ = s.split_once('=')?.1; + let _ = s.split_once('=')?.1; + let _ = s.rsplit_once('=')?.0; + let _ = s.rsplit_once('=')?.0; None } @@ -38,10 +36,9 @@ fn main() { let _ = [0, 1, 2].splitn(2, |&x| x == 1).nth(1).unwrap(); // `rsplitn` gives the results in the reverse order of `rsplit_once` - let _ = "key=value".rsplitn(2, '=').next().unwrap(); let _ = "key=value".rsplit_once('=').unwrap().0; - let _ = "key=value".rsplit_once('=').map(|x| x.1); let (_, _) = "key=value".rsplit_once('=').map(|(x, y)| (y, x)).unwrap(); + let _ = s.rsplit_once('=').map(|x| x.0); } fn _msrv_1_51() { diff --git a/tests/ui/manual_split_once.rs b/tests/ui/manual_split_once.rs index 80e02952dbd..c8a1d7b91e5 100644 --- a/tests/ui/manual_split_once.rs +++ b/tests/ui/manual_split_once.rs @@ -2,7 +2,7 @@ #![feature(custom_inner_attributes)] #![warn(clippy::manual_split_once)] -#![allow(clippy::iter_skip_next, clippy::iter_nth_zero, clippy::needless_splitn)] +#![allow(clippy::iter_skip_next, clippy::iter_nth_zero)] extern crate itertools; @@ -10,27 +10,25 @@ use itertools::Itertools; fn main() { - let _ = "key=value".splitn(2, '=').next(); let _ = "key=value".splitn(2, '=').nth(2); - let _ = "key=value".splitn(2, '=').next().unwrap(); - let _ = "key=value".splitn(2, '=').nth(0).unwrap(); let _ = "key=value".splitn(2, '=').nth(1).unwrap(); let _ = "key=value".splitn(2, '=').skip(1).next().unwrap(); let (_, _) = "key=value".splitn(2, '=').next_tuple().unwrap(); let s = String::from("key=value"); - let _ = s.splitn(2, '=').next().unwrap(); + let _ = s.splitn(2, '=').nth(1).unwrap(); let s = Box::::from("key=value"); - let _ = s.splitn(2, '=').nth(0).unwrap(); + let _ = s.splitn(2, '=').nth(1).unwrap(); let s = &"key=value"; - let _ = s.splitn(2, '=').skip(0).next().unwrap(); + let _ = s.splitn(2, '=').skip(1).next().unwrap(); fn _f(s: &str) -> Option<&str> { - let _ = s.splitn(2, "key=value").next()?; - let _ = s.splitn(2, "key=value").nth(1)?; - let _ = s.splitn(2, "key=value").skip(1).next()?; + let _ = s.splitn(2, '=').nth(1)?; + let _ = s.splitn(2, '=').skip(1).next()?; + let _ = s.rsplitn(2, '=').nth(1)?; + let _ = s.rsplitn(2, '=').skip(1).next()?; None } @@ -38,10 +36,9 @@ fn _f(s: &str) -> Option<&str> { let _ = [0, 1, 2].splitn(2, |&x| x == 1).nth(1).unwrap(); // `rsplitn` gives the results in the reverse order of `rsplit_once` - let _ = "key=value".rsplitn(2, '=').next().unwrap(); let _ = "key=value".rsplitn(2, '=').nth(1).unwrap(); - let _ = "key=value".rsplitn(2, '=').nth(0); let (_, _) = "key=value".rsplitn(2, '=').next_tuple().unwrap(); + let _ = s.rsplitn(2, '=').nth(1); } fn _msrv_1_51() { diff --git a/tests/ui/manual_split_once.stderr b/tests/ui/manual_split_once.stderr index af9c7a2d41b..9635e6c5e4b 100644 --- a/tests/ui/manual_split_once.stderr +++ b/tests/ui/manual_split_once.stderr @@ -1,100 +1,88 @@ error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:13:13 + --> $DIR/manual_split_once.rs:14:13 | -LL | let _ = "key=value".splitn(2, '=').next(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Some("key=value".split_once('=').map_or("key=value", |x| x.0))` +LL | let _ = "key=value".splitn(2, '=').nth(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1` | = note: `-D clippy::manual-split-once` implied by `-D warnings` error: manual implementation of `split_once` --> $DIR/manual_split_once.rs:15:13 | -LL | let _ = "key=value".splitn(2, '=').next().unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').map_or("key=value", |x| x.0)` - -error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:16:13 - | -LL | let _ = "key=value".splitn(2, '=').nth(0).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').map_or("key=value", |x| x.0)` - -error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:17:13 - | -LL | let _ = "key=value".splitn(2, '=').nth(1).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1` - -error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:18:13 - | LL | let _ = "key=value".splitn(2, '=').skip(1).next().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1` error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:19:18 + --> $DIR/manual_split_once.rs:16:18 | LL | let (_, _) = "key=value".splitn(2, '=').next_tuple().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=')` +error: manual implementation of `split_once` + --> $DIR/manual_split_once.rs:19:13 + | +LL | let _ = s.splitn(2, '=').nth(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').unwrap().1` + error: manual implementation of `split_once` --> $DIR/manual_split_once.rs:22:13 | -LL | let _ = s.splitn(2, '=').next().unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').map_or(&*s, |x| x.0)` +LL | let _ = s.splitn(2, '=').nth(1).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').unwrap().1` error: manual implementation of `split_once` --> $DIR/manual_split_once.rs:25:13 | -LL | let _ = s.splitn(2, '=').nth(0).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').map_or(&*s, |x| x.0)` +LL | let _ = s.splitn(2, '=').skip(1).next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').unwrap().1` error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:28:13 + --> $DIR/manual_split_once.rs:28:17 | -LL | let _ = s.splitn(2, '=').skip(0).next().unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=').map_or(*s, |x| x.0)` +LL | let _ = s.splitn(2, '=').nth(1)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=')?.1` error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:31:17 + --> $DIR/manual_split_once.rs:29:17 | -LL | let _ = s.splitn(2, "key=value").next()?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once("key=value").map_or(s, |x| x.0)` - -error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:32:17 - | -LL | let _ = s.splitn(2, "key=value").nth(1)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once("key=value")?.1` - -error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:33:17 - | -LL | let _ = s.splitn(2, "key=value").skip(1).next()?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once("key=value")?.1` +LL | let _ = s.splitn(2, '=').skip(1).next()?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.split_once('=')?.1` error: manual implementation of `rsplit_once` - --> $DIR/manual_split_once.rs:42:13 + --> $DIR/manual_split_once.rs:30:17 + | +LL | let _ = s.rsplitn(2, '=').nth(1)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.rsplit_once('=')?.0` + +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:31:17 + | +LL | let _ = s.rsplitn(2, '=').skip(1).next()?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.rsplit_once('=')?.0` + +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:39:13 | LL | let _ = "key=value".rsplitn(2, '=').nth(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').unwrap().0` error: manual implementation of `rsplit_once` - --> $DIR/manual_split_once.rs:43:13 - | -LL | let _ = "key=value".rsplitn(2, '=').nth(0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map(|x| x.1)` - -error: manual implementation of `rsplit_once` - --> $DIR/manual_split_once.rs:44:18 + --> $DIR/manual_split_once.rs:40:18 | LL | let (_, _) = "key=value".rsplitn(2, '=').next_tuple().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".rsplit_once('=').map(|(x, y)| (y, x))` +error: manual implementation of `rsplit_once` + --> $DIR/manual_split_once.rs:41:13 + | +LL | let _ = s.rsplitn(2, '=').nth(1); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `s.rsplit_once('=').map(|x| x.0)` + error: manual implementation of `split_once` - --> $DIR/manual_split_once.rs:55:13 + --> $DIR/manual_split_once.rs:52:13 | LL | let _ = "key=value".splitn(2, '=').nth(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split_once('=').unwrap().1` -error: aborting due to 16 previous errors +error: aborting due to 14 previous errors diff --git a/tests/ui/needless_splitn.fixed b/tests/ui/needless_splitn.fixed index f6a4b2f17d3..61f5fc4e679 100644 --- a/tests/ui/needless_splitn.fixed +++ b/tests/ui/needless_splitn.fixed @@ -24,4 +24,24 @@ fn main() { let _ = str.rsplitn(2, '=').nth(1); let (_, _) = str.rsplitn(2, '=').next_tuple().unwrap(); let (_, _) = str.rsplit('=').next_tuple().unwrap(); + + let _ = str.split('=').next(); + let _ = str.split('=').nth(3); + let _ = str.splitn(5, '=').nth(4); + let _ = str.splitn(5, '=').nth(5); +} + +fn _question_mark(s: &str) -> Option<()> { + let _ = s.split('=').next()?; + let _ = s.split('=').nth(0)?; + let _ = s.rsplit('=').next()?; + let _ = s.rsplit('=').nth(0)?; + + Some(()) +} + +fn _test_msrv() { + #![clippy::msrv = "1.51"] + // `manual_split_once` MSRV shouldn't apply to `needless_splitn` + let _ = "key=value".split('=').nth(0).unwrap(); } diff --git a/tests/ui/needless_splitn.rs b/tests/ui/needless_splitn.rs index 6ba32255bb2..71d9a7077fa 100644 --- a/tests/ui/needless_splitn.rs +++ b/tests/ui/needless_splitn.rs @@ -24,4 +24,24 @@ fn main() { let _ = str.rsplitn(2, '=').nth(1); let (_, _) = str.rsplitn(2, '=').next_tuple().unwrap(); let (_, _) = str.rsplitn(3, '=').next_tuple().unwrap(); + + let _ = str.splitn(5, '=').next(); + let _ = str.splitn(5, '=').nth(3); + let _ = str.splitn(5, '=').nth(4); + let _ = str.splitn(5, '=').nth(5); +} + +fn _question_mark(s: &str) -> Option<()> { + let _ = s.splitn(2, '=').next()?; + let _ = s.splitn(2, '=').nth(0)?; + let _ = s.rsplitn(2, '=').next()?; + let _ = s.rsplitn(2, '=').nth(0)?; + + Some(()) +} + +fn _test_msrv() { + #![clippy::msrv = "1.51"] + // `manual_split_once` MSRV shouldn't apply to `needless_splitn` + let _ = "key=value".splitn(2, '=').nth(0).unwrap(); } diff --git a/tests/ui/needless_splitn.stderr b/tests/ui/needless_splitn.stderr index 66de2256554..f112b29e7f2 100644 --- a/tests/ui/needless_splitn.stderr +++ b/tests/ui/needless_splitn.stderr @@ -36,5 +36,47 @@ error: unnecessary use of `rsplitn` LL | let (_, _) = str.rsplitn(3, '=').next_tuple().unwrap(); | ^^^^^^^^^^^^^^^^^^^ help: try this: `str.rsplit('=')` -error: aborting due to 6 previous errors +error: unnecessary use of `splitn` + --> $DIR/needless_splitn.rs:28:13 + | +LL | let _ = str.splitn(5, '=').next(); + | ^^^^^^^^^^^^^^^^^^ help: try this: `str.split('=')` + +error: unnecessary use of `splitn` + --> $DIR/needless_splitn.rs:29:13 + | +LL | let _ = str.splitn(5, '=').nth(3); + | ^^^^^^^^^^^^^^^^^^ help: try this: `str.split('=')` + +error: unnecessary use of `splitn` + --> $DIR/needless_splitn.rs:35:13 + | +LL | let _ = s.splitn(2, '=').next()?; + | ^^^^^^^^^^^^^^^^ help: try this: `s.split('=')` + +error: unnecessary use of `splitn` + --> $DIR/needless_splitn.rs:36:13 + | +LL | let _ = s.splitn(2, '=').nth(0)?; + | ^^^^^^^^^^^^^^^^ help: try this: `s.split('=')` + +error: unnecessary use of `rsplitn` + --> $DIR/needless_splitn.rs:37:13 + | +LL | let _ = s.rsplitn(2, '=').next()?; + | ^^^^^^^^^^^^^^^^^ help: try this: `s.rsplit('=')` + +error: unnecessary use of `rsplitn` + --> $DIR/needless_splitn.rs:38:13 + | +LL | let _ = s.rsplitn(2, '=').nth(0)?; + | ^^^^^^^^^^^^^^^^^ help: try this: `s.rsplit('=')` + +error: unnecessary use of `splitn` + --> $DIR/needless_splitn.rs:46:13 + | +LL | let _ = "key=value".splitn(2, '=').nth(0).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `"key=value".split('=')` + +error: aborting due to 13 previous errors