Skip to content

Commit 8fad54e

Browse files
y21y21
committed
new lint: read_line_without_trim 
1 parent ba3bd8f commit 8fad54e

7 files changed

+179
-0
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5134,6 +5134,7 @@ Released 2018-09-13
51345134
[`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
51355135
[`rc_clone_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_clone_in_vec_init
51365136
[`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex
5137+
[`read_line_without_trim`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_line_without_trim
51375138
[`read_zero_byte_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec
51385139
[`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl
51395140
[`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
389389
crate::methods::OR_THEN_UNWRAP_INFO,
390390
crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO,
391391
crate::methods::RANGE_ZIP_WITH_LEN_INFO,
392+
crate::methods::READ_LINE_WITHOUT_TRIM_INFO,
392393
crate::methods::REPEAT_ONCE_INFO,
393394
crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO,
394395
crate::methods::SEARCH_IS_SOME_INFO,

‎clippy_lints/src/methods/mod.rs

+34
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ mod or_fun_call;
7272
mod or_then_unwrap;
7373
mod path_buf_push_overwrite;
7474
mod range_zip_with_len;
75+
mod read_line_without_trim;
7576
mod repeat_once;
7677
mod search_is_some;
7778
mod seek_from_current;
@@ -3348,6 +3349,35 @@ declare_clippy_lint! {
33483349
"checks for usage of `Iterator::fold` with a type that implements `Try`"
33493350
}
33503351

3352+
declare_clippy_lint! {
3353+
/// Looks for calls to [`Stdin::read_line`] to read a line from the standard input
3354+
/// into a string, then later attempting to parse this string into a type without first trimming it, which will
3355+
/// always fail because the string has a trailing newline in it.
3356+
///
3357+
/// ### Why is this bad?
3358+
/// The `.parse()` call will always fail.
3359+
///
3360+
/// ### Example
3361+
/// ```rust,ignore
3362+
/// let mut input = String::new();
3363+
/// std::io::stdin().read_line(&mut input).expect("Failed to read a line");
3364+
/// let num: i32 = input.parse().expect("Not a number!");
3365+
/// assert_eq!(num, 42); // we never even get here!
3366+
/// ```
3367+
/// Use instead:
3368+
/// ```rust,ignore
3369+
/// let mut input = String::new();
3370+
/// std::io::stdin().read_line(&mut input).expect("Failed to read a line");
3371+
/// let num: i32 = input.trim_end().parse().expect("Not a number!");
3372+
/// // ^^^^^^^^^^^ remove the trailing newline
3373+
/// assert_eq!(num, 42);
3374+
/// ```
3375+
#[clippy::version = "1.72.0"]
3376+
pub READ_LINE_WITHOUT_TRIM,
3377+
correctness,
3378+
"calling `Stdin::read_line`, then trying to parse it without first trimming"
3379+
}
3380+
33513381
pub struct Methods {
33523382
avoid_breaking_exported_api: bool,
33533383
msrv: Msrv,
@@ -3468,6 +3498,7 @@ impl_lint_pass!(Methods => [
34683498
REPEAT_ONCE,
34693499
STABLE_SORT_PRIMITIVE,
34703500
UNIT_HASH,
3501+
READ_LINE_WITHOUT_TRIM,
34713502
UNNECESSARY_SORT_BY,
34723503
VEC_RESIZE_TO_ZERO,
34733504
VERBOSE_FILE_READS,
@@ -3879,6 +3910,9 @@ impl Methods {
38793910
("read_to_string", [_]) => {
38803911
verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_STRING_MSG);
38813912
},
3913+
("read_line", [arg]) => {
3914+
read_line_without_trim::check(cx, expr, recv, arg);
3915+
}
38823916
("repeat", [arg]) => {
38833917
repeat_once::check(cx, expr, recv, arg);
38843918
},

‎clippy_lints/src/methods/read_line_without_trim.rs

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
use std::ops::ControlFlow;
2+
3+
use clippy_utils::{
4+
diagnostics::span_lint_and_then, get_parent_expr, match_def_path, source::snippet, ty::is_type_diagnostic_item,
5+
visitors::for_each_local_use_after_expr,
6+
};
7+
use rustc_errors::Applicability;
8+
use rustc_hir::Expr;
9+
use rustc_hir::QPath;
10+
use rustc_hir::{def::Res, ExprKind};
11+
use rustc_lint::LateContext;
12+
use rustc_middle::ty::{self, Ty};
13+
use rustc_span::sym;
14+
15+
use super::READ_LINE_WITHOUT_TRIM;
16+
17+
/// Will a `.parse::<ty>()` call fail if the input has a trailing newline?
18+
fn parse_fails_on_trailing_newline(ty: Ty<'_>) -> bool {
19+
// only allow a very limited set of types for now, for which we 100% know parsing will fail
20+
matches!(ty.kind(), ty::Float(_) | ty::Bool | ty::Int(_) | ty::Uint(_))
21+
}
22+
23+
pub fn check(cx: &LateContext<'_>, call: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
24+
if let Some(recv_adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
25+
&& match_def_path(cx, recv_adt.did(), &["std", "io", "stdio", "Stdin"])
26+
&& let ExprKind::Path(QPath::Resolved(_, path)) = arg.peel_borrows().kind
27+
&& let Res::Local(local_id) = path.res
28+
{
29+
// We've checked that `call` is a call to `Stdin::read_line()` with the right receiver,
30+
// now let's check if the first use of the string passed to `::read_line()` is
31+
// parsed into a type that will always fail if it has a trailing newline.
32+
for_each_local_use_after_expr(cx, local_id, call.hir_id, |expr| {
33+
if let Some(parent) = get_parent_expr(cx, expr)
34+
&& let ExprKind::MethodCall(segment, .., span) = parent.kind
35+
&& segment.ident.name == sym!(parse)
36+
&& let parse_result_ty = cx.typeck_results().expr_ty(parent)
37+
&& is_type_diagnostic_item(cx, parse_result_ty, sym::Result)
38+
&& let ty::Adt(_, substs) = parse_result_ty.kind()
39+
&& let Some(ok_ty) = substs[0].as_type()
40+
&& parse_fails_on_trailing_newline(ok_ty)
41+
{
42+
let local_snippet = snippet(cx, expr.span, "<expr>");
43+
span_lint_and_then(
44+
cx,
45+
READ_LINE_WITHOUT_TRIM,
46+
span,
47+
"calling `.parse()` without trimming the trailing newline character",
48+
|diag| {
49+
diag.span_note(call.span, "call to `.read_line()` here, \
50+
which leaves a trailing newline character in the buffer, \
51+
which in turn will cause `.parse()` to fail");
52+
53+
diag.span_suggestion(
54+
expr.span,
55+
"try",
56+
format!("{local_snippet}.trim_end()"),
57+
Applicability::MachineApplicable,
58+
);
59+
}
60+
);
61+
}
62+
63+
// only consider the first use to prevent this scenario:
64+
// ```
65+
// let mut s = String::new();
66+
// std::io::stdin().read_line(&mut s);
67+
// s.pop();
68+
// let _x: i32 = s.parse().unwrap();
69+
// ```
70+
// this is actually fine, because the pop call removes the trailing newline.
71+
ControlFlow::<(), ()>::Break(())
72+
});
73+
}
74+
}

‎tests/ui/read_line_without_trim.fixed

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
#![warn(clippy::read_line_without_trim)]
5+
6+
fn main() {
7+
let mut input = String::new();
8+
std::io::stdin().read_line(&mut input).unwrap();
9+
input.pop();
10+
let _x: i32 = input.parse().unwrap(); // don't trigger here, newline character is popped
11+
12+
let mut input = String::new();
13+
std::io::stdin().read_line(&mut input).unwrap();
14+
let _x: i32 = input.trim_end().parse().unwrap();
15+
16+
let mut input = String::new();
17+
std::io::stdin().read_line(&mut input).unwrap();
18+
let _x = input.trim_end().parse::<i32>().unwrap();
19+
}

‎tests/ui/read_line_without_trim.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
#![warn(clippy::read_line_without_trim)]
5+
6+
fn main() {
7+
let mut input = String::new();
8+
std::io::stdin().read_line(&mut input).unwrap();
9+
input.pop();
10+
let _x: i32 = input.parse().unwrap(); // don't trigger here, newline character is popped
11+
12+
let mut input = String::new();
13+
std::io::stdin().read_line(&mut input).unwrap();
14+
let _x: i32 = input.parse().unwrap();
15+
16+
let mut input = String::new();
17+
std::io::stdin().read_line(&mut input).unwrap();
18+
let _x = input.parse::<i32>().unwrap();
19+
}

‎tests/ui/read_line_without_trim.stderr

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
error: calling `.parse()` without trimming the trailing newline character
2+
--> $DIR/read_line_without_trim.rs:14:25
3+
|
4+
LL | let _x: i32 = input.parse().unwrap();
5+
| ----- ^^^^^^^
6+
| |
7+
| help: try: `input.trim_end()`
8+
|
9+
note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
10+
--> $DIR/read_line_without_trim.rs:13:5
11+
|
12+
LL | std::io::stdin().read_line(&mut input).unwrap();
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
= note: `-D clippy::read-line-without-trim` implied by `-D warnings`
15+
16+
error: calling `.parse()` without trimming the trailing newline character
17+
--> $DIR/read_line_without_trim.rs:18:20
18+
|
19+
LL | let _x = input.parse::<i32>().unwrap();
20+
| ----- ^^^^^^^^^^^^^^
21+
| |
22+
| help: try: `input.trim_end()`
23+
|
24+
note: call to `.read_line()` here, which leaves a trailing newline character in the buffer, which in turn will cause `.parse()` to fail
25+
--> $DIR/read_line_without_trim.rs:17:5
26+
|
27+
LL | std::io::stdin().read_line(&mut input).unwrap();
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
30+
error: aborting due to 2 previous errors
31+

0 commit comments

Comments
 (0)