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 https://github.com/rust-lang/rust-clippy/issues/4916

changelog: add lint on File::{read_to_end, read_to_string}

CHANGELOG.md

@@ -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

README.md

@@ -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:
 

clippy_lints/src/lib.rs

@@ -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;
@@ -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,
@@ -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),
@@ -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),
@@ -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),
     ]);
 

clippy_lints/src/utils/paths.rs

@@ -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"];

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
+}

src/lintlist/mod.rs

@@ -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",
@@ -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",

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(())
+}

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
+