diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 1b275516dfce..db475950086d 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -370,6 +370,7 @@ store.register_lints(&[ methods::UNWRAP_USED, methods::USELESS_ASREF, methods::VEC_RESIZE_TO_ZERO, + methods::VERBOSE_FILE_READS, methods::WRONG_SELF_CONVENTION, methods::ZST_OFFSET, minmax::MIN_MAX, @@ -585,7 +586,6 @@ store.register_lints(&[ useless_conversion::USELESS_CONVERSION, vec::USELESS_VEC, vec_init_then_push::VEC_INIT_THEN_PUSH, - verbose_file_reads::VERBOSE_FILE_READS, wildcard_imports::ENUM_GLOB_USE, wildcard_imports::WILDCARD_IMPORTS, write::POSITIONAL_NAMED_FORMAT_PARAMETERS, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 7fc5eef5f8a6..dd1e1e1a8e33 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -40,6 +40,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(methods::GET_UNWRAP), LintId::of(methods::MAP_ERR_IGNORE), LintId::of(methods::UNWRAP_USED), + LintId::of(methods::VERBOSE_FILE_READS), LintId::of(misc_early::SEPARATED_LITERAL_SUFFIX), LintId::of(misc_early::UNNEEDED_FIELD_PATTERN), LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX), @@ -81,7 +82,6 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(unicode::NON_ASCII_LITERAL), LintId::of(unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS), LintId::of(unwrap_in_result::UNWRAP_IN_RESULT), - LintId::of(verbose_file_reads::VERBOSE_FILE_READS), LintId::of(write::PRINT_STDERR), LintId::of(write::PRINT_STDOUT), LintId::of(write::USE_DEBUG), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ff68fbd19643..dbea55a04d62 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -391,7 +391,6 @@ mod use_self; mod useless_conversion; mod vec; mod vec_init_then_push; -mod verbose_file_reads; mod wildcard_imports; mod write; mod zero_div_zero; @@ -792,7 +791,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(option_env_unwrap::OptionEnvUnwrap)); let warn_on_all_wildcard_imports = conf.warn_on_all_wildcard_imports; store.register_late_pass(move || Box::new(wildcard_imports::WildcardImports::new(warn_on_all_wildcard_imports))); - store.register_late_pass(|| Box::new(verbose_file_reads::VerboseFileReads)); store.register_late_pass(|| Box::new(redundant_pub_crate::RedundantPubCrate::default())); store.register_late_pass(|| Box::new(unnamed_address::UnnamedAddress)); store.register_late_pass(move || Box::new(dereference::Dereferencing::new(msrv))); diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 1a225c9bc951..1cfe8c4191ef 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -91,6 +91,7 @@ mod unwrap_used; mod useless_asref; mod utils; mod vec_resize_to_zero; +mod verbose_file_reads; mod wrong_self_convention; mod zst_offset; @@ -2930,6 +2931,33 @@ declare_clippy_lint! { "emptying a vector with `resize(0, an_int)` instead of `clear()` is probably an argument inversion mistake" } +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) + /// + /// ### 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(); + /// ``` + #[clippy::version = "1.44.0"] + pub VERBOSE_FILE_READS, + restriction, + "use of `File::read_to_end` or `File::read_to_string`" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -3050,6 +3078,7 @@ impl_lint_pass!(Methods => [ UNIT_HASH, UNNECESSARY_SORT_BY, VEC_RESIZE_TO_ZERO, + VERBOSE_FILE_READS, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3441,6 +3470,12 @@ impl Methods { ("push", [arg]) => { path_buf_push_overwrite::check(cx, expr, arg); }, + ("read_to_end", [_]) => { + verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_END_MSG); + }, + ("read_to_string", [_]) => { + verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_STRING_MSG); + }, ("repeat", [arg]) => { repeat_once::check(cx, expr, recv, arg); }, diff --git a/clippy_lints/src/methods/verbose_file_reads.rs b/clippy_lints/src/methods/verbose_file_reads.rs new file mode 100644 index 000000000000..2fe5ae9a9ad8 --- /dev/null +++ b/clippy_lints/src/methods/verbose_file_reads.rs @@ -0,0 +1,28 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::is_trait_method; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::LateContext; +use rustc_span::sym; + +use super::VERBOSE_FILE_READS; + +pub(super) const READ_TO_END_MSG: (&str, &str) = ("use of `File::read_to_end`", "consider using `fs::read` instead"); +pub(super) const READ_TO_STRING_MSG: (&str, &str) = ( + "use of `File::read_to_string`", + "consider using `fs::read_to_string` instead", +); + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + recv: &'tcx Expr<'_>, + (msg, help): (&str, &str), +) { + if is_trait_method(cx, expr, sym::IoRead) + && matches!(recv.kind, ExprKind::Path(QPath::Resolved(None, _))) + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty_adjusted(recv).peel_refs(), sym::File) + { + span_lint_and_help(cx, VERBOSE_FILE_READS, expr.span, msg, None, help); + } +} diff --git a/clippy_lints/src/verbose_file_reads.rs b/clippy_lints/src/verbose_file_reads.rs deleted file mode 100644 index afd0077a6580..000000000000 --- a/clippy_lints/src/verbose_file_reads.rs +++ /dev/null @@ -1,88 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::paths; -use clippy_utils::ty::match_type; -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) - /// - /// ### 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(); - /// ``` - #[clippy::version = "1.44.0"] - pub VERBOSE_FILE_READS, - restriction, - "use of `File::read_to_end` or `File::read_to_string`" -} - -declare_lint_pass!(VerboseFileReads => [VERBOSE_FILE_READS]); - -impl<'tcx> LateLintPass<'tcx> for VerboseFileReads { - fn check_expr(&mut self, cx: &LateContext<'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`", - None, - "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`", - None, - "consider using `fs::read_to_string` instead", - ); - } - } -} - -fn is_file_read_to_end<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { - if_chain! { - if let ExprKind::MethodCall(method_name, [recv, ..], _) = expr.kind; - if method_name.ident.as_str() == "read_to_end"; - if let ExprKind::Path(QPath::Resolved(None, _)) = &recv.kind; - let ty = cx.typeck_results().expr_ty(recv); - if match_type(cx, ty, &paths::FILE); - then { - return true - } - } - false -} - -fn is_file_read_to_string<'tcx>(cx: &LateContext<'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.typeck_results().expr_ty(&exprs[0]); - if match_type(cx, ty, &paths::FILE); - then { - return true - } - } - false -}