new lint: read_line_without_trim

Author: y21

CHANGELOG.md

@@ -5101,6 +5101,7 @@ Released 2018-09-13
 [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
 [`rc_clone_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_clone_in_vec_init
 [`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex
+[`read_line_without_trim`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_line_without_trim
 [`read_zero_byte_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec
 [`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl
 [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation

clippy_lints/src/declared_lints.rs

@@ -388,6 +388,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::methods::OR_THEN_UNWRAP_INFO,
     crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
     crate::methods::RANGE_ZIP_WITH_LEN_INFO,
+    crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
     crate::methods::REPEAT_ONCE_INFO,
     crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
     crate::methods::SEARCH_IS_SOME_INFO,

clippy_lints/src/methods/mod.rs

@@ -71,6 +71,7 @@ mod or_fun_call;
 mod or_then_unwrap;
 mod path_buf_push_overwrite;
 mod range_zip_with_len;
+mod read_line_without_trim;
 mod repeat_once;
 mod search_is_some;
 mod seek_from_current;
@@ -3284,6 +3285,35 @@ declare_clippy_lint! {
     "calling `.drain(..).collect()` to move all elements into a new collection"
 }
 
+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.
+    ///
+    /// ### Why is this bad?
+    /// The `.parse()` call will always fail.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let mut input = String::new();
+    /// std::io::stdin().read_line(&mut input).expect("Failed to read a line");
+    /// let num: i32 = input.parse().expect("Not a number!");
+    /// assert_eq!(num, 42); // we never even get here!
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut input = String::new();
+    /// std::io::stdin().read_line(&mut input).expect("Failed to read a line");
+    /// let num: i32 = input.trim_end().parse().expect("Not a number!");
+    /// //                  ^^^^^^^^^^^ remove the trailing newline
+    /// assert_eq!(num, 42);
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub READ_LINE_WITHOUT_TRIM,
+    correctness,
+    "calling `Stdin::read_line`, then trying to parse it without first trimming"
+}
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -3403,6 +3433,7 @@ impl_lint_pass!(Methods => [
     REPEAT_ONCE,
     STABLE_SORT_PRIMITIVE,
     UNIT_HASH,
+    READ_LINE_WITHOUT_TRIM,
     UNNECESSARY_SORT_BY,
     VEC_RESIZE_TO_ZERO,
     VERBOSE_FILE_READS,
@@ -3810,6 +3841,9 @@ impl Methods {
                 ("read_to_string", [_]) => {
                     verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_STRING_MSG);
                 },
+                ("read_line", [arg]) => {
+                    read_line_without_trim::check(cx, expr, recv, arg);
+                }
                 ("repeat", [arg]) => {
                     repeat_once::check(cx, expr, recv, arg);
                 },

clippy_lints/src/methods/read_line_without_trim.rs

@@ -0,0 +1,77 @@
+use std::ops::ControlFlow;
+
+use clippy_utils::{
+    diagnostics::span_lint_and_then, get_parent_expr, match_def_path, source::snippet, ty::is_type_diagnostic_item,
+    visitors::for_each_local_use_after_expr,
+};
+use rustc_errors::Applicability;
+use rustc_hir::Expr;
+use rustc_hir::QPath;
+use rustc_hir::{def::Res, ExprKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::{Ty, TyKind};
+use rustc_span::sym;
+
+use super::READ_LINE_WITHOUT_TRIM;
+
+/// 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
+    matches!(
+        ty.kind(),
+        TyKind::Float(_) | TyKind::Bool | TyKind::Int(_) | TyKind::Uint(_)
+    )
+}
+
+pub fn check(cx: &LateContext<'_>, call: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
+    if let Some(recv_adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
+        && match_def_path(cx, recv_adt.did(), &["std", "io", "stdio", "Stdin"])
+        && let ExprKind::Path(QPath::Resolved(_, path)) = arg.peel_borrows().kind
+        && 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.
+        for_each_local_use_after_expr(cx, local_id, call.hir_id, |expr| {
+            if let Some(parent) = get_parent_expr(cx, expr)
+                && 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 TyKind::Adt(_, substs) = parse_result_ty.kind()
+                && let Some(ok_ty) = substs[0].as_type()
+                && parse_fails_on_trailing_newline(ok_ty)
+            {
+                let local_snippet = 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,
+                        );
+                    }
+                );
+            }
+
+            // only consider the first use to prevent this scenario:
+            // ```
+            // let mut s = String::new();
+            // std::io::stdin().read_line(&mut s);
+            // s.pop();
+            // let _x: i32 = s.parse().unwrap();
+            // ```
+            // this is actually fine, because the pop call removes the trailing newline.
+            ControlFlow::<(), ()>::Break(())
+        });
+    }
+}

tests/ui/read_line_without_trim.fixed

@@ -0,0 +1,19 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::read_line_without_trim)]
+
+fn main() {
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    input.pop();
+    let _x: i32 = input.parse().unwrap(); // don't trigger here, newline character is popped
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    let _x: i32 = input.trim_end().parse().unwrap();
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    let _x = input.trim_end().parse::<i32>().unwrap();
+}

tests/ui/read_line_without_trim.rs

@@ -0,0 +1,19 @@
+//@run-rustfix
+
+#![allow(unused)]
+#![warn(clippy::read_line_without_trim)]
+
+fn main() {
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    input.pop();
+    let _x: i32 = input.parse().unwrap(); // don't trigger here, newline character is popped
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    let _x: i32 = input.parse().unwrap();
+
+    let mut input = String::new();
+    std::io::stdin().read_line(&mut input).unwrap();
+    let _x = input.parse::<i32>().unwrap();
+}

tests/ui/read_line_without_trim.stderr

@@ -0,0 +1,31 @@
+error: calling `.parse()` without trimming the trailing newline character
+  --> $DIR/read_line_without_trim.rs:14: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
+  --> $DIR/read_line_without_trim.rs:13:5
+   |
+LL |     std::io::stdin().read_line(&mut input).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: `-D clippy::read-line-without-trim` implied by `-D warnings`
+
+error: calling `.parse()` without trimming the trailing newline character
+  --> $DIR/read_line_without_trim.rs:18: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
+  --> $DIR/read_line_without_trim.rs:17:5
+   |
+LL |     std::io::stdin().read_line(&mut input).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+