Skip to content

Commit

Permalink
Auto merge of #5272 - jmeyers35:file_read_lint, r=flip1995
Browse files Browse the repository at this point in the history
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 rust-lang/rust-clippy#4916

changelog: add lint on File::{read_to_end, read_to_string}
  • Loading branch information
bors committed Mar 10, 2020
2 parents 9d5ffe8 + a4ba102 commit fdce47b
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1417,6 +1417,7 @@ Released 2018-09-13
[`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
[`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
[`verbose_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_bit_mask
[`verbose_file_reads`]: https://rust-lang.github.io/rust-clippy/master/index.html#verbose_file_reads
[`while_immutable_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_immutable_condition
[`while_let_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_loop
[`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -5,7 +5,7 @@

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

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

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

Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -310,6 +310,7 @@ pub mod unused_self;
pub mod unwrap;
pub mod use_self;
pub mod vec;
pub mod verbose_file_reads;
pub mod wildcard_dependencies;
pub mod wildcard_imports;
pub mod write;
Expand Down Expand Up @@ -815,6 +816,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&unwrap::UNNECESSARY_UNWRAP,
&use_self::USE_SELF,
&vec::USELESS_VEC,
&verbose_file_reads::VERBOSE_FILE_READS,
&wildcard_dependencies::WILDCARD_DEPENDENCIES,
&wildcard_imports::ENUM_GLOB_USE,
&wildcard_imports::WILDCARD_IMPORTS,
Expand Down Expand Up @@ -1015,6 +1017,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box option_env_unwrap::OptionEnvUnwrap);
store.register_late_pass(|| box wildcard_imports::WildcardImports);
store.register_early_pass(|| box macro_use::MacroUseImports);
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1372,6 +1375,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unwrap::PANICKING_UNWRAP),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&vec::USELESS_VEC),
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
LintId::of(&write::PRINTLN_EMPTY_STRING),
LintId::of(&write::PRINT_LITERAL),
LintId::of(&write::PRINT_WITH_NEWLINE),
Expand Down Expand Up @@ -1555,6 +1559,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::UNNECESSARY_CAST),
LintId::of(&types::VEC_BOX),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO),
]);

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Expand Up @@ -31,6 +31,7 @@ pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"];
pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
pub const EXIT: [&str; 3] = ["std", "process", "exit"];
pub const FILE: [&str; 3] = ["std", "fs", "File"];
pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"];
pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"];
Expand Down
83 changes: 83 additions & 0 deletions clippy_lints/src/verbose_file_reads.rs
@@ -0,0 +1,83 @@
use crate::utils::{match_type, paths, span_lint_and_help};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for use of File::read_to_end and File::read_to_string.
///
/// **Why is this bad?** `fs::{read, read_to_string}` provide the same functionality when `buf` is empty with fewer imports and no intermediate values.
/// 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)
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust,no_run
/// # use std::io::Read;
/// # use std::fs::File;
/// let mut f = File::open("foo.txt").unwrap();
/// let mut bytes = Vec::new();
/// f.read_to_end(&mut bytes).unwrap();
/// ```
/// Can be written more concisely as
/// ```rust,no_run
/// # use std::fs;
/// let mut bytes = fs::read("foo.txt").unwrap();
/// ```
pub VERBOSE_FILE_READS,
complexity,
"use of `File::read_to_end` or `File::read_to_string`"
}

declare_lint_pass!(VerboseFileReads => [VERBOSE_FILE_READS]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VerboseFileReads {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) {
if is_file_read_to_end(cx, expr) {
span_lint_and_help(
cx,
VERBOSE_FILE_READS,
expr.span,
"use of `File::read_to_end`",
"consider using `fs::read` instead",
);
} else if is_file_read_to_string(cx, expr) {
span_lint_and_help(
cx,
VERBOSE_FILE_READS,
expr.span,
"use of `File::read_to_string`",
"consider using `fs::read_to_string` instead",
)
}
}
}

fn is_file_read_to_end<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind;
if method_name.ident.as_str() == "read_to_end";
if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind;
let ty = cx.tables.expr_ty(&exprs[0]);
if match_type(cx, ty, &paths::FILE);
then {
return true
}
}
false
}

fn is_file_read_to_string<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let ExprKind::MethodCall(method_name, _, exprs) = expr.kind;
if method_name.ident.as_str() == "read_to_string";
if let ExprKind::Path(QPath::Resolved(None, _)) = &exprs[0].kind;
let ty = cx.tables.expr_ty(&exprs[0]);
if match_type(cx, ty, &paths::FILE);
then {
return true
}
}
false
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 360] = [
pub const ALL_LINTS: [Lint; 361] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -2401,6 +2401,13 @@ pub const ALL_LINTS: [Lint; 360] = [
deprecation: None,
module: "bit_mask",
},
Lint {
name: "verbose_file_reads",
group: "complexity",
desc: "use of `File::read_to_end` or `File::read_to_string`",
deprecation: None,
module: "verbose_file_reads",
},
Lint {
name: "while_immutable_condition",
group: "correctness",
Expand Down
28 changes: 28 additions & 0 deletions tests/ui/verbose_file_reads.rs
@@ -0,0 +1,28 @@
#![warn(clippy::verbose_file_reads)]
use std::env::temp_dir;
use std::fs::File;
use std::io::Read;

struct Struct;
// To make sure we only warn on File::{read_to_end, read_to_string} calls
impl Struct {
pub fn read_to_end(&self) {}

pub fn read_to_string(&self) {}
}

fn main() -> std::io::Result<()> {
let path = "foo.txt";
// Lint shouldn't catch this
let s = Struct;
s.read_to_end();
s.read_to_string();
// Should catch this
let mut f = File::open(&path)?;
let mut buffer = Vec::new();
f.read_to_end(&mut buffer)?;
// ...and this
let mut string_buffer = String::new();
f.read_to_string(&mut string_buffer)?;
Ok(())
}
19 changes: 19 additions & 0 deletions tests/ui/verbose_file_reads.stderr
@@ -0,0 +1,19 @@
error: use of `File::read_to_end`
--> $DIR/verbose_file_reads.rs:23:5
|
LL | f.read_to_end(&mut buffer)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::verbose-file-reads` implied by `-D warnings`
= help: consider using `fs::read` instead

error: use of `File::read_to_string`
--> $DIR/verbose_file_reads.rs:26:5
|
LL | f.read_to_string(&mut string_buffer)?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using `fs::read_to_string` instead

error: aborting due to 2 previous errors

0 comments on commit fdce47b

Please sign in to comment.