Add new lint rewind_instead_of_seek_to_start

Signed-off-by: Doru-Florin Blanzeanu <[email protected]>

Author: dblnz

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

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,

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: 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 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<RustcVersion>,
@@ -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);
                 },

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);
+            },
+        );
+    }
+}

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"];

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: io::Seek>(t: &mut T) {
+    t.seek(io::SeekFrom::Start(0));
+}
+```
+Use instead:
+```
+fn foo<T: io::Seek>(t: &mut T) {
+    t.rewind();
+}
+```

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<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);
+}

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<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);
+}

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
+