Skip to content

Commit b3d81e3

Browse files
authored
str_split: reduce suggestion diff (#16418)
Now, only `call_span` is replaced, so the receiver is not a part of the diff. This also removes the need to create a snippet for the receiver. changelog: [`str_split`]: reduce suggestion diff
2 parents 301aae6 + 3157c0b commit b3d81e3

5 files changed

Lines changed: 103 additions & 39 deletions

File tree

clippy_lints/src/methods/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5551,7 +5551,7 @@ impl Methods {
55515551
unnecessary_sort_by::check(cx, expr, call_span, arg, true);
55525552
},
55535553
(sym::split, [arg]) => {
5554-
str_split::check(cx, expr, recv, arg);
5554+
str_split::check(cx, expr, recv, call_span, arg);
55555555
},
55565556
(sym::splitn | sym::rsplitn, [count_arg, pat_arg]) => {
55575557
if let Some(Constant::Int(count)) = ConstEvalCtxt::new(cx).eval(count_arg) {
Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,48 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::snippet_with_context;
1+
use clippy_utils::diagnostics::span_lint_and_then;
32
use clippy_utils::sym;
43
use clippy_utils::visitors::is_const_evaluatable;
54
use rustc_ast::ast::LitKind;
65
use rustc_errors::Applicability;
76
use rustc_hir::{Expr, ExprKind};
87
use rustc_lint::LateContext;
8+
use rustc_span::Span;
99

1010
use super::STR_SPLIT_AT_NEWLINE;
1111

12-
pub(super) fn check<'a>(cx: &LateContext<'a>, expr: &'_ Expr<'_>, split_recv: &'a Expr<'_>, split_arg: &'_ Expr<'_>) {
12+
pub(super) fn check<'a>(
13+
cx: &LateContext<'a>,
14+
expr: &'_ Expr<'_>,
15+
split_recv: &'a Expr<'_>,
16+
split_span: Span,
17+
split_arg: &'_ Expr<'_>,
18+
) {
1319
// We're looking for `A.trim().split(B)`, where the adjusted type of `A` is `&str` (e.g. an
1420
// expression returning `String`), and `B` is a `Pattern` that hard-codes a newline (either `"\n"`
1521
// or `"\r\n"`). There are a lot of ways to specify a pattern, and this lint only checks the most
1622
// basic ones: a `'\n'`, `"\n"`, and `"\r\n"`.
17-
if let ExprKind::MethodCall(trim_method_name, trim_recv, [], _) = split_recv.kind
23+
if let ExprKind::MethodCall(trim_method_name, trim_recv, [], trim_span) = split_recv.kind
1824
&& trim_method_name.ident.name == sym::trim
1925
&& cx.typeck_results().expr_ty_adjusted(trim_recv).peel_refs().is_str()
2026
&& !is_const_evaluatable(cx, trim_recv)
2127
&& let ExprKind::Lit(split_lit) = split_arg.kind
22-
&& (matches!(split_lit.node, LitKind::Char('\n'))
23-
|| matches!(split_lit.node, LitKind::Str(sym::LF | sym::CRLF, _)))
28+
&& matches!(
29+
split_lit.node,
30+
LitKind::Char('\n') | LitKind::Str(sym::LF | sym::CRLF, _)
31+
)
2432
{
25-
let mut app = Applicability::MaybeIncorrect;
26-
span_lint_and_sugg(
33+
span_lint_and_then(
2734
cx,
2835
STR_SPLIT_AT_NEWLINE,
2936
expr.span,
3037
"using `str.trim().split()` with hard-coded newlines",
31-
"use `str.lines()` instead",
32-
format!(
33-
"{}.lines()",
34-
snippet_with_context(cx, trim_recv.span, expr.span.ctxt(), "..", &mut app).0
35-
),
36-
app,
38+
|diag| {
39+
diag.span_suggestion_verbose(
40+
trim_span.to(split_span), // combine the call spans of the two methods
41+
"use `str.lines()` instead",
42+
"lines()",
43+
Applicability::MaybeIncorrect,
44+
);
45+
},
3746
);
3847
}
3948
}

tests/ui/str_split.fixed

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#![warn(clippy::str_split_at_newline)]
2-
#![allow(clippy::needless_lifetimes)]
32

4-
use core::str::Split;
53
use std::ops::Deref;
64

75
struct NotStr<'a> {

tests/ui/str_split.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#![warn(clippy::str_split_at_newline)]
2-
#![allow(clippy::needless_lifetimes)]
32

4-
use core::str::Split;
53
use std::ops::Deref;
64

75
struct NotStr<'a> {

tests/ui/str_split.stderr

Lines changed: 79 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,124 @@
11
error: using `str.trim().split()` with hard-coded newlines
2-
--> tests/ui/str_split.rs:60:13
2+
--> tests/ui/str_split.rs:58:13
33
|
44
LL | let _ = s1.trim().split('\n');
5-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
5+
| ^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::str-split-at-newline` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::str_split_at_newline)]`
9+
help: use `str.lines()` instead
10+
|
11+
LL - let _ = s1.trim().split('\n');
12+
LL + let _ = s1.lines();
13+
|
914

1015
error: using `str.trim().split()` with hard-coded newlines
11-
--> tests/ui/str_split.rs:63:13
16+
--> tests/ui/str_split.rs:61:13
1217
|
1318
LL | let _ = s1.trim().split("\n");
14-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
19+
| ^^^^^^^^^^^^^^^^^^^^^
20+
|
21+
help: use `str.lines()` instead
22+
|
23+
LL - let _ = s1.trim().split("\n");
24+
LL + let _ = s1.lines();
25+
|
1526

1627
error: using `str.trim().split()` with hard-coded newlines
17-
--> tests/ui/str_split.rs:65:13
28+
--> tests/ui/str_split.rs:63:13
1829
|
1930
LL | let _ = s1.trim().split("\r\n");
20-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
31+
| ^^^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
help: use `str.lines()` instead
34+
|
35+
LL - let _ = s1.trim().split("\r\n");
36+
LL + let _ = s1.lines();
37+
|
2138

2239
error: using `str.trim().split()` with hard-coded newlines
23-
--> tests/ui/str_split.rs:69:13
40+
--> tests/ui/str_split.rs:67:13
2441
|
2542
LL | let _ = s2.trim().split('\n');
26-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
43+
| ^^^^^^^^^^^^^^^^^^^^^
44+
|
45+
help: use `str.lines()` instead
46+
|
47+
LL - let _ = s2.trim().split('\n');
48+
LL + let _ = s2.lines();
49+
|
2750

2851
error: using `str.trim().split()` with hard-coded newlines
29-
--> tests/ui/str_split.rs:72:13
52+
--> tests/ui/str_split.rs:70:13
3053
|
3154
LL | let _ = s2.trim().split("\n");
32-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
55+
| ^^^^^^^^^^^^^^^^^^^^^
56+
|
57+
help: use `str.lines()` instead
58+
|
59+
LL - let _ = s2.trim().split("\n");
60+
LL + let _ = s2.lines();
61+
|
3362

3463
error: using `str.trim().split()` with hard-coded newlines
35-
--> tests/ui/str_split.rs:74:13
64+
--> tests/ui/str_split.rs:72:13
3665
|
3766
LL | let _ = s2.trim().split("\r\n");
38-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
67+
| ^^^^^^^^^^^^^^^^^^^^^^^
68+
|
69+
help: use `str.lines()` instead
70+
|
71+
LL - let _ = s2.trim().split("\r\n");
72+
LL + let _ = s2.lines();
73+
|
3974

4075
error: using `str.trim().split()` with hard-coded newlines
41-
--> tests/ui/str_split.rs:79:13
76+
--> tests/ui/str_split.rs:77:13
4277
|
4378
LL | let _ = s3.trim().split('\n');
44-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
79+
| ^^^^^^^^^^^^^^^^^^^^^
80+
|
81+
help: use `str.lines()` instead
82+
|
83+
LL - let _ = s3.trim().split('\n');
84+
LL + let _ = s3.lines();
85+
|
4586

4687
error: using `str.trim().split()` with hard-coded newlines
47-
--> tests/ui/str_split.rs:82:13
88+
--> tests/ui/str_split.rs:80:13
4889
|
4990
LL | let _ = s3.trim().split("\n");
50-
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
91+
| ^^^^^^^^^^^^^^^^^^^^^
92+
|
93+
help: use `str.lines()` instead
94+
|
95+
LL - let _ = s3.trim().split("\n");
96+
LL + let _ = s3.lines();
97+
|
5198

5299
error: using `str.trim().split()` with hard-coded newlines
53-
--> tests/ui/str_split.rs:84:13
100+
--> tests/ui/str_split.rs:82:13
54101
|
55102
LL | let _ = s3.trim().split("\r\n");
56-
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
103+
| ^^^^^^^^^^^^^^^^^^^^^^^
104+
|
105+
help: use `str.lines()` instead
106+
|
107+
LL - let _ = s3.trim().split("\r\n");
108+
LL + let _ = s3.lines();
109+
|
57110

58111
error: using `str.trim().split()` with hard-coded newlines
59-
--> tests/ui/str_split.rs:88:13
112+
--> tests/ui/str_split.rs:86:13
60113
|
61114
LL | let _ = make_str!(s1).trim().split('\n');
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `make_str!(s1).lines()`
115+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
116+
|
117+
help: use `str.lines()` instead
118+
|
119+
LL - let _ = make_str!(s1).trim().split('\n');
120+
LL + let _ = make_str!(s1).lines();
121+
|
63122

64123
error: aborting due to 10 previous errors
65124

0 commit comments

Comments
 (0)