Skip to content

Commit c7a705a

Browse files
borsbors
committed
Auto merge of rust-lang#8575 - FoseFx:trim_split_whitespace2, r=flip1995
add `trim_split_whitespace` Closes rust-lang#8521 changelog: [`trim_split_whitespace`]
2 parents 751fd0d + fea177f commit c7a705a

9 files changed

+295
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3736,6 +3736,7 @@ Released 2018-09-13
37363736
[`transmute_undefined_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_undefined_repr
37373737
[`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts
37383738
[`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
3739+
[`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
37393740
[`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
37403741
[`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
37413742
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err

‎clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
280280
LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
281281
LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
282282
LintId::of(strings::STRING_FROM_UTF8_AS_BYTES),
283+
LintId::of(strings::TRIM_SPLIT_WHITESPACE),
283284
LintId::of(strlen_on_c_strings::STRLEN_ON_C_STRINGS),
284285
LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
285286
LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),

‎clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ store.register_lints(&[
482482
strings::STRING_SLICE,
483483
strings::STRING_TO_STRING,
484484
strings::STR_TO_STRING,
485+
strings::TRIM_SPLIT_WHITESPACE,
485486
strlen_on_c_strings::STRLEN_ON_C_STRINGS,
486487
suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS,
487488
suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,

‎clippy_lints/src/lib.register_style.rs

+1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
105105
LintId::of(returns::NEEDLESS_RETURN),
106106
LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
107107
LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
108+
LintId::of(strings::TRIM_SPLIT_WHITESPACE),
108109
LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
109110
LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
110111
LintId::of(unit_types::LET_UNIT_VALUE),

‎clippy_lints/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
889889
store.register_late_pass(|| Box::new(bytes_count_to_len::BytesCountToLen));
890890
let max_include_file_size = conf.max_include_file_size;
891891
store.register_late_pass(move || Box::new(large_include_file::LargeIncludeFile::new(max_include_file_size)));
892+
store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace));
892893
// add lints here, do not remove this comment, it's used in `new_lint`
893894
}
894895

‎clippy_lints/src/strings.rs

+56
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use clippy_utils::{get_parent_expr, is_lint_allowed, match_function_call, method
55
use clippy_utils::{peel_blocks, SpanlessEq};
66
use if_chain::if_chain;
77
use rustc_errors::Applicability;
8+
use rustc_hir::def_id::DefId;
89
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath};
910
use rustc_lint::{LateContext, LateLintPass, LintContext};
1011
use rustc_middle::lint::in_external_macro;
@@ -451,3 +452,58 @@ impl<'tcx> LateLintPass<'tcx> for StringToString {
451452
}
452453
}
453454
}
455+
456+
declare_clippy_lint! {
457+
/// ### What it does
458+
/// Warns about calling `str::trim` (or variants) before `str::split_whitespace`.
459+
///
460+
/// ### Why is this bad?
461+
/// `split_whitespace` already ignores leading and trailing whitespace.
462+
///
463+
/// ### Example
464+
/// ```rust
465+
/// " A B C ".trim().split_whitespace();
466+
/// ```
467+
/// Use instead:
468+
/// ```rust
469+
/// " A B C ".split_whitespace();
470+
/// ```
471+
#[clippy::version = "1.62.0"]
472+
pub TRIM_SPLIT_WHITESPACE,
473+
style,
474+
"using `str::trim()` or alike before `str::split_whitespace`"
475+
}
476+
declare_lint_pass!(TrimSplitWhitespace => [TRIM_SPLIT_WHITESPACE]);
477+
478+
impl<'tcx> LateLintPass<'tcx> for TrimSplitWhitespace {
479+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
480+
let tyckres = cx.typeck_results();
481+
if_chain! {
482+
if let ExprKind::MethodCall(path, [split_recv], split_ws_span) = expr.kind;
483+
if path.ident.name == sym!(split_whitespace);
484+
if let Some(split_ws_def_id) = tyckres.type_dependent_def_id(expr.hir_id);
485+
if cx.tcx.is_diagnostic_item(sym::str_split_whitespace, split_ws_def_id);
486+
if let ExprKind::MethodCall(path, [_trim_recv], trim_span) = split_recv.kind;
487+
if let trim_fn_name @ ("trim" | "trim_start" | "trim_end") = path.ident.name.as_str();
488+
if let Some(trim_def_id) = tyckres.type_dependent_def_id(split_recv.hir_id);
489+
if is_one_of_trim_diagnostic_items(cx, trim_def_id);
490+
then {
491+
span_lint_and_sugg(
492+
cx,
493+
TRIM_SPLIT_WHITESPACE,
494+
trim_span.with_hi(split_ws_span.lo()),
495+
&format!("found call to `str::{}` before `str::split_whitespace`", trim_fn_name),
496+
&format!("remove `{}()`", trim_fn_name),
497+
String::new(),
498+
Applicability::MachineApplicable,
499+
);
500+
}
501+
}
502+
}
503+
}
504+
505+
fn is_one_of_trim_diagnostic_items(cx: &LateContext<'_>, trim_def_id: DefId) -> bool {
506+
cx.tcx.is_diagnostic_item(sym::str_trim, trim_def_id)
507+
|| cx.tcx.is_diagnostic_item(sym::str_trim_start, trim_def_id)
508+
|| cx.tcx.is_diagnostic_item(sym::str_trim_end, trim_def_id)
509+
}

‎tests/ui/trim_split_whitespace.fixed

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// run-rustfix
2+
#![warn(clippy::trim_split_whitespace)]
3+
#![allow(clippy::let_unit_value)]
4+
5+
struct Custom;
6+
impl Custom {
7+
fn trim(self) -> Self {
8+
self
9+
}
10+
fn split_whitespace(self) {}
11+
}
12+
13+
struct DerefStr(&'static str);
14+
impl std::ops::Deref for DerefStr {
15+
type Target = str;
16+
fn deref(&self) -> &Self::Target {
17+
self.0
18+
}
19+
}
20+
21+
struct DerefStrAndCustom(&'static str);
22+
impl std::ops::Deref for DerefStrAndCustom {
23+
type Target = str;
24+
fn deref(&self) -> &Self::Target {
25+
self.0
26+
}
27+
}
28+
impl DerefStrAndCustom {
29+
fn trim(self) -> Self {
30+
self
31+
}
32+
fn split_whitespace(self) {}
33+
}
34+
35+
struct DerefStrAndCustomSplit(&'static str);
36+
impl std::ops::Deref for DerefStrAndCustomSplit {
37+
type Target = str;
38+
fn deref(&self) -> &Self::Target {
39+
self.0
40+
}
41+
}
42+
impl DerefStrAndCustomSplit {
43+
#[allow(dead_code)]
44+
fn split_whitespace(self) {}
45+
}
46+
47+
struct DerefStrAndCustomTrim(&'static str);
48+
impl std::ops::Deref for DerefStrAndCustomTrim {
49+
type Target = str;
50+
fn deref(&self) -> &Self::Target {
51+
self.0
52+
}
53+
}
54+
impl DerefStrAndCustomTrim {
55+
fn trim(self) -> Self {
56+
self
57+
}
58+
}
59+
60+
fn main() {
61+
// &str
62+
let _ = " A B C ".split_whitespace(); // should trigger lint
63+
let _ = " A B C ".split_whitespace(); // should trigger lint
64+
let _ = " A B C ".split_whitespace(); // should trigger lint
65+
66+
// String
67+
let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
68+
let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
69+
let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
70+
71+
// Custom
72+
let _ = Custom.trim().split_whitespace(); // should not trigger lint
73+
74+
// Deref<Target=str>
75+
let s = DerefStr(" A B C ");
76+
let _ = s.split_whitespace(); // should trigger lint
77+
78+
// Deref<Target=str> + custom impl
79+
let s = DerefStrAndCustom(" A B C ");
80+
let _ = s.trim().split_whitespace(); // should not trigger lint
81+
82+
// Deref<Target=str> + only custom split_ws() impl
83+
let s = DerefStrAndCustomSplit(" A B C ");
84+
let _ = s.split_whitespace(); // should trigger lint
85+
// Expl: trim() is called on str (deref) and returns &str.
86+
// Thus split_ws() is called on str as well and the custom impl on S is unused
87+
88+
// Deref<Target=str> + only custom trim() impl
89+
let s = DerefStrAndCustomTrim(" A B C ");
90+
let _ = s.trim().split_whitespace(); // should not trigger lint
91+
}

‎tests/ui/trim_split_whitespace.rs

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// run-rustfix
2+
#![warn(clippy::trim_split_whitespace)]
3+
#![allow(clippy::let_unit_value)]
4+
5+
struct Custom;
6+
impl Custom {
7+
fn trim(self) -> Self {
8+
self
9+
}
10+
fn split_whitespace(self) {}
11+
}
12+
13+
struct DerefStr(&'static str);
14+
impl std::ops::Deref for DerefStr {
15+
type Target = str;
16+
fn deref(&self) -> &Self::Target {
17+
self.0
18+
}
19+
}
20+
21+
struct DerefStrAndCustom(&'static str);
22+
impl std::ops::Deref for DerefStrAndCustom {
23+
type Target = str;
24+
fn deref(&self) -> &Self::Target {
25+
self.0
26+
}
27+
}
28+
impl DerefStrAndCustom {
29+
fn trim(self) -> Self {
30+
self
31+
}
32+
fn split_whitespace(self) {}
33+
}
34+
35+
struct DerefStrAndCustomSplit(&'static str);
36+
impl std::ops::Deref for DerefStrAndCustomSplit {
37+
type Target = str;
38+
fn deref(&self) -> &Self::Target {
39+
self.0
40+
}
41+
}
42+
impl DerefStrAndCustomSplit {
43+
#[allow(dead_code)]
44+
fn split_whitespace(self) {}
45+
}
46+
47+
struct DerefStrAndCustomTrim(&'static str);
48+
impl std::ops::Deref for DerefStrAndCustomTrim {
49+
type Target = str;
50+
fn deref(&self) -> &Self::Target {
51+
self.0
52+
}
53+
}
54+
impl DerefStrAndCustomTrim {
55+
fn trim(self) -> Self {
56+
self
57+
}
58+
}
59+
60+
fn main() {
61+
// &str
62+
let _ = " A B C ".trim().split_whitespace(); // should trigger lint
63+
let _ = " A B C ".trim_start().split_whitespace(); // should trigger lint
64+
let _ = " A B C ".trim_end().split_whitespace(); // should trigger lint
65+
66+
// String
67+
let _ = (" A B C ").to_string().trim().split_whitespace(); // should trigger lint
68+
let _ = (" A B C ").to_string().trim_start().split_whitespace(); // should trigger lint
69+
let _ = (" A B C ").to_string().trim_end().split_whitespace(); // should trigger lint
70+
71+
// Custom
72+
let _ = Custom.trim().split_whitespace(); // should not trigger lint
73+
74+
// Deref<Target=str>
75+
let s = DerefStr(" A B C ");
76+
let _ = s.trim().split_whitespace(); // should trigger lint
77+
78+
// Deref<Target=str> + custom impl
79+
let s = DerefStrAndCustom(" A B C ");
80+
let _ = s.trim().split_whitespace(); // should not trigger lint
81+
82+
// Deref<Target=str> + only custom split_ws() impl
83+
let s = DerefStrAndCustomSplit(" A B C ");
84+
let _ = s.trim().split_whitespace(); // should trigger lint
85+
// Expl: trim() is called on str (deref) and returns &str.
86+
// Thus split_ws() is called on str as well and the custom impl on S is unused
87+
88+
// Deref<Target=str> + only custom trim() impl
89+
let s = DerefStrAndCustomTrim(" A B C ");
90+
let _ = s.trim().split_whitespace(); // should not trigger lint
91+
}

‎tests/ui/trim_split_whitespace.stderr

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
error: found call to `str::trim` before `str::split_whitespace`
2+
--> $DIR/trim_split_whitespace.rs:62:23
3+
|
4+
LL | let _ = " A B C ".trim().split_whitespace(); // should trigger lint
5+
| ^^^^^^^ help: remove `trim()`
6+
|
7+
= note: `-D clippy::trim-split-whitespace` implied by `-D warnings`
8+
9+
error: found call to `str::trim_start` before `str::split_whitespace`
10+
--> $DIR/trim_split_whitespace.rs:63:23
11+
|
12+
LL | let _ = " A B C ".trim_start().split_whitespace(); // should trigger lint
13+
| ^^^^^^^^^^^^^ help: remove `trim_start()`
14+
15+
error: found call to `str::trim_end` before `str::split_whitespace`
16+
--> $DIR/trim_split_whitespace.rs:64:23
17+
|
18+
LL | let _ = " A B C ".trim_end().split_whitespace(); // should trigger lint
19+
| ^^^^^^^^^^^ help: remove `trim_end()`
20+
21+
error: found call to `str::trim` before `str::split_whitespace`
22+
--> $DIR/trim_split_whitespace.rs:67:37
23+
|
24+
LL | let _ = (" A B C ").to_string().trim().split_whitespace(); // should trigger lint
25+
| ^^^^^^^ help: remove `trim()`
26+
27+
error: found call to `str::trim_start` before `str::split_whitespace`
28+
--> $DIR/trim_split_whitespace.rs:68:37
29+
|
30+
LL | let _ = (" A B C ").to_string().trim_start().split_whitespace(); // should trigger lint
31+
| ^^^^^^^^^^^^^ help: remove `trim_start()`
32+
33+
error: found call to `str::trim_end` before `str::split_whitespace`
34+
--> $DIR/trim_split_whitespace.rs:69:37
35+
|
36+
LL | let _ = (" A B C ").to_string().trim_end().split_whitespace(); // should trigger lint
37+
| ^^^^^^^^^^^ help: remove `trim_end()`
38+
39+
error: found call to `str::trim` before `str::split_whitespace`
40+
--> $DIR/trim_split_whitespace.rs:76:15
41+
|
42+
LL | let _ = s.trim().split_whitespace(); // should trigger lint
43+
| ^^^^^^^ help: remove `trim()`
44+
45+
error: found call to `str::trim` before `str::split_whitespace`
46+
--> $DIR/trim_split_whitespace.rs:84:15
47+
|
48+
LL | let _ = s.trim().split_whitespace(); // should trigger lint
49+
| ^^^^^^^ help: remove `trim()`
50+
51+
error: aborting due to 8 previous errors
52+

0 commit comments

Comments
 (0)