Skip to content

Commit 37a0c79

Browse files
torfsentorfsen
committed
8733: Implement suggested changes
The following changes were recommended by @Jarcho: * Make `STR_SPLIT_AT_NEWLINE` `pedantic` instead of `suspicious` * Fix applicability regression (use `MaybeIncorrect` instead of `MachineApplicable`) * Don't warn when splitting constant strings * Merge `if` statements
1 parent bcb0b51 commit 37a0c79

File tree

5 files changed

+57
-104
lines changed

5 files changed

+57
-104
lines changed

‎clippy_lints/src/methods/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -3849,7 +3849,7 @@ declare_clippy_lint! {
38493849
/// ```
38503850
#[clippy::version = "1.76.0"]
38513851
pub STR_SPLIT_AT_NEWLINE,
3852-
suspicious,
3852+
pedantic,
38533853
"splitting a trimmed string at hard-coded newlines"
38543854
}
38553855

‎clippy_lints/src/methods/str_split.rs

+19-22
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,39 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_context;
3+
use clippy_utils::visitors::is_const_evaluatable;
34
use rustc_ast::ast::LitKind;
45
use rustc_errors::Applicability;
56
use rustc_hir::{Expr, ExprKind};
67
use rustc_lint::LateContext;
78

89
use super::STR_SPLIT_AT_NEWLINE;
910

10-
pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, split_recv: &'_ Expr<'_>, split_arg: &'_ Expr<'_>) {
11+
pub(super) fn check<'a>(cx: &LateContext<'a>, expr: &'_ Expr<'_>, split_recv: &'a Expr<'_>, split_arg: &'_ Expr<'_>) {
1112
// We're looking for `A.trim().split(B)`, where the adjusted type of `A` is `&str` (e.g. a `str`
1213
// literal or an expression returning `String`), and `B` is a `Pattern` that hard-codes a
1314
// newline (either `"\n"` or `"\r\n"`). There are a lot of ways to specify a pattern, and
1415
// this lint only checks the most basic ones: a `'\n'`, `"\n"`, and `"\r\n"`.
1516

16-
if let ExprKind::MethodCall(trim_method_name, trim_recv, trim_args, _) = split_recv.kind
17+
if let ExprKind::MethodCall(trim_method_name, trim_recv, [], _) = split_recv.kind
1718
&& trim_method_name.ident.as_str() == "trim"
18-
&& trim_args.is_empty()
1919
&& cx.typeck_results().expr_ty_adjusted(trim_recv).peel_refs().is_str()
20+
&& !is_const_evaluatable(cx, trim_recv)
2021
&& let ExprKind::Lit(split_lit) = split_arg.kind
22+
&& (matches!(split_lit.node, LitKind::Char('\n'))
23+
|| matches!(split_lit.node, LitKind::Str(sym, _) if (sym.as_str() == "\n" || sym.as_str() == "\r\n")))
2124
{
22-
let char_n = matches!(split_lit.node, LitKind::Char('\n'));
23-
let str_n_or_rn =
24-
matches!(split_lit.node, LitKind::Str(sym, _) if (sym.as_str() == "\n" || sym.as_str() == "\r\n"));
25-
26-
if char_n || str_n_or_rn {
27-
let mut app = Applicability::MachineApplicable;
28-
span_lint_and_sugg(
29-
cx,
30-
STR_SPLIT_AT_NEWLINE,
31-
expr.span,
32-
"using `str.trim().split()` with hard-coded newlines is not portable",
33-
"use `str.lines()` instead",
34-
format!(
35-
"{}.lines()",
36-
snippet_with_context(cx, trim_recv.span, expr.span.ctxt(), "..", &mut app).0
37-
),
38-
app,
39-
);
40-
}
25+
let mut app = Applicability::MaybeIncorrect;
26+
span_lint_and_sugg(
27+
cx,
28+
STR_SPLIT_AT_NEWLINE,
29+
expr.span,
30+
"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,
37+
);
4138
}
4239
}

‎tests/ui/str_split.fixed

+9-22
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ macro_rules! trim_split {
4444
}
4545

4646
macro_rules! make_str {
47-
() => {
48-
"hello\nworld\n"
47+
( $x: expr ) => {
48+
format!("x={}", $x)
4949
};
5050
}
5151

@@ -61,12 +61,6 @@ fn main() {
6161
let _ = s1.lines();
6262
let _ = s1.lines();
6363

64-
// Splitting a `str` literal at "\n" or "\r\n" after trimming should warn
65-
let _ = "hello\nworld\n".lines();
66-
#[allow(clippy::single_char_pattern)]
67-
let _ = "hello\nworld\n".lines();
68-
let _ = "hello\nworld\n".lines();
69-
7064
// Splitting a `String` variable at "\n" or "\r\n" after trimming should warn
7165
let _ = s2.lines();
7266
#[allow(clippy::single_char_pattern)]
@@ -81,23 +75,23 @@ fn main() {
8175
let _ = s3.lines();
8276

8377
// If the `&str` is generated by a macro then the macro should not be expanded in the suggested fix.
84-
let _ = make_str!().lines();
78+
let _ = make_str!(s1).lines();
8579

8680
// CASES THAT SHOULD NOT EMIT A LINT
8781

82+
// Splitting a `str` constant at "\n" or "\r\n" after trimming should not warn
83+
let _ = "hello\nworld\n".trim().split('\n');
84+
#[allow(clippy::single_char_pattern)]
85+
let _ = "hello\nworld\n".trim().split("\n");
86+
let _ = "hello\nworld\n".trim().split("\r\n");
87+
8888
// Splitting a `str` variable at "\n" or "\r\n" without trimming should not warn, since it is not
8989
// equivalent
9090
let _ = s1.split('\n');
9191
#[allow(clippy::single_char_pattern)]
9292
let _ = s1.split("\n");
9393
let _ = s1.split("\r\n");
9494

95-
// Splitting a `str` literal at "\n" or "\r\n" without trimming should not warn.
96-
let _ = "hello\nworld\n".split('\n');
97-
#[allow(clippy::single_char_pattern)]
98-
let _ = "hello\nworld\n".split("\n");
99-
let _ = "hello\nworld\n".split("\r\n");
100-
10195
// Splitting a `String` variable at "\n" or "\r\n" without trimming should not warn.
10296
let _ = s2.split('\n');
10397
#[allow(clippy::single_char_pattern)]
@@ -117,13 +111,6 @@ fn main() {
117111
let _ = s1.trim().split("\n\r");
118112
let _ = s1.trim().split("\r \n");
119113

120-
// Splitting a `str` literal at other separators should not warn
121-
let _ = "hello\nworld\n".trim().split('\r');
122-
#[allow(clippy::single_char_pattern)]
123-
let _ = "hello\nworld\n".trim().split("\r");
124-
let _ = "hello\nworld\n".trim().split("\n\r");
125-
let _ = "hello\nworld\n".trim().split("\r \n");
126-
127114
// Splitting a `String` variable at other separators should not warn
128115
let _ = s2.trim().split('\r');
129116
#[allow(clippy::single_char_pattern)]

‎tests/ui/str_split.rs

+9-22
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ macro_rules! trim_split {
4444
}
4545

4646
macro_rules! make_str {
47-
() => {
48-
"hello\nworld\n"
47+
( $x: expr ) => {
48+
format!("x={}", $x)
4949
};
5050
}
5151

@@ -61,12 +61,6 @@ fn main() {
6161
let _ = s1.trim().split("\n");
6262
let _ = s1.trim().split("\r\n");
6363

64-
// Splitting a `str` literal at "\n" or "\r\n" after trimming should warn
65-
let _ = "hello\nworld\n".trim().split('\n');
66-
#[allow(clippy::single_char_pattern)]
67-
let _ = "hello\nworld\n".trim().split("\n");
68-
let _ = "hello\nworld\n".trim().split("\r\n");
69-
7064
// Splitting a `String` variable at "\n" or "\r\n" after trimming should warn
7165
let _ = s2.trim().split('\n');
7266
#[allow(clippy::single_char_pattern)]
@@ -81,23 +75,23 @@ fn main() {
8175
let _ = s3.trim().split("\r\n");
8276

8377
// If the `&str` is generated by a macro then the macro should not be expanded in the suggested fix.
84-
let _ = make_str!().trim().split('\n');
78+
let _ = make_str!(s1).trim().split('\n');
8579

8680
// CASES THAT SHOULD NOT EMIT A LINT
8781

82+
// Splitting a `str` constant at "\n" or "\r\n" after trimming should not warn
83+
let _ = "hello\nworld\n".trim().split('\n');
84+
#[allow(clippy::single_char_pattern)]
85+
let _ = "hello\nworld\n".trim().split("\n");
86+
let _ = "hello\nworld\n".trim().split("\r\n");
87+
8888
// Splitting a `str` variable at "\n" or "\r\n" without trimming should not warn, since it is not
8989
// equivalent
9090
let _ = s1.split('\n');
9191
#[allow(clippy::single_char_pattern)]
9292
let _ = s1.split("\n");
9393
let _ = s1.split("\r\n");
9494

95-
// Splitting a `str` literal at "\n" or "\r\n" without trimming should not warn.
96-
let _ = "hello\nworld\n".split('\n');
97-
#[allow(clippy::single_char_pattern)]
98-
let _ = "hello\nworld\n".split("\n");
99-
let _ = "hello\nworld\n".split("\r\n");
100-
10195
// Splitting a `String` variable at "\n" or "\r\n" without trimming should not warn.
10296
let _ = s2.split('\n');
10397
#[allow(clippy::single_char_pattern)]
@@ -117,13 +111,6 @@ fn main() {
117111
let _ = s1.trim().split("\n\r");
118112
let _ = s1.trim().split("\r \n");
119113

120-
// Splitting a `str` literal at other separators should not warn
121-
let _ = "hello\nworld\n".trim().split('\r');
122-
#[allow(clippy::single_char_pattern)]
123-
let _ = "hello\nworld\n".trim().split("\r");
124-
let _ = "hello\nworld\n".trim().split("\n\r");
125-
let _ = "hello\nworld\n".trim().split("\r \n");
126-
127114
// Splitting a `String` variable at other separators should not warn
128115
let _ = s2.trim().split('\r');
129116
#[allow(clippy::single_char_pattern)]

‎tests/ui/str_split.stderr

+19-37
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: using `str.trim().split()` with hard-coded newlines is not portable
1+
error: using `str.trim().split()` with hard-coded newlines
22
--> $DIR/str_split.rs:59:13
33
|
44
LL | let _ = s1.trim().split('\n');
@@ -7,77 +7,59 @@ LL | let _ = s1.trim().split('\n');
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)]`
99

10-
error: using `str.trim().split()` with hard-coded newlines is not portable
10+
error: using `str.trim().split()` with hard-coded newlines
1111
--> $DIR/str_split.rs:61:13
1212
|
1313
LL | let _ = s1.trim().split("\n");
1414
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
1515

16-
error: using `str.trim().split()` with hard-coded newlines is not portable
16+
error: using `str.trim().split()` with hard-coded newlines
1717
--> $DIR/str_split.rs:62:13
1818
|
1919
LL | let _ = s1.trim().split("\r\n");
2020
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
2121

22-
error: using `str.trim().split()` with hard-coded newlines is not portable
22+
error: using `str.trim().split()` with hard-coded newlines
2323
--> $DIR/str_split.rs:65:13
2424
|
25-
LL | let _ = "hello\nworld\n".trim().split('\n');
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
27-
28-
error: using `str.trim().split()` with hard-coded newlines is not portable
29-
--> $DIR/str_split.rs:67:13
30-
|
31-
LL | let _ = "hello\nworld\n".trim().split("\n");
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
33-
34-
error: using `str.trim().split()` with hard-coded newlines is not portable
35-
--> $DIR/str_split.rs:68:13
36-
|
37-
LL | let _ = "hello\nworld\n".trim().split("\r\n");
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
39-
40-
error: using `str.trim().split()` with hard-coded newlines is not portable
41-
--> $DIR/str_split.rs:71:13
42-
|
4325
LL | let _ = s2.trim().split('\n');
4426
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
4527

46-
error: using `str.trim().split()` with hard-coded newlines is not portable
47-
--> $DIR/str_split.rs:73:13
28+
error: using `str.trim().split()` with hard-coded newlines
29+
--> $DIR/str_split.rs:67:13
4830
|
4931
LL | let _ = s2.trim().split("\n");
5032
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
5133

52-
error: using `str.trim().split()` with hard-coded newlines is not portable
53-
--> $DIR/str_split.rs:74:13
34+
error: using `str.trim().split()` with hard-coded newlines
35+
--> $DIR/str_split.rs:68:13
5436
|
5537
LL | let _ = s2.trim().split("\r\n");
5638
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
5739

58-
error: using `str.trim().split()` with hard-coded newlines is not portable
59-
--> $DIR/str_split.rs:78:13
40+
error: using `str.trim().split()` with hard-coded newlines
41+
--> $DIR/str_split.rs:72:13
6042
|
6143
LL | let _ = s3.trim().split('\n');
6244
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
6345

64-
error: using `str.trim().split()` with hard-coded newlines is not portable
65-
--> $DIR/str_split.rs:80:13
46+
error: using `str.trim().split()` with hard-coded newlines
47+
--> $DIR/str_split.rs:74:13
6648
|
6749
LL | let _ = s3.trim().split("\n");
6850
| ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
6951

70-
error: using `str.trim().split()` with hard-coded newlines is not portable
71-
--> $DIR/str_split.rs:81:13
52+
error: using `str.trim().split()` with hard-coded newlines
53+
--> $DIR/str_split.rs:75:13
7254
|
7355
LL | let _ = s3.trim().split("\r\n");
7456
| ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
7557

76-
error: using `str.trim().split()` with hard-coded newlines is not portable
77-
--> $DIR/str_split.rs:84:13
58+
error: using `str.trim().split()` with hard-coded newlines
59+
--> $DIR/str_split.rs:78:13
7860
|
79-
LL | let _ = make_str!().trim().split('\n');
80-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `make_str!().lines()`
61+
LL | let _ = make_str!(s1).trim().split('\n');
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `make_str!(s1).lines()`
8163

82-
error: aborting due to 13 previous errors
64+
error: aborting due to 10 previous errors
8365

0 commit comments

Comments
 (0)