From 8d6ce3177bd5d4d05723dec1cdd869effc926fc3 Mon Sep 17 00:00:00 2001 From: Doru-Florin Blanzeanu Date: Sun, 16 Oct 2022 19:31:03 +0000 Subject: [PATCH] Add new lint `rewind_instead_of_seek_to_start` Signed-off-by: Doru-Florin Blanzeanu --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/mod.rs | 36 +++++++ .../rewind_instead_of_seek_to_start.rs | 45 +++++++++ clippy_utils/src/paths.rs | 2 + src/docs/rewind_instead_of_seek_to_start.txt | 22 +++++ .../ui/rewind_instead_of_seek_to_start.fixed | 94 +++++++++++++++++++ tests/ui/rewind_instead_of_seek_to_start.rs | 94 +++++++++++++++++++ .../ui/rewind_instead_of_seek_to_start.stderr | 16 ++++ 9 files changed, 311 insertions(+) create mode 100644 clippy_lints/src/methods/rewind_instead_of_seek_to_start.rs create mode 100644 src/docs/rewind_instead_of_seek_to_start.txt create mode 100644 tests/ui/rewind_instead_of_seek_to_start.fixed create mode 100644 tests/ui/rewind_instead_of_seek_to_start.rs create mode 100644 tests/ui/rewind_instead_of_seek_to_start.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5a4d4a5..09f914fe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4195,6 +4195,7 @@ Released 2018-09-13 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used [`return_self_not_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges +[`rewind_instead_of_seek_to_start`]: https://rust-lang.github.io/rust-clippy/master/index.html#rewind_instead_of_seek_to_start [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index c6ae0bddc..ad482cfc2 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -361,6 +361,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, + crate::methods::REWIND_INSTEAD_OF_SEEK_TO_START_INFO, crate::methods::SEARCH_IS_SOME_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 fb92779be..e794af7d6 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -68,6 +68,7 @@ mod or_then_unwrap; mod path_buf_push_overwrite; mod range_zip_with_len; mod repeat_once; +mod rewind_instead_of_seek_to_start; mod search_is_some; mod single_char_add_str; mod single_char_insert_string; @@ -3066,6 +3067,37 @@ declare_clippy_lint! { "iterating on map using `iter` when `keys` or `values` would do" } +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for jumps to the start of a stream that implements `Seek` + /// and uses the `seek` method providing `Start` as parameter. + /// + /// ### Why is this bad? + /// + /// Readability. There is a specific method that was implemented for + /// this exact scenario. + /// + /// ### Example + /// ```rust + /// # use std::io; + /// fn foo(t: &mut T) { + /// t.seek(io::SeekFrom::Start(0)); + /// } + /// ``` + /// Use instead: + /// ```rust + /// # use std::io; + /// fn foo(t: &mut T) { + /// t.rewind(); + /// } + /// ``` + #[clippy::version = "1.66.0"] + pub REWIND_INSTEAD_OF_SEEK_TO_START, + complexity, + "jumping to the start of stream using `seek` method" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -3190,6 +3222,7 @@ impl_lint_pass!(Methods => [ VEC_RESIZE_TO_ZERO, VERBOSE_FILE_READS, ITER_KV_MAP, + REWIND_INSTEAD_OF_SEEK_TO_START, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3604,6 +3637,9 @@ impl Methods { ("resize", [count_arg, default_arg]) => { vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span); }, + ("seek", [arg]) => { + rewind_instead_of_seek_to_start::check(cx, expr, recv, arg, span); + }, ("sort", []) => { stable_sort_primitive::check(cx, expr, recv); }, diff --git a/clippy_lints/src/methods/rewind_instead_of_seek_to_start.rs b/clippy_lints/src/methods/rewind_instead_of_seek_to_start.rs new file mode 100644 index 000000000..97b33dec8 --- /dev/null +++ b/clippy_lints/src/methods/rewind_instead_of_seek_to_start.rs @@ -0,0 +1,45 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::implements_trait; +use clippy_utils::{get_trait_def_id, match_def_path, paths}; +use rustc_ast::ast::{LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::Span; + +use super::REWIND_INSTEAD_OF_SEEK_TO_START; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + recv: &'tcx Expr<'_>, + arg: &'tcx Expr<'_>, + name_span: Span, +) { + // Get receiver type + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + + if let Some(seek_trait_id) = get_trait_def_id(cx, &paths::STD_IO_SEEK) && + implements_trait(cx, ty, seek_trait_id, &[]) && + let ExprKind::Call(func, args1) = arg.kind && + let ExprKind::Path(ref path) = func.kind && + let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id() && + match_def_path(cx, def_id, &paths::STD_IO_SEEKFROM_START) && + args1.len() == 1 && + let ExprKind::Lit(ref lit) = args1[0].kind && + let LitKind::Int(0, LitIntType::Unsuffixed) = lit.node + { + let method_call_span = expr.span.with_lo(name_span.lo()); + span_lint_and_then( + cx, + REWIND_INSTEAD_OF_SEEK_TO_START, + method_call_span, + "used `seek` to go to the start of the stream", + |diag| { + let app = Applicability::MachineApplicable; + + diag.span_suggestion(method_call_span, "replace with", "rewind()", app); + }, + ); + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index bc8514734..e37c7e34c 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -115,6 +115,8 @@ pub const STDERR: [&str; 4] = ["std", "io", "stdio", "stderr"]; pub const STDOUT: [&str; 4] = ["std", "io", "stdio", "stdout"]; 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_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"]; pub const STRING_NEW: [&str; 4] = ["alloc", "string", "String", "new"]; diff --git a/src/docs/rewind_instead_of_seek_to_start.txt b/src/docs/rewind_instead_of_seek_to_start.txt new file mode 100644 index 000000000..bef11b231 --- /dev/null +++ b/src/docs/rewind_instead_of_seek_to_start.txt @@ -0,0 +1,22 @@ +### What it does + +Checks for jumps to the start of a stream that implements `Seek` +and uses the `seek` method providing `Start` as parameter. + +### Why is this bad? + +Readability. There is a specific method that was implemented for +this exact scenario. + +### Example +``` +fn foo(t: &mut T) { + t.seek(io::SeekFrom::Start(0)); +} +``` +Use instead: +``` +fn foo(t: &mut T) { + t.rewind(); +} +``` \ No newline at end of file diff --git a/tests/ui/rewind_instead_of_seek_to_start.fixed b/tests/ui/rewind_instead_of_seek_to_start.fixed new file mode 100644 index 000000000..037a288b6 --- /dev/null +++ b/tests/ui/rewind_instead_of_seek_to_start.fixed @@ -0,0 +1,94 @@ +// run-rustfix +#![allow(unused)] +#![warn(clippy::rewind_instead_of_seek_to_start)] + +use std::fs::OpenOptions; +use std::io::{Read, Seek, SeekFrom, Write}; + +struct StructWithSeekMethod {} + +impl StructWithSeekMethod { + fn seek(&mut self, from: SeekFrom) {} +} + +trait MySeekTrait { + fn seek(&mut self, from: SeekFrom) {} +} + +struct StructWithSeekTrait {} +impl MySeekTrait for StructWithSeekTrait {} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_method(t: &mut StructWithSeekMethod) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_method_owned_false(mut t: StructWithSeekMethod) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait(t: &mut StructWithSeekTrait) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait_owned(mut t: StructWithSeekTrait) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait_bound(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn seek_to_start(t: &mut T) { + t.rewind(); +} + +// This should trigger clippy warning +fn owned_seek_to_start(mut t: T) { + t.rewind(); +} + +// This should NOT trigger clippy warning because +// it does not seek to start +fn seek_to_5(t: &mut T) { + t.seek(SeekFrom::Start(5)); +} + +// This should NOT trigger clippy warning because +// it does not seek to start +fn seek_to_end(t: &mut T) { + t.seek(SeekFrom::End(0)); +} + +fn main() { + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let mut my_struct_trait = StructWithSeekTrait {}; + seek_to_start_false_trait_bound(&mut my_struct_trait); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + seek_to_5(&mut f); + seek_to_end(&mut f); + seek_to_start(&mut f); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} diff --git a/tests/ui/rewind_instead_of_seek_to_start.rs b/tests/ui/rewind_instead_of_seek_to_start.rs new file mode 100644 index 000000000..262242512 --- /dev/null +++ b/tests/ui/rewind_instead_of_seek_to_start.rs @@ -0,0 +1,94 @@ +// run-rustfix +#![allow(unused)] +#![warn(clippy::rewind_instead_of_seek_to_start)] + +use std::fs::OpenOptions; +use std::io::{Read, Seek, SeekFrom, Write}; + +struct StructWithSeekMethod {} + +impl StructWithSeekMethod { + fn seek(&mut self, from: SeekFrom) {} +} + +trait MySeekTrait { + fn seek(&mut self, from: SeekFrom) {} +} + +struct StructWithSeekTrait {} +impl MySeekTrait for StructWithSeekTrait {} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_method(t: &mut StructWithSeekMethod) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_method_owned_false(mut t: StructWithSeekMethod) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait(t: &mut StructWithSeekTrait) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait_owned(mut t: StructWithSeekTrait) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// StructWithSeekMethod does not implement std::io::Seek; +fn seek_to_start_false_trait_bound(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn seek_to_start(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn owned_seek_to_start(mut t: T) { + t.seek(SeekFrom::Start(0)); +} + +// This should NOT trigger clippy warning because +// it does not seek to start +fn seek_to_5(t: &mut T) { + t.seek(SeekFrom::Start(5)); +} + +// This should NOT trigger clippy warning because +// it does not seek to start +fn seek_to_end(t: &mut T) { + t.seek(SeekFrom::End(0)); +} + +fn main() { + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let mut my_struct_trait = StructWithSeekTrait {}; + seek_to_start_false_trait_bound(&mut my_struct_trait); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + seek_to_5(&mut f); + seek_to_end(&mut f); + seek_to_start(&mut f); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} diff --git a/tests/ui/rewind_instead_of_seek_to_start.stderr b/tests/ui/rewind_instead_of_seek_to_start.stderr new file mode 100644 index 000000000..f985471ac --- /dev/null +++ b/tests/ui/rewind_instead_of_seek_to_start.stderr @@ -0,0 +1,16 @@ +error: used `seek` to go to the start of the stream + --> $DIR/rewind_instead_of_seek_to_start.rs:53:7 + | +LL | t.seek(SeekFrom::Start(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` + | + = note: `-D clippy::rewind-instead-of-seek-to-start` implied by `-D warnings` + +error: used `seek` to go to the start of the stream + --> $DIR/rewind_instead_of_seek_to_start.rs:58:7 + | +LL | t.seek(SeekFrom::Start(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` + +error: aborting due to 2 previous errors +