From 6efb3a2b9ac9e0477b6b222c051c15c98233b424 Mon Sep 17 00:00:00 2001 From: koka Date: Tue, 25 Oct 2022 12:26:06 +0900 Subject: [PATCH] feat: add new lint `seek_from_current` addresses https://github.com/rust-lang/rust-clippy/issues/7886 added `seek_from_current` complexity lint. it checks use of `Seek#seek` with `SeekFrom::Current(0)` and suggests `Seek#stream_position` method fix: add msrv fix: register LintInfo fix: remove unnecessary files fix: add test for msrv fix: remove fix fix: remove docs --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 48 +++++++++++++++++++ clippy_lints/src/methods/seek_from_current.rs | 48 +++++++++++++++++++ clippy_utils/src/msrvs.rs | 2 +- clippy_utils/src/paths.rs | 1 + tests/ui/seek_from_current.fixed | 26 ++++++++++ tests/ui/seek_from_current.rs | 26 ++++++++++ tests/ui/seek_from_current.stderr | 10 ++++ 9 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/methods/seek_from_current.rs create mode 100644 tests/ui/seek_from_current.fixed create mode 100644 tests/ui/seek_from_current.rs create mode 100644 tests/ui/seek_from_current.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e91365c69a..b47e80f6979 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4200,6 +4200,7 @@ Released 2018-09-13 [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push [`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some +[`seek_from_current`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current [`seek_to_start_instead_of_rewind`]: https://rust-lang.github.io/rust-clippy/master/index.html#seek_to_start_instead_of_rewind [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a903d46b2a4..fff26307e34 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -363,6 +363,7 @@ crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, crate::methods::SEARCH_IS_SOME_INFO, + crate::methods::SEEK_FROM_CURRENT_INFO, crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO, crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO, crate::methods::SINGLE_CHAR_ADD_STR_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 4fd1e3e54ae..b29c7d9f253 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -69,6 +69,7 @@ mod range_zip_with_len; mod repeat_once; mod search_is_some; +mod seek_from_current; mod seek_to_start_instead_of_rewind; mod single_char_add_str; mod single_char_insert_string; @@ -3067,6 +3068,49 @@ "iterating on map using `iter` when `keys` or `values` would do" } +declare_clippy_lint! { + /// ### What it does + /// + /// Checks an argument of `seek` method of `Seek` trait + /// and if it start seek from `SeekFrom::Current(0)`, suggests `stream_position` instead. + /// + /// ### Why is this bad? + /// + /// Readability. Use dedicated method. + /// + /// ### Example + /// + /// ```rust + /// use std::fs::File; + /// use std::io::{self, Write, Seek, SeekFrom}; + /// + /// fn main() -> io::Result<()> { + /// let mut f = File::create("foo.txt")?; + /// f.write_all(b"Hello")?; + /// eprintln!("Written {} bytes", f.seek(SeekFrom::Current(0))?); + /// + /// Ok(()) + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::fs::File; + /// use std::io::{self, Write, Seek, SeekFrom}; + /// + /// fn main() -> io::Result<()> { + /// let mut f = File::create("foo.txt")?; + /// f.write_all(b"Hello")?; + /// eprintln!("Written {} bytes", f.stream_position()?); + /// + /// Ok(()) + /// } + /// ``` + #[clippy::version = "1.66.0"] + pub SEEK_FROM_CURRENT, + complexity, + "use dedicated method for seek from current position" +} + declare_clippy_lint! { /// ### What it does /// @@ -3222,6 +3266,7 @@ pub fn new( VEC_RESIZE_TO_ZERO, VERBOSE_FILE_READS, ITER_KV_MAP, + SEEK_FROM_CURRENT, SEEK_TO_START_INSTEAD_OF_REWIND, ]); @@ -3638,6 +3683,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span); }, ("seek", [arg]) => { + if meets_msrv(self.msrv, msrvs::SEEK_FROM_CURRENT) { + seek_from_current::check(cx, expr, recv, arg); + } if meets_msrv(self.msrv, msrvs::SEEK_REWIND) { seek_to_start_instead_of_rewind::check(cx, expr, recv, arg, span); } diff --git a/clippy_lints/src/methods/seek_from_current.rs b/clippy_lints/src/methods/seek_from_current.rs new file mode 100644 index 00000000000..361a3082f94 --- /dev/null +++ b/clippy_lints/src/methods/seek_from_current.rs @@ -0,0 +1,48 @@ +use rustc_ast::ast::{LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; + +use clippy_utils::{ + diagnostics::span_lint_and_sugg, get_trait_def_id, match_def_path, paths, source::snippet_with_applicability, + ty::implements_trait, +}; + +use super::SEEK_FROM_CURRENT; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'tcx Expr<'_>, arg: &'tcx Expr<'_>) { + let ty = cx.typeck_results().expr_ty(recv); + + if let Some(def_id) = get_trait_def_id(cx, &paths::STD_IO_SEEK) { + if implements_trait(cx, ty, def_id, &[]) && arg_is_seek_from_current(cx, arg) { + let mut applicability = Applicability::MachineApplicable; + let snip = snippet_with_applicability(cx, recv.span, "..", &mut applicability); + + span_lint_and_sugg( + cx, + SEEK_FROM_CURRENT, + expr.span, + "using `SeekFrom::Current` to start from current position", + "replace with", + format!("{snip}.stream_position()"), + applicability, + ); + } + } +} + +fn arg_is_seek_from_current<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool { + if let ExprKind::Call(f, args) = expr.kind && + let ExprKind::Path(ref path) = f.kind && + let Some(def_id) = cx.qpath_res(path, f.hir_id).opt_def_id() && + match_def_path(cx, def_id, &paths::STD_IO_SEEK_FROM_CURRENT) { + // check if argument of `SeekFrom::Current` is `0` + if args.len() == 1 && + let ExprKind::Lit(ref lit) = args[0].kind && + let LitKind::Int(0, LitIntType::Unsuffixed) = lit.node { + return true + } + } + + false +} diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 04b504d044d..9a672e2ddfd 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -17,7 +17,7 @@ macro_rules! msrv_aliases { 1,58,0 { FORMAT_ARGS_CAPTURE } 1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR } 1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST } - 1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS } + 1,51,0 { BORROW_AS_PTR, SEEK_FROM_CURRENT, UNSIGNED_ABS } 1,50,0 { BOOL_THEN, CLAMP } 1,47,0 { TAU } 1,46,0 { CONST_IF_MATCH } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index e37c7e34c0c..6c09c146082 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -116,6 +116,7 @@ pub const CONVERT_IDENTITY: [&str; 3] = ["core", "convert", "identity"]; pub const STD_FS_CREATE_DIR: [&str; 3] = ["std", "fs", "create_dir"]; pub const STD_IO_SEEK: [&str; 3] = ["std", "io", "Seek"]; +pub const STD_IO_SEEK_FROM_CURRENT: [&str; 4] = ["std", "io", "SeekFrom", "Current"]; pub const STD_IO_SEEKFROM_START: [&str; 4] = ["std", "io", "SeekFrom", "Start"]; pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"]; pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"]; diff --git a/tests/ui/seek_from_current.fixed b/tests/ui/seek_from_current.fixed new file mode 100644 index 00000000000..4b5303324bc --- /dev/null +++ b/tests/ui/seek_from_current.fixed @@ -0,0 +1,26 @@ +// run-rustfix +#![warn(clippy::seek_from_current)] +#![feature(custom_inner_attributes)] + +use std::fs::File; +use std::io::{self, Seek, SeekFrom, Write}; + +fn _msrv_1_50() -> io::Result<()> { + #![clippy::msrv = "1.50"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn _msrv_1_51() -> io::Result<()> { + #![clippy::msrv = "1.51"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.stream_position()?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn main() {} diff --git a/tests/ui/seek_from_current.rs b/tests/ui/seek_from_current.rs new file mode 100644 index 00000000000..f93639261a1 --- /dev/null +++ b/tests/ui/seek_from_current.rs @@ -0,0 +1,26 @@ +// run-rustfix +#![warn(clippy::seek_from_current)] +#![feature(custom_inner_attributes)] + +use std::fs::File; +use std::io::{self, Seek, SeekFrom, Write}; + +fn _msrv_1_50() -> io::Result<()> { + #![clippy::msrv = "1.50"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn _msrv_1_51() -> io::Result<()> { + #![clippy::msrv = "1.51"] + let mut f = File::create("foo.txt")?; + f.write_all(b"Hi!")?; + f.seek(SeekFrom::Current(0))?; + f.seek(SeekFrom::Current(1))?; + Ok(()) +} + +fn main() {} diff --git a/tests/ui/seek_from_current.stderr b/tests/ui/seek_from_current.stderr new file mode 100644 index 00000000000..db1125b53cd --- /dev/null +++ b/tests/ui/seek_from_current.stderr @@ -0,0 +1,10 @@ +error: using `SeekFrom::Current` to start from current position + --> $DIR/seek_from_current.rs:21:5 + | +LL | f.seek(SeekFrom::Current(0))?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `f.stream_position()` + | + = note: `-D clippy::seek-from-current` implied by `-D warnings` + +error: aborting due to previous error +