Skip to content

Commit fdce47b

Browse files
committed
Auto merge of #5272 - jmeyers35:file_read_lint, r=flip1995
add lint on File::read_to_string and File::read_to_end Adds lint `verbose_file_reads` which checks for use of File::read_to_end and File::read_to_string. Closes #4916 changelog: add lint on File::{read_to_end, read_to_string}
2 parents 9d5ffe8 + a4ba102 commit fdce47b

File tree

8 files changed

+146
-2
lines changed

8 files changed

+146
-2
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1417,6 +1417,7 @@ Released 2018-09-13
14171417
[`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
14181418
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
14191419
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
1420+
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
14201421
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
14211422
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
14221423
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are 360 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1111

clippy_lints/src/lib.rs

+5
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,7 @@ pub mod unused_self;
310310
pub mod unwrap;
311311
pub mod use_self;
312312
pub mod vec;
313+
pub mod verbose_file_reads;
313314
pub mod wildcard_dependencies;
314315
pub mod wildcard_imports;
315316
pub mod write;
@@ -815,6 +816,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
815816
&unwrap::UNNECESSARY_UNWRAP,
816817
&use_self::USE_SELF,
817818
&vec::USELESS_VEC,
819+
&verbose_file_reads::VERBOSE_FILE_READS,
818820
&wildcard_dependencies::WILDCARD_DEPENDENCIES,
819821
&wildcard_imports::ENUM_GLOB_USE,
820822
&wildcard_imports::WILDCARD_IMPORTS,
@@ -1015,6 +1017,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10151017
store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap);
10161018
store.register_late_pass(|| box wildcard_imports::WildcardImports);
10171019
store.register_early_pass(|| box macro_use::MacroUseImports);
1020+
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
10181021

10191022
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
10201023
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1372,6 +1375,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13721375
LintId::of(&unwrap::PANICKING_UNWRAP),
13731376
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
13741377
LintId::of(&vec::USELESS_VEC),
1378+
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
13751379
LintId::of(&write::PRINTLN_EMPTY_STRING),
13761380
LintId::of(&write::PRINT_LITERAL),
13771381
LintId::of(&write::PRINT_WITH_NEWLINE),
@@ -1555,6 +1559,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15551559
LintId::of(&types::UNNECESSARY_CAST),
15561560
LintId::of(&types::VEC_BOX),
15571561
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
1562+
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
15581563
LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO),
15591564
]);
15601565

clippy_lints/src/utils/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"];
3131
pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
3232
pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
3333
pub const EXIT: [&str; 3] = ["std", "process", "exit"];
34+
pub const FILE: [&str; 3] = ["std", "fs", "File"];
3435
pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"];
3536
pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
3637
pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"];
+83
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use crate::utils::{match_type, paths, span_lint_and_help};
2+
use if_chain::if_chain;
3+
use rustc_hir::{Expr, ExprKind, QPath};
4+
use rustc_lint::{LateContext, LateLintPass};
5+
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
7+
declare_clippy_lint! {
8+
/// **What it does:** Checks for use of File::read_to_end and File::read_to_string.
9+
///
10+
/// **Why is this bad?** `fs::{read, read_to_string}` provide the same functionality when `buf` is empty with fewer imports and no intermediate values.
11+
/// See also: [fs::read docs](https://doc.rust-lang.org/std/fs/fn.read.html), [fs::read_to_string docs](https://doc.rust-lang.org/std/fs/fn.read_to_string.html)
12+
/// **Known problems:** None.
13+
///
14+
/// **Example:**
15+
///
16+
/// ```rust,no_run
17+
/// # use std::io::Read;
18+
/// # use std::fs::File;
19+
/// let mut f = File::open("foo.txt").unwrap();
20+
/// let mut bytes = Vec::new();
21+
/// f.read_to_end(&mut bytes).unwrap();
22+
/// ```
23+
/// Can be written more concisely as
24+
/// ```rust,no_run
25+
/// # use std::fs;
26+
/// let mut bytes = fs::read("foo.txt").unwrap();
27+
/// ```
28+
pub VERBOSE_FILE_READS,
29+
complexity,
30+
"use of `File::read_to_end` or `File::read_to_string`"
31+
}
32+
33+
declare_lint_pass!(VerboseFileReads => [VERBOSE_FILE_READS]);
34+
35+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VerboseFileReads {
36+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) {
37+
if is_file_read_to_end(cx, expr) {
38+
span_lint_and_help(
39+
cx,
40+
VERBOSE_FILE_READS,
41+
expr.span,
42+
"use of `File::read_to_end`",
43+
"consider using `fs::read` instead",
44+
);
45+
} else if is_file_read_to_string(cx, expr) {
46+
span_lint_and_help(
47+
cx,
48+
VERBOSE_FILE_READS,
49+
expr.span,
50+
"use of `File::read_to_string`",
51+
"consider using `fs::read_to_string` instead",
52+
)
53+
}
54+
}
55+
}
56+
57+
fn is_file_read_to_end<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
58+
if_chain! {
59+
if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind;
60+
if method_name.ident.as_str() == "read_to_end";
61+
if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind;
62+
let ty = cx.tables.expr_ty(&exprs[0]);
63+
if match_type(cx, ty, &paths::FILE);
64+
then {
65+
return true
66+
}
67+
}
68+
false
69+
}
70+
71+
fn is_file_read_to_string<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
72+
if_chain! {
73+
if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind;
74+
if method_name.ident.as_str() == "read_to_string";
75+
if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind;
76+
let ty = cx.tables.expr_ty(&exprs[0]);
77+
if match_type(cx, ty, &paths::FILE);
78+
then {
79+
return true
80+
}
81+
}
82+
false
83+
}

src/lintlist/mod.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 360] = [
9+
pub const ALL_LINTS: [Lint; 361] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -2401,6 +2401,13 @@ pub const ALL_LINTS: [Lint; 360] = [
24012401
deprecation: None,
24022402
module: "bit_mask",
24032403
},
2404+
Lint {
2405+
name: "verbose_file_reads",
2406+
group: "complexity",
2407+
desc: "use of `File::read_to_end` or `File::read_to_string`",
2408+
deprecation: None,
2409+
module: "verbose_file_reads",
2410+
},
24042411
Lint {
24052412
name: "while_immutable_condition",
24062413
group: "correctness",

tests/ui/verbose_file_reads.rs

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
#![warn(clippy::verbose_file_reads)]
2+
use std::env::temp_dir;
3+
use std::fs::File;
4+
use std::io::Read;
5+
6+
struct Struct;
7+
// To make sure we only warn on File::{read_to_end, read_to_string} calls
8+
impl Struct {
9+
pub fn read_to_end(&self) {}
10+
11+
pub fn read_to_string(&self) {}
12+
}
13+
14+
fn main() -> std::io::Result<()> {
15+
let path = "foo.txt";
16+
// Lint shouldn't catch this
17+
let s = Struct;
18+
s.read_to_end();
19+
s.read_to_string();
20+
// Should catch this
21+
let mut f = File::open(&path)?;
22+
let mut buffer = Vec::new();
23+
f.read_to_end(&mut buffer)?;
24+
// ...and this
25+
let mut string_buffer = String::new();
26+
f.read_to_string(&mut string_buffer)?;
27+
Ok(())
28+
}

tests/ui/verbose_file_reads.stderr

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: use of `File::read_to_end`
2+
--> $DIR/verbose_file_reads.rs:23:5
3+
|
4+
LL | f.read_to_end(&mut buffer)?;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::verbose-file-reads` implied by `-D warnings`
8+
= help: consider using `fs::read` instead
9+
10+
error: use of `File::read_to_string`
11+
--> $DIR/verbose_file_reads.rs:26:5
12+
|
13+
LL | f.read_to_string(&mut string_buffer)?;
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= help: consider using `fs::read_to_string` instead
17+
18+
error: aborting due to 2 previous errors
19+

0 commit comments

Comments
 (0)