diff options
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 7 | ||||
| -rw-r--r-- | clippy_lints/src/methods/read_line_without_trim.rs | 122 | ||||
| -rw-r--r-- | tests/ui/read_line_without_trim.fixed | 13 | ||||
| -rw-r--r-- | tests/ui/read_line_without_trim.rs | 13 | ||||
| -rw-r--r-- | tests/ui/read_line_without_trim.stderr | 50 |
5 files changed, 138 insertions, 67 deletions
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 820f4c85890..adb696ad509 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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 diff --git a/clippy_lints/src/methods/read_line_without_trim.rs b/clippy_lints/src/methods/read_line_without_trim.rs index f4b4f73af56..0c8b6284284 100644 --- a/clippy_lints/src/methods/read_line_without_trim.rs +++ b/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, + ); + }); } } diff --git a/tests/ui/read_line_without_trim.fixed b/tests/ui/read_line_without_trim.fixed index 03a99b17dce..523ad555274 100644 --- a/tests/ui/read_line_without_trim.fixed +++ b/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"); + } } diff --git a/tests/ui/read_line_without_trim.rs b/tests/ui/read_line_without_trim.rs index 65510aea0fd..e31ff0cde61 100644 --- a/tests/ui/read_line_without_trim.rs +++ b/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"); + } } diff --git a/tests/ui/read_line_without_trim.stderr b/tests/ui/read_line_without_trim.stderr index 7d0ac26c3c5..b54229f762a 100644 --- a/tests/ui/read_line_without_trim.stderr +++ b/tests/ui/read_line_without_trim.stderr @@ -1,4 +1,4 @@ -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(); @@ -6,7 +6,7 @@ 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(); @@ -14,7 +14,7 @@ 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(); @@ -22,13 +22,13 @@ 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(); @@ -36,13 +36,13 @@ 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(); @@ -50,13 +50,13 @@ 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(); @@ -64,11 +64,39 @@ 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 |