Skip to content

Commit

Permalink
chore: use multipart_suggestions for str_splitn (#13789)
Browse files Browse the repository at this point in the history
This addresses #13099 for the manual_split_once test, using the
str_splitn lint.

changelog: [str_splitn]: Updated str_splitn to use multipart_suggestions
where appropriate
  • Loading branch information
Jarcho authored Dec 6, 2024
2 parents f83a227 + dcc4025 commit 5198941
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 80 deletions.
22 changes: 13 additions & 9 deletions clippy_lints/src/methods/str_splitn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn check_manual_split_once_indirect(
let ctxt = expr.span.ctxt();
let mut parents = cx.tcx.hir().parent_iter(expr.hir_id);
if let (_, Node::LetStmt(local)) = parents.next()?
&& let PatKind::Binding(BindingMode::MUT, iter_binding_id, iter_ident, None) = local.pat.kind
&& let PatKind::Binding(BindingMode::MUT, iter_binding_id, _, None) = local.pat.kind
&& let (iter_stmt_id, Node::Stmt(_)) = parents.next()?
&& let (_, Node::Block(enclosing_block)) = parents.next()?
&& let mut stmts = enclosing_block
Expand Down Expand Up @@ -162,16 +162,20 @@ fn check_manual_split_once_indirect(
UnwrapKind::Unwrap => ".unwrap()",
UnwrapKind::QuestionMark => "?",
};
diag.span_suggestion_verbose(
local.span,
format!("try `{r}split_once`"),
format!("let ({lhs}, {rhs}) = {self_snip}.{r}split_once({pat_snip}){unwrap};"),

// Add a multipart suggestion
diag.multipart_suggestion(
format!("replace with `{r}split_once`"),
vec![
(
local.span,
format!("let ({lhs}, {rhs}) = {self_snip}.{r}split_once({pat_snip}){unwrap};"),
),
(first.span, String::new()), // Remove the first usage
(second.span, String::new()), // Remove the second usage
],
app,
);

let remove_msg = format!("remove the `{iter_ident}` usages");
diag.span_suggestion(first.span, remove_msg.clone(), "", app);
diag.span_suggestion(second.span, remove_msg, "", app);
});
}

Expand Down
144 changes: 144 additions & 0 deletions tests/ui/manual_split_once.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
#![warn(clippy::manual_split_once)]
#![allow(unused, clippy::iter_skip_next, clippy::iter_nth_zero)]

extern crate itertools;

#[allow(unused_imports)]
use itertools::Itertools;

fn main() {
let _ = "key=value".splitn(2, '=').nth(2);
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('=').unwrap().1;

let s = Box::<str>::from("key=value");
let _ = s.split_once('=').unwrap().1;

let s = &"key=value";
let _ = s.split_once('=').unwrap().1;

fn _f(s: &str) -> Option<&str> {
let _ = s.split_once('=')?.1;
let _ = s.split_once('=')?.1;
let _ = s.rsplit_once('=')?.0;
let _ = s.rsplit_once('=')?.0;
None
}

// Don't lint, slices don't have `split_once`
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".rsplit_once('=').unwrap().0;
let (_, _) = "key=value".rsplit_once('=').map(|(x, y)| (y, x)).unwrap();
let _ = s.rsplit_once('=').map(|x| x.0);
}

fn indirect() -> Option<()> {
let (l, r) = "a.b.c".split_once('.').unwrap();



let (l, r) = "a.b.c".split_once('.')?;



let (l, r) = "a.b.c".rsplit_once('.').unwrap();



let (l, r) = "a.b.c".rsplit_once('.')?;



// could lint, currently doesn't

let mut iter = "a.b.c".splitn(2, '.');
let other = 1;
let l = iter.next()?;
let r = iter.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let mut mut_binding = iter.next()?;
let r = iter.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let tuple = (iter.next()?, iter.next()?);

// should not lint

let mut missing_unwrap = "a.b.c".splitn(2, '.');
let l = missing_unwrap.next();
let r = missing_unwrap.next();

let mut mixed_unrap = "a.b.c".splitn(2, '.');
let unwrap = mixed_unrap.next().unwrap();
let question_mark = mixed_unrap.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let same_name = iter.next()?;
let same_name = iter.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let shadows_existing = "d";
let shadows_existing = iter.next()?;
let r = iter.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let becomes_shadowed = iter.next()?;
let becomes_shadowed = "d";
let r = iter.next()?;

let mut iter = "a.b.c".splitn(2, '.');
let l = iter.next()?;
let r = iter.next()?;
let third_usage = iter.next()?;

let mut n_three = "a.b.c".splitn(3, '.');
let l = n_three.next()?;
let r = n_three.next()?;

let mut iter = "a.b.c".splitn(2, '.');
{
let in_block = iter.next()?;
}
let r = iter.next()?;

let mut lacks_binding = "a.b.c".splitn(2, '.');
let _ = lacks_binding.next()?;
let r = lacks_binding.next()?;

let mut mapped = "a.b.c".splitn(2, '.').map(|_| "~");
let l = iter.next()?;
let r = iter.next()?;

let mut assigned = "";
let mut iter = "a.b.c".splitn(2, '.');
let l = iter.next()?;
assigned = iter.next()?;

None
}

#[clippy::msrv = "1.51"]
fn _msrv_1_51() {
// `str::split_once` was stabilized in 1.52. Do not lint this
let _ = "key=value".splitn(2, '=').nth(1).unwrap();

let mut iter = "a.b.c".splitn(2, '.');
let a = iter.next().unwrap();
let b = iter.next().unwrap();
}

#[clippy::msrv = "1.52"]
fn _msrv_1_52() {
let _ = "key=value".split_once('=').unwrap().1;

let (a, b) = "a.b.c".split_once('.').unwrap();


}
2 changes: 0 additions & 2 deletions tests/ui/manual_split_once.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#![warn(clippy::manual_split_once)]
#![allow(unused, clippy::iter_skip_next, clippy::iter_nth_zero)]

//@no-rustfix: need to change the suggestion to a multipart suggestion

extern crate itertools;

#[allow(unused_imports)]
Expand Down
Loading

0 comments on commit 5198941

Please sign in to comment.