add trim_split_whitespace

rust-lang · May 4, 2022 · fea177f · fea177f
1 parent 760f293
commit fea177f
Show file tree

Hide file tree

Showing 9 changed files with 295 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3736,6 +3736,7 @@ Released 2018-09-13
 [`transmute_undefined_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_undefined_repr
 [`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts
 [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
+[`trim_split_whitespace`]: https://rust-lang.github.io/rust-clippy/master/index.html#trim_split_whitespace
 [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err

diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
@@ -281,6 +281,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
     LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
     LintId::of(strings::STRING_FROM_UTF8_AS_BYTES),
+    LintId::of(strings::TRIM_SPLIT_WHITESPACE),
     LintId::of(strlen_on_c_strings::STRLEN_ON_C_STRINGS),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),

diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
@@ -482,6 +482,7 @@ store.register_lints(&[
     strings::STRING_SLICE,
     strings::STRING_TO_STRING,
     strings::STR_TO_STRING,
+    strings::TRIM_SPLIT_WHITESPACE,
     strlen_on_c_strings::STRLEN_ON_C_STRINGS,
     suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS,
     suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,

diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
@@ -105,6 +105,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(returns::NEEDLESS_RETURN),
     LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
     LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
+    LintId::of(strings::TRIM_SPLIT_WHITESPACE),
     LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
     LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
     LintId::of(unit_types::LET_UNIT_VALUE),

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -887,6 +887,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(bytes_count_to_len::BytesCountToLen));
     let max_include_file_size = conf.max_include_file_size;
     store.register_late_pass(move || Box::new(large_include_file::LargeIncludeFile::new(max_include_file_size)));
+    store.register_late_pass(|| Box::new(strings::TrimSplitWhitespace));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

diff --git a/clippy_lints/src/strings.rs b/clippy_lints/src/strings.rs
@@ -5,6 +5,7 @@ use clippy_utils::{get_parent_expr, is_lint_allowed, match_function_call, method
 use clippy_utils::{peel_blocks, SpanlessEq};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
+use rustc_hir::def_id::DefId;
 use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, QPath};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
@@ -451,3 +452,58 @@ impl<'tcx> LateLintPass<'tcx> for StringToString {
         }
     }
 }
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Warns about calling `str::trim` (or variants) before `str::split_whitespace`.
+    ///
+    /// ### Why is this bad?
+    /// `split_whitespace` already ignores leading and trailing whitespace.
+    ///
+    /// ### Example
+    /// ```rust
+    /// " A B C ".trim().split_whitespace();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// " A B C ".split_whitespace();
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub TRIM_SPLIT_WHITESPACE,
+    style,
+    "using `str::trim()` or alike before `str::split_whitespace`"
+}
+declare_lint_pass!(TrimSplitWhitespace => [TRIM_SPLIT_WHITESPACE]);
+
+impl<'tcx> LateLintPass<'tcx> for TrimSplitWhitespace {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
+        let tyckres = cx.typeck_results();
+        if_chain! {
+            if let ExprKind::MethodCall(path, [split_recv], split_ws_span) = expr.kind;
+            if path.ident.name == sym!(split_whitespace);
+            if let Some(split_ws_def_id) = tyckres.type_dependent_def_id(expr.hir_id);
+            if cx.tcx.is_diagnostic_item(sym::str_split_whitespace, split_ws_def_id);
+            if let ExprKind::MethodCall(path, [_trim_recv], trim_span) = split_recv.kind;
+            if let trim_fn_name @ ("trim" | "trim_start" | "trim_end") = path.ident.name.as_str();
+            if let Some(trim_def_id) = tyckres.type_dependent_def_id(split_recv.hir_id);
+            if is_one_of_trim_diagnostic_items(cx, trim_def_id);
+            then {
+                span_lint_and_sugg(
+                    cx,
+                    TRIM_SPLIT_WHITESPACE,
+                    trim_span.with_hi(split_ws_span.lo()),
+                    &format!("found call to `str::{}` before `str::split_whitespace`", trim_fn_name),
+                    &format!("remove `{}()`", trim_fn_name),
+                    String::new(),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+}
+
+fn is_one_of_trim_diagnostic_items(cx: &LateContext<'_>, trim_def_id: DefId) -> bool {
+    cx.tcx.is_diagnostic_item(sym::str_trim, trim_def_id)
+        || cx.tcx.is_diagnostic_item(sym::str_trim_start, trim_def_id)
+        || cx.tcx.is_diagnostic_item(sym::str_trim_end, trim_def_id)
+}
diff --git a/tests/ui/trim_split_whitespace.fixed b/tests/ui/trim_split_whitespace.fixed
@@ -0,0 +1,91 @@
+// run-rustfix
+#![warn(clippy::trim_split_whitespace)]
+#![allow(clippy::let_unit_value)]
+
+struct Custom;
+impl Custom {
+    fn trim(self) -> Self {
+        self
+    }
+    fn split_whitespace(self) {}
+}
+
+struct DerefStr(&'static str);
+impl std::ops::Deref for DerefStr {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+
+struct DerefStrAndCustom(&'static str);
+impl std::ops::Deref for DerefStrAndCustom {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustom {
+    fn trim(self) -> Self {
+        self
+    }
+    fn split_whitespace(self) {}
+}
+
+struct DerefStrAndCustomSplit(&'static str);
+impl std::ops::Deref for DerefStrAndCustomSplit {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustomSplit {
+    #[allow(dead_code)]
+    fn split_whitespace(self) {}
+}
+
+struct DerefStrAndCustomTrim(&'static str);
+impl std::ops::Deref for DerefStrAndCustomTrim {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustomTrim {
+    fn trim(self) -> Self {
+        self
+    }
+}
+
+fn main() {
+    // &str
+    let _ = " A B C ".split_whitespace(); // should trigger lint
+    let _ = " A B C ".split_whitespace(); // should trigger lint
+    let _ = " A B C ".split_whitespace(); // should trigger lint
+
+    // String
+    let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
+    let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
+    let _ = (" A B C ").to_string().split_whitespace(); // should trigger lint
+
+    // Custom
+    let _ = Custom.trim().split_whitespace(); // should not trigger lint
+
+    // Deref<Target=str>
+    let s = DerefStr(" A B C ");
+    let _ = s.split_whitespace(); // should trigger lint
+
+    // Deref<Target=str> + custom impl
+    let s = DerefStrAndCustom(" A B C ");
+    let _ = s.trim().split_whitespace(); // should not trigger lint
+
+    // Deref<Target=str> + only custom split_ws() impl
+    let s = DerefStrAndCustomSplit(" A B C ");
+    let _ = s.split_whitespace(); // should trigger lint
+    // Expl: trim() is called on str (deref) and returns &str.
+    //       Thus split_ws() is called on str as well and the custom impl on S is unused
+
+    // Deref<Target=str> + only custom trim() impl
+    let s = DerefStrAndCustomTrim(" A B C ");
+    let _ = s.trim().split_whitespace(); // should not trigger lint
+}
diff --git a/tests/ui/trim_split_whitespace.rs b/tests/ui/trim_split_whitespace.rs
@@ -0,0 +1,91 @@
+// run-rustfix
+#![warn(clippy::trim_split_whitespace)]
+#![allow(clippy::let_unit_value)]
+
+struct Custom;
+impl Custom {
+    fn trim(self) -> Self {
+        self
+    }
+    fn split_whitespace(self) {}
+}
+
+struct DerefStr(&'static str);
+impl std::ops::Deref for DerefStr {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+
+struct DerefStrAndCustom(&'static str);
+impl std::ops::Deref for DerefStrAndCustom {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustom {
+    fn trim(self) -> Self {
+        self
+    }
+    fn split_whitespace(self) {}
+}
+
+struct DerefStrAndCustomSplit(&'static str);
+impl std::ops::Deref for DerefStrAndCustomSplit {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustomSplit {
+    #[allow(dead_code)]
+    fn split_whitespace(self) {}
+}
+
+struct DerefStrAndCustomTrim(&'static str);
+impl std::ops::Deref for DerefStrAndCustomTrim {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.0
+    }
+}
+impl DerefStrAndCustomTrim {
+    fn trim(self) -> Self {
+        self
+    }
+}
+
+fn main() {
+    // &str
+    let _ = " A B C ".trim().split_whitespace(); // should trigger lint
+    let _ = " A B C ".trim_start().split_whitespace(); // should trigger lint
+    let _ = " A B C ".trim_end().split_whitespace(); // should trigger lint
+
+    // String
+    let _ = (" A B C ").to_string().trim().split_whitespace(); // should trigger lint
+    let _ = (" A B C ").to_string().trim_start().split_whitespace(); // should trigger lint
+    let _ = (" A B C ").to_string().trim_end().split_whitespace(); // should trigger lint
+
+    // Custom
+    let _ = Custom.trim().split_whitespace(); // should not trigger lint
+
+    // Deref<Target=str>
+    let s = DerefStr(" A B C ");
+    let _ = s.trim().split_whitespace(); // should trigger lint
+
+    // Deref<Target=str> + custom impl
+    let s = DerefStrAndCustom(" A B C ");
+    let _ = s.trim().split_whitespace(); // should not trigger lint
+
+    // Deref<Target=str> + only custom split_ws() impl
+    let s = DerefStrAndCustomSplit(" A B C ");
+    let _ = s.trim().split_whitespace(); // should trigger lint
+    // Expl: trim() is called on str (deref) and returns &str.
+    //       Thus split_ws() is called on str as well and the custom impl on S is unused
+
+    // Deref<Target=str> + only custom trim() impl
+    let s = DerefStrAndCustomTrim(" A B C ");
+    let _ = s.trim().split_whitespace(); // should not trigger lint
+}
diff --git a/tests/ui/trim_split_whitespace.stderr b/tests/ui/trim_split_whitespace.stderr
@@ -0,0 +1,52 @@
+error: found call to `str::trim` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:62:23
+   |
+LL |     let _ = " A B C ".trim().split_whitespace(); // should trigger lint
+   |                       ^^^^^^^ help: remove `trim()`
+   |
+   = note: `-D clippy::trim-split-whitespace` implied by `-D warnings`
+
+error: found call to `str::trim_start` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:63:23
+   |
+LL |     let _ = " A B C ".trim_start().split_whitespace(); // should trigger lint
+   |                       ^^^^^^^^^^^^^ help: remove `trim_start()`
+
+error: found call to `str::trim_end` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:64:23
+   |
+LL |     let _ = " A B C ".trim_end().split_whitespace(); // should trigger lint
+   |                       ^^^^^^^^^^^ help: remove `trim_end()`
+
+error: found call to `str::trim` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:67:37
+   |
+LL |     let _ = (" A B C ").to_string().trim().split_whitespace(); // should trigger lint
+   |                                     ^^^^^^^ help: remove `trim()`
+
+error: found call to `str::trim_start` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:68:37
+   |
+LL |     let _ = (" A B C ").to_string().trim_start().split_whitespace(); // should trigger lint
+   |                                     ^^^^^^^^^^^^^ help: remove `trim_start()`
+
+error: found call to `str::trim_end` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:69:37
+   |
+LL |     let _ = (" A B C ").to_string().trim_end().split_whitespace(); // should trigger lint
+   |                                     ^^^^^^^^^^^ help: remove `trim_end()`
+
+error: found call to `str::trim` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:76:15
+   |
+LL |     let _ = s.trim().split_whitespace(); // should trigger lint
+   |               ^^^^^^^ help: remove `trim()`
+
+error: found call to `str::trim` before `str::split_whitespace`
+  --> $DIR/trim_split_whitespace.rs:84:15
+   |
+LL |     let _ = s.trim().split_whitespace(); // should trigger lint
+   |               ^^^^^^^ help: remove `trim()`
+
+error: aborting due to 8 previous errors
+