diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 5 | ||||
| -rw-r--r-- | clippy_lints/src/utils/paths.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/verbose_file_reads.rs | 83 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 9 | ||||
| -rw-r--r-- | tests/ui/verbose_file_reads.rs | 28 | ||||
| -rw-r--r-- | tests/ui/verbose_file_reads.stderr | 19 |
8 files changed, 146 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c3f291bc046..32cbbe80101 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 8635b2b6d5a..2181c296e9b 100644 --- a/README.md +++ b/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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 02c016c385d..1eac69678a6 100644 --- a/clippy_lints/src/lib.rs +++ b/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), ]); diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 5ec04d965c8..6cb1f694fd5 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/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"]; diff --git a/clippy_lints/src/verbose_file_reads.rs b/clippy_lints/src/verbose_file_reads.rs new file mode 100644 index 00000000000..37885317c58 --- /dev/null +++ b/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 +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 4ae4ccc91f0..fd948953ea2 100644 --- a/src/lintlist/mod.rs +++ b/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", @@ -2402,6 +2402,13 @@ pub const ALL_LINTS: [Lint; 360] = [ 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", desc: "variables used within while expression are not mutated in the body", diff --git a/tests/ui/verbose_file_reads.rs b/tests/ui/verbose_file_reads.rs new file mode 100644 index 00000000000..e0065e05ade --- /dev/null +++ b/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(()) +} diff --git a/tests/ui/verbose_file_reads.stderr b/tests/ui/verbose_file_reads.stderr new file mode 100644 index 00000000000..550b6ab679f --- /dev/null +++ b/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 + |