add lint on File::read_to_string and File::read_to_end

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,82 @@
+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.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust, ignore
+ /// let mut f = File::open("foo.txt")?;
+ /// let mut bytes = Vec::new();
+ /// f.read_to_end(&mut bytes)?;
+ /// ```
+ /// Can be written more concisely as
+ /// ```rust, ignore
+ /// let mut bytes = fs::read("foo.txt")?;
+ /// ```
+ 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",
+ )
+ } else {
+ // Don't care
+ }
+ }
+}
+
+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,30 @@
+#![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 mut path = temp_dir();
+ path.push("test.txt");
+ let file = File::create(&path)?;
+ // 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:25: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:28:5
+ |
+LL | f.read_to_string(&mut string_buffer)?;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+ = help: consider using fs::read_to_string instead
+
+error: aborting due to 2 previous errors
+