8733: Move lint to methods type

Author: torfsen

CHANGELOG.md

@@ -5576,10 +5576,10 @@ Released 2018-09-13
 [`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
-[`splitting_strings_at_newlines`]: https://rust-lang.github.io/rust-clippy/master/index.html#splitting_strings_at_newlines
 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
 [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc
 [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core
+[`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
 [`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign

clippy_lints/src/declared_lints.rs

@@ -433,6 +433,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::STABLE_SORT_PRIMITIVE_INFO,
     crate::methods::STRING_EXTEND_CHARS_INFO,
     crate::methods::STRING_LIT_CHARS_ANY_INFO,
+    crate::methods::STR_SPLIT_AT_NEWLINE_INFO,
     crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO,
     crate::methods::SUSPICIOUS_MAP_INFO,
     crate::methods::SUSPICIOUS_SPLITN_INFO,
@@ -626,7 +627,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO,
     crate::size_of_ref::SIZE_OF_REF_INFO,
     crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO,
-    crate::splitting_strings_at_newlines::SPLITTING_STRINGS_AT_NEWLINES_INFO,
     crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_INFO,
     crate::std_instead_of_core::STD_INSTEAD_OF_CORE_INFO,

clippy_lints/src/lib.rs

@@ -310,7 +310,6 @@ mod single_range_in_vec_init;
 mod size_of_in_element_count;
 mod size_of_ref;
 mod slow_vector_initialization;
-mod splitting_strings_at_newlines;
 mod std_instead_of_core;
 mod strings;
 mod strlen_on_c_strings;
@@ -1081,7 +1080,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences));
     store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions));
     store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion));
-    store.register_late_pass(|_| Box::new(splitting_strings_at_newlines::SplittingStringsAtNewlines));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/methods/mod.rs

@@ -93,6 +93,7 @@ mod single_char_pattern;
 mod single_char_push_string;
 mod skip_while_next;
 mod stable_sort_primitive;
+mod str_split;
 mod str_splitn;
 mod string_extend_chars;
 mod string_lit_chars_any;
@@ -3829,6 +3830,29 @@ declare_clippy_lint! {
     "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usages of `str.trim().split("\n")` and `str.trim().split("\r\n")`.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Hard-coding the line endings makes the code less portable. `str.lines` should be used instead.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// "some\ntext\nwith\nnewlines\n".trim().split('\n');
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// "some\ntext\nwith\nnewlines\n".lines();
+    /// ```
+    #[clippy::version = "1.76.0"]
+    pub STR_SPLIT_AT_NEWLINE,
+    suspicious,
+    "splitting a trimmed string at hard-coded newlines"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3983,6 +4007,7 @@ impl_lint_pass!(Methods => [
     RESULT_FILTER_MAP,
     ITER_FILTER_IS_SOME,
     ITER_FILTER_IS_OK,
+    STR_SPLIT_AT_NEWLINE,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4569,6 +4594,9 @@ impl Methods {
                 ("sort_unstable_by", [arg]) => {
                     unnecessary_sort_by::check(cx, expr, recv, arg, true);
                 },
+                ("split", [arg]) => {
+                    str_split::check(cx, expr, recv, arg);
+                },
                 ("splitn" | "rsplitn", [count_arg, pat_arg]) => {
                     if let Some(Constant::Int(count)) = constant(cx, cx.typeck_results(), count_arg) {
                         suspicious_splitn::check(cx, name, expr, recv, count);

clippy_lints/src/methods/str_split.rs

@@ -0,0 +1,42 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_context;
+use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+
+use super::STR_SPLIT_AT_NEWLINE;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, split_recv: &'_ Expr<'_>, split_arg: &'_ Expr<'_>) {
+    // We're looking for `A.trim().split(B)`, where the adjusted type of `A` is `&str` (e.g. a `str`
+    // literal or an expression returning `String`), and `B` is a `Pattern` that hard-codes a
+    // newline (either `"\n"` or `"\r\n"`). There are a lot of ways to specify a pattern, and
+    // this lint only checks the most basic ones: a `'\n'`, `"\n"`, and `"\r\n"`.
+
+    if let ExprKind::MethodCall(trim_method_name, trim_recv, trim_args, _) = split_recv.kind
+        && trim_method_name.ident.as_str() == "trim"
+        && trim_args.is_empty()
+        && cx.typeck_results().expr_ty_adjusted(trim_recv).peel_refs().is_str()
+        && let ExprKind::Lit(split_lit) = split_arg.kind
+    {
+        let char_n = matches!(split_lit.node, LitKind::Char('\n'));
+        let str_n_or_rn =
+            matches!(split_lit.node, LitKind::Str(sym, _) if (sym.as_str() == "\n" || sym.as_str() == "\r\n"));
+
+        if char_n || str_n_or_rn {
+            let mut app = Applicability::MachineApplicable;
+            span_lint_and_sugg(
+                cx,
+                STR_SPLIT_AT_NEWLINE,
+                expr.span,
+                "using `str.trim().split()` with hard-coded newlines is not portable",
+                "use `str.lines()` instead",
+                format!(
+                    "{}.lines()",
+                    snippet_with_context(cx, trim_recv.span, expr.span.ctxt(), "..", &mut app).0
+                ),
+                app,
+            );
+        }
+    }
+}

clippy_lints/src/splitting_strings_at_newlines.rs

@@ -1,4 +1,4 @@
-#![warn(clippy::splitting_strings_at_newlines)]
+#![warn(clippy::str_split_at_newline)]
 
 use core::str::Split;
 use std::ops::Deref;

tests/ui/splitting_strings_at_newlines.fixed renamed to tests/ui/str_split.fixed

@@ -1,4 +1,4 @@
-#![warn(clippy::splitting_strings_at_newlines)]
+#![warn(clippy::str_split_at_newline)]
 
 use core::str::Split;
 use std::ops::Deref;

tests/ui/splitting_strings_at_newlines.rs renamed to tests/ui/str_split.rs

@@ -1,80 +1,80 @@
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:59:13
+  --> $DIR/str_split.rs:59:13
    |
 LL |     let _ = s1.trim().split('\n');
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
    |
-   = note: `-D clippy::splitting-strings-at-newlines` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::splitting_strings_at_newlines)]`
+   = note: `-D clippy::str-split-at-newline` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::str_split_at_newline)]`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:61:13
+  --> $DIR/str_split.rs:61:13
    |
 LL |     let _ = s1.trim().split("\n");
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:62:13
+  --> $DIR/str_split.rs:62:13
    |
 LL |     let _ = s1.trim().split("\r\n");
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s1.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:65:13
+  --> $DIR/str_split.rs:65:13
    |
 LL |     let _ = "hello\nworld\n".trim().split('\n');
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:67:13
+  --> $DIR/str_split.rs:67:13
    |
 LL |     let _ = "hello\nworld\n".trim().split("\n");
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:68:13
+  --> $DIR/str_split.rs:68:13
    |
 LL |     let _ = "hello\nworld\n".trim().split("\r\n");
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `"hello\nworld\n".lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:71:13
+  --> $DIR/str_split.rs:71:13
    |
 LL |     let _ = s2.trim().split('\n');
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:73:13
+  --> $DIR/str_split.rs:73:13
    |
 LL |     let _ = s2.trim().split("\n");
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:74:13
+  --> $DIR/str_split.rs:74:13
    |
 LL |     let _ = s2.trim().split("\r\n");
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s2.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:78:13
+  --> $DIR/str_split.rs:78:13
    |
 LL |     let _ = s3.trim().split('\n');
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:80:13
+  --> $DIR/str_split.rs:80:13
    |
 LL |     let _ = s3.trim().split("\n");
    |             ^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:81:13
+  --> $DIR/str_split.rs:81:13
    |
 LL |     let _ = s3.trim().split("\r\n");
    |             ^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `s3.lines()`
 
 error: using `str.trim().split()` with hard-coded newlines is not portable
-  --> $DIR/splitting_strings_at_newlines.rs:84:13
+  --> $DIR/str_split.rs:84:13
    |
 LL |     let _ = make_str!().trim().split('\n');
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `str.lines()` instead: `make_str!().lines()`