diff options
| author | bors <bors@rust-lang.org> | 2022-10-25 00:14:59 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-10-25 00:14:59 +0000 |
| commit | 039af9c9e758d010dad7610fe543ffcc2b1bdd8a (patch) | |
| tree | 3cad38bc5015ed99c4d17c3f5ca79b7e1495d31e | |
| parent | b698a151b3eb1c6cd6a70d3e9d9d0aab2711f672 (diff) | |
| parent | b9b9d6a751ca3a9ccdd64f50390361e339f5124f (diff) | |
| download | rust-039af9c9e758d010dad7610fe543ffcc2b1bdd8a.tar.gz rust-039af9c9e758d010dad7610fe543ffcc2b1bdd8a.zip | |
Auto merge of #9667 - dorublanzeanu:master, r=giraffate
add new lint `seek_to_start_instead_of_rewind ` changelog: `seek_to_start_instead_of_rewind`: new lint to suggest using `rewind` instead of `seek` to start Resolve #8600
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 38 | ||||
| -rw-r--r-- | clippy_lints/src/methods/seek_to_start_instead_of_rewind.rs | 45 | ||||
| -rw-r--r-- | clippy_utils/src/msrvs.rs | 1 | ||||
| -rw-r--r-- | clippy_utils/src/paths.rs | 2 | ||||
| -rw-r--r-- | src/docs/seek_to_start_instead_of_rewind.txt | 22 | ||||
| -rw-r--r-- | tests/ui/seek_to_start_instead_of_rewind.fixed | 137 | ||||
| -rw-r--r-- | tests/ui/seek_to_start_instead_of_rewind.rs | 137 | ||||
| -rw-r--r-- | tests/ui/seek_to_start_instead_of_rewind.stderr | 22 |
10 files changed, 406 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a1d194794..7e91365c69a 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_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 [`self_named_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_module_files diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 2bb8dfee152..a903d46b2a4 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -363,6 +363,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, crate::methods::SEARCH_IS_SOME_INFO, + crate::methods::SEEK_TO_START_INSTEAD_OF_REWIND_INFO, crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO, crate::methods::SINGLE_CHAR_ADD_STR_INFO, crate::methods::SINGLE_CHAR_PATTERN_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index fb92779be2a..4fd1e3e54ae 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -69,6 +69,7 @@ mod path_buf_push_overwrite; mod range_zip_with_len; mod repeat_once; mod search_is_some; +mod seek_to_start_instead_of_rewind; mod single_char_add_str; mod single_char_insert_string; mod single_char_pattern; @@ -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: io::Seek>(t: &mut T) { + /// t.seek(io::SeekFrom::Start(0)); + /// } + /// ``` + /// Use instead: + /// ```rust + /// # use std::io; + /// fn foo<T: io::Seek>(t: &mut T) { + /// t.rewind(); + /// } + /// ``` + #[clippy::version = "1.66.0"] + pub SEEK_TO_START_INSTEAD_OF_REWIND, + complexity, + "jumping to the start of stream using `seek` method" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option<RustcVersion>, @@ -3190,6 +3222,7 @@ impl_lint_pass!(Methods => [ VEC_RESIZE_TO_ZERO, VERBOSE_FILE_READS, ITER_KV_MAP, + SEEK_TO_START_INSTEAD_OF_REWIND, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -3604,6 +3637,11 @@ impl Methods { ("resize", [count_arg, default_arg]) => { vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span); }, + ("seek", [arg]) => { + if meets_msrv(self.msrv, msrvs::SEEK_REWIND) { + seek_to_start_instead_of_rewind::check(cx, expr, recv, arg, span); + } + }, ("sort", []) => { stable_sort_primitive::check(cx, expr, recv); }, diff --git a/clippy_lints/src/methods/seek_to_start_instead_of_rewind.rs b/clippy_lints/src/methods/seek_to_start_instead_of_rewind.rs new file mode 100644 index 00000000000..7e3bed1e41a --- /dev/null +++ b/clippy_lints/src/methods/seek_to_start_instead_of_rewind.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::SEEK_TO_START_INSTEAD_OF_REWIND; + +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, + SEEK_TO_START_INSTEAD_OF_REWIND, + 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/msrvs.rs b/clippy_utils/src/msrvs.rs index 9780794fa99..04b504d044d 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -38,4 +38,5 @@ msrv_aliases! { 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } 1,16,0 { STR_REPEAT } + 1,55,0 { SEEK_REWIND } } diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index bc851473430..e37c7e34c0c 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/seek_to_start_instead_of_rewind.txt b/src/docs/seek_to_start_instead_of_rewind.txt new file mode 100644 index 00000000000..bef11b2312a --- /dev/null +++ b/src/docs/seek_to_start_instead_of_rewind.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: io::Seek>(t: &mut T) { + t.seek(io::SeekFrom::Start(0)); +} +``` +Use instead: +``` +fn foo<T: io::Seek>(t: &mut T) { + t.rewind(); +} +``` \ No newline at end of file diff --git a/tests/ui/seek_to_start_instead_of_rewind.fixed b/tests/ui/seek_to_start_instead_of_rewind.fixed new file mode 100644 index 00000000000..464b6cdef63 --- /dev/null +++ b/tests/ui/seek_to_start_instead_of_rewind.fixed @@ -0,0 +1,137 @@ +// run-rustfix +#![allow(unused)] +#![feature(custom_inner_attributes)] +#![warn(clippy::seek_to_start_instead_of_rewind)] + +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<T>(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<T>(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: MySeekTrait>(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn seek_to_start<T: Seek>(t: &mut T) { + t.rewind(); +} + +// This should trigger clippy warning +fn owned_seek_to_start<T: Seek>(mut t: T) { + t.rewind(); +} + +// This should NOT trigger clippy warning because +// it does not seek to start +fn seek_to_5<T: Seek>(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: Seek>(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); +} + +fn msrv_1_54() { + #![clippy::msrv = "1.54"] + + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + + f.seek(SeekFrom::Start(0)); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} + +fn msrv_1_55() { + #![clippy::msrv = "1.55"] + + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + + f.rewind(); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} diff --git a/tests/ui/seek_to_start_instead_of_rewind.rs b/tests/ui/seek_to_start_instead_of_rewind.rs new file mode 100644 index 00000000000..68e09bd7c1f --- /dev/null +++ b/tests/ui/seek_to_start_instead_of_rewind.rs @@ -0,0 +1,137 @@ +// run-rustfix +#![allow(unused)] +#![feature(custom_inner_attributes)] +#![warn(clippy::seek_to_start_instead_of_rewind)] + +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<T>(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<T>(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: MySeekTrait>(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn seek_to_start<T: Seek>(t: &mut T) { + t.seek(SeekFrom::Start(0)); +} + +// This should trigger clippy warning +fn owned_seek_to_start<T: Seek>(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: Seek>(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: Seek>(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); +} + +fn msrv_1_54() { + #![clippy::msrv = "1.54"] + + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + + f.seek(SeekFrom::Start(0)); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} + +fn msrv_1_55() { + #![clippy::msrv = "1.55"] + + let mut f = OpenOptions::new() + .write(true) + .read(true) + .create(true) + .open("foo.txt") + .unwrap(); + + let hello = "Hello!\n"; + write!(f, "{hello}").unwrap(); + + f.seek(SeekFrom::Start(0)); + + let mut buf = String::new(); + f.read_to_string(&mut buf).unwrap(); + + assert_eq!(&buf, hello); +} diff --git a/tests/ui/seek_to_start_instead_of_rewind.stderr b/tests/ui/seek_to_start_instead_of_rewind.stderr new file mode 100644 index 00000000000..de0eec5d909 --- /dev/null +++ b/tests/ui/seek_to_start_instead_of_rewind.stderr @@ -0,0 +1,22 @@ +error: used `seek` to go to the start of the stream + --> $DIR/seek_to_start_instead_of_rewind.rs:54:7 + | +LL | t.seek(SeekFrom::Start(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` + | + = note: `-D clippy::seek-to-start-instead-of-rewind` implied by `-D warnings` + +error: used `seek` to go to the start of the stream + --> $DIR/seek_to_start_instead_of_rewind.rs:59:7 + | +LL | t.seek(SeekFrom::Start(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` + +error: used `seek` to go to the start of the stream + --> $DIR/seek_to_start_instead_of_rewind.rs:131:7 + | +LL | f.seek(SeekFrom::Start(0)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `rewind()` + +error: aborting due to 3 previous errors + |
