about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-10-26 00:07:16 +0000
committerbors <bors@rust-lang.org>2022-10-26 00:07:16 +0000
commit7182a6ba0d14a1b968ae81ef44b61dc718bdb385 (patch)
tree6349fe84051f8f2457bfbe43200ef8b38bd9f5fb
parent634987b49e5401c93421d1c37e75757396b2e5a6 (diff)
parent6efb3a2b9ac9e0477b6b222c051c15c98233b424 (diff)
downloadrust-7182a6ba0d14a1b968ae81ef44b61dc718bdb385.tar.gz
rust-7182a6ba0d14a1b968ae81ef44b61dc718bdb385.zip
Auto merge of #9681 - koka831:feat/add-seek-from-current-lint, r=giraffate
feat: add new lint `seek_from_current`

changelog: `seek_from_current`: new lint to suggest using `stream_position` instead of seek from current position with `SeekFrom::Current(0)`

addresses https://github.com/rust-lang/rust-clippy/issues/7886.

This PR is related to https://github.com/rust-lang/rust-clippy/pull/9667, so I will update `methods/mod.rs` if it get conflicted.
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs48
-rw-r--r--clippy_lints/src/methods/seek_from_current.rs48
-rw-r--r--clippy_utils/src/msrvs.rs2
-rw-r--r--clippy_utils/src/paths.rs1
-rw-r--r--tests/ui/seek_from_current.fixed26
-rw-r--r--tests/ui/seek_from_current.rs26
-rw-r--r--tests/ui/seek_from_current.stderr10
9 files changed, 162 insertions, 1 deletions
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 @@ 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_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 path_buf_push_overwrite;
 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;
@@ -3070,6 +3071,49 @@ declare_clippy_lint! {
 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
+    ///
     /// Checks for jumps to the start of a stream that implements `Seek`
     /// and uses the `seek` method providing `Start` as parameter.
     ///
@@ -3222,6 +3266,7 @@ impl_lint_pass!(Methods => [
     VEC_RESIZE_TO_ZERO,
     VERBOSE_FILE_READS,
     ITER_KV_MAP,
+    SEEK_FROM_CURRENT,
     SEEK_TO_START_INSTEAD_OF_REWIND,
 ]);
 
@@ -3638,6 +3683,9 @@ impl Methods {
                     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 @@ 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 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_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
+