restructure lint code, update description, more cases

Author: y21

clippy_lints/src/methods/mod.rs

@@ -3420,11 +3420,12 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// ### What it does
     /// Looks for calls to [`Stdin::read_line`] to read a line from the standard input
-    /// into a string, then later attempting to parse this string into a type without first trimming it, which will
-    /// always fail because the string has a trailing newline in it.
+    /// into a string, then later attempting to use that string for an operation that will never
+    /// work for strings with a trailing newline character in it (e.g. parsing into a `i32`).
     ///
     /// ### Why is this bad?
-    /// The `.parse()` call will always fail.
+    /// The operation will always fail at runtime no matter what the user enters, thus
+    /// making it a useless operation.
     ///
     /// ### Example
     /// ```rust,ignore

clippy_lints/src/methods/read_line_without_trim.rs

@@ -15,6 +15,16 @@ use rustc_span::sym;
 
 use super::READ_LINE_WITHOUT_TRIM;
 
+fn expr_is_string_literal_without_trailing_newline(expr: &Expr<'_>) -> bool {
+    if let ExprKind::Lit(lit) = expr.kind
+        && let LitKind::Str(sym, _) = lit.node
+    {
+        !sym.as_str().ends_with('\n')
+    } else {
+        false
+    }
+}
+
 /// Will a `.parse::<ty>()` call fail if the input has a trailing newline?
 fn parse_fails_on_trailing_newline(ty: Ty<'_>) -> bool {
     // only allow a very limited set of types for now, for which we 100% know parsing will fail
@@ -28,69 +38,75 @@ pub fn check(cx: &LateContext<'_>, call: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<
         && let Res::Local(local_id) = path.res
     {
         // We've checked that `call` is a call to `Stdin::read_line()` with the right receiver,
-        // now let's check if the first use of the string passed to `::read_line()` is
-        // parsed into a type that will always fail if it has a trailing newline.
+        // now let's check if the first use of the string passed to `::read_line()`
+        // is used for operations that will always fail (e.g. parsing "6\n" into a number)
         for_each_local_use_after_expr(cx, local_id, call.hir_id, |expr| {
             if let Some(parent) = get_parent_expr(cx, expr) {
-                if let ExprKind::MethodCall(segment, .., span) = parent.kind
-                    && segment.ident.name == sym!(parse)
-                    && let parse_result_ty = cx.typeck_results().expr_ty(parent)
-                    && is_type_diagnostic_item(cx, parse_result_ty, sym::Result)
-                    && let ty::Adt(_, substs) = parse_result_ty.kind()
-                    && let Some(ok_ty) = substs[0].as_type()
-                    && parse_fails_on_trailing_newline(ok_ty)
-                {
-                    let local_snippet: std::borrow::Cow<'_, str> = snippet(cx, expr.span, "<expr>");
-                    span_lint_and_then(
-                        cx,
-                        READ_LINE_WITHOUT_TRIM,
-                        span,
-                        "calling `.parse()` without trimming the trailing newline character",
-                        |diag| {
-                            diag.span_note(
-                                call.span,
-                                "call to `.read_line()` here, \
-                                which leaves a trailing newline character in the buffer, \
-                                which in turn will cause `.parse()` to fail",
-                            );
-
-                            diag.span_suggestion(
-                                expr.span,
-                                "try",
-                                format!("{local_snippet}.trim_end()"),
-                                Applicability::MachineApplicable,
-                            );
-                        },
-                    );
-                } else if let ExprKind::Binary(binop, _, right) = parent.kind
+                let data = if let ExprKind::MethodCall(segment, recv, args, span) = parent.kind {
+                    if segment.ident.name == sym!(parse)
+                        && let parse_result_ty = cx.typeck_results().expr_ty(parent)
+                        && is_type_diagnostic_item(cx, parse_result_ty, sym::Result)
+                        && let ty::Adt(_, substs) = parse_result_ty.kind()
+                        && let Some(ok_ty) = substs[0].as_type()
+                        && parse_fails_on_trailing_newline(ok_ty)
+                    {
+                        // Called `s.parse::<T>()` where `T` is a type we know for certain will fail
+                        // if the input has a trailing newline
+                        Some((
+                            span,
+                            "calling `.parse()` on a string without trimming the trailing newline character",
+                            "checking",
+                        ))
+                    } else if segment.ident.name == sym!(ends_with)
+                        && recv.span == expr.span
+                        && let [arg] = args
+                        && expr_is_string_literal_without_trailing_newline(arg)
+                    {
+                        // Called `s.ends_with(<some string literal>)` where the argument is a string literal that does
+                        // not end with a newline, thus always evaluating to false
+                        Some((
+                            parent.span,
+                            "checking the end of a string without trimming the trailing newline character",
+                            "parsing",
+                        ))
+                    } else {
+                        None
+                    }
+                } else if let ExprKind::Binary(binop, left, right) = parent.kind
                     && let BinOpKind::Eq = binop.node
-                    && let ExprKind::Lit(lit) = right.kind
-                    && let LitKind::Str(sym, _) = lit.node
-                    && !sym.as_str().ends_with('\n')
+                    && (expr_is_string_literal_without_trailing_newline(left)
+                        || expr_is_string_literal_without_trailing_newline(right))
                 {
-                    span_lint_and_then(
-                        cx,
-                        READ_LINE_WITHOUT_TRIM,
+                    // `s == <some string literal>` where the string literal does not end with a newline
+                    Some((
                         parent.span,
                         "comparing a string literal without trimming the trailing newline character",
-                        |diag| {
-                            let local_snippet = snippet(cx, expr.span, "<expr>");
+                        "comparison",
+                    ))
+                } else {
+                    None
+                };
+
+                if let Some((primary_span, lint_message, operation)) = data {
+                    span_lint_and_then(cx, READ_LINE_WITHOUT_TRIM, primary_span, lint_message, |diag| {
+                        let local_snippet = snippet(cx, expr.span, "<expr>");
 
-                            diag.span_note(
-                                call.span,
+                        diag.span_note(
+                            call.span,
+                            format!(
                                 "call to `.read_line()` here, \
                                 which leaves a trailing newline character in the buffer, \
-                                which in turn will cause the comparison to always fail",
-                            );
+                                which in turn will cause the {operation} to always fail"
+                            ),
+                        );
 
-                            diag.span_suggestion(
-                                expr.span,
-                                "try",
-                                format!("{local_snippet}.trim_end()"),
-                                Applicability::MachineApplicable,
-                            );
-                        },
-                    );
+                        diag.span_suggestion(
+                            expr.span,
+                            "try",
+                            format!("{local_snippet}.trim_end()"),
+                            Applicability::MachineApplicable,
+                        );
+                    });
                 }
             }
 

tests/ui/read_line_without_trim.fixed

@@ -31,4 +31,17 @@ fn main() {
     std::io::stdin().read_line(&mut input).unwrap();
     // this is actually ok, so don't lint here
     let _x = input.parse::<String>().unwrap();
+
+    // comparing with string literals
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    if input.trim_end() == "foo" {
+        println!("This will never ever execute!");
+    }
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    if input.trim_end().ends_with("foo") {
+        println!("Neither will this");
+    }
 }

tests/ui/read_line_without_trim.rs

@@ -31,4 +31,17 @@ fn main() {
     std::io::stdin().read_line(&mut input).unwrap();
     // this is actually ok, so don't lint here
     let _x = input.parse::<String>().unwrap();
+
+    // comparing with string literals
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    if input == "foo" {
+        println!("This will never ever execute!");
+    }
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    if input.ends_with("foo") {
+        println!("Neither will this");
+    }
 }

tests/ui/read_line_without_trim.stderr

@@ -1,74 +1,102 @@
-error: calling `.parse()` without trimming the trailing newline character
+error: calling `.parse()` on a string without trimming the trailing newline character
   --> tests/ui/read_line_without_trim.rs:12:25
    |
 LL |     let _x: i32 = input.parse().unwrap();
    |                   ----- ^^^^^^^
    |                   |
    |                   help: try: `input.trim_end()`
    |
-note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the checking to always fail
   --> tests/ui/read_line_without_trim.rs:11:5
    |
 LL |     std::io::stdin().read_line(&mut input).unwrap();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: `-D clippy::read-line-without-trim` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::read_line_without_trim)]`
 
-error: calling `.parse()` without trimming the trailing newline character
+error: calling `.parse()` on a string without trimming the trailing newline character
   --> tests/ui/read_line_without_trim.rs:16:20
    |
 LL |     let _x = input.parse::<i32>().unwrap();
    |              ----- ^^^^^^^^^^^^^^
    |              |
    |              help: try: `input.trim_end()`
    |
-note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the checking to always fail
   --> tests/ui/read_line_without_trim.rs:15:5
    |
 LL |     std::io::stdin().read_line(&mut input).unwrap();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: calling `.parse()` without trimming the trailing newline character
+error: calling `.parse()` on a string without trimming the trailing newline character
   --> tests/ui/read_line_without_trim.rs:20:20
    |
 LL |     let _x = input.parse::<u32>().unwrap();
    |              ----- ^^^^^^^^^^^^^^
    |              |
    |              help: try: `input.trim_end()`
    |
-note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the checking to always fail
   --> tests/ui/read_line_without_trim.rs:19:5
    |
 LL |     std::io::stdin().read_line(&mut input).unwrap();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: calling `.parse()` without trimming the trailing newline character
+error: calling `.parse()` on a string without trimming the trailing newline character
   --> tests/ui/read_line_without_trim.rs:24:20
    |
 LL |     let _x = input.parse::<f32>().unwrap();
    |              ----- ^^^^^^^^^^^^^^
    |              |
    |              help: try: `input.trim_end()`
    |
-note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the checking to always fail
   --> tests/ui/read_line_without_trim.rs:23:5
    |
 LL |     std::io::stdin().read_line(&mut input).unwrap();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: calling `.parse()` without trimming the trailing newline character
+error: calling `.parse()` on a string without trimming the trailing newline character
   --> tests/ui/read_line_without_trim.rs:28:20
    |
 LL |     let _x = input.parse::<bool>().unwrap();
    |              ----- ^^^^^^^^^^^^^^^
    |              |
    |              help: try: `input.trim_end()`
    |
-note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the checking to always fail
   --> tests/ui/read_line_without_trim.rs:27:5
    |
 LL |     std::io::stdin().read_line(&mut input).unwrap();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 5 previous errors
+error: comparing a string literal without trimming the trailing newline character
+  --> tests/ui/read_line_without_trim.rs:38:8
+   |
+LL |     if input == "foo" {
+   |        -----^^^^^^^^^
+   |        |
+   |        help: try: `input.trim_end()`
+   |
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the comparison to always fail
+  --> tests/ui/read_line_without_trim.rs:37:5
+   |
+LL |     std::io::stdin().read_line(&mut input).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: checking the end of a string without trimming the trailing newline character
+  --> tests/ui/read_line_without_trim.rs:44:8
+   |
+LL |     if input.ends_with("foo") {
+   |        -----^^^^^^^^^^^^^^^^^
+   |        |
+   |        help: try: `input.trim_end()`
+   |
+note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause the parsing to always fail
+  --> tests/ui/read_line_without_trim.rs:43:5
+   |
+LL |     std::io::stdin().read_line(&mut input).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 7 previous errors