Skip to content

add new lint seek_to_start_instead_of_rewind #9667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4199,6 +4199,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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,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,
Expand Down
38 changes: 38 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
},
Expand Down
45 changes: 45 additions & 0 deletions clippy_lints/src/methods/seek_to_start_instead_of_rewind.rs
Original file line number Diff line number Diff line change
@@ -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);
},
);
}
}
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,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 }
}
2 changes: 2 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
22 changes: 22 additions & 0 deletions src/docs/seek_to_start_instead_of_rewind.txt
Original file line number Diff line number Diff line change
@@ -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();
}
```
137 changes: 137 additions & 0 deletions tests/ui/seek_to_start_instead_of_rewind.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
Loading