Skip to content

Commit

Permalink
Auto merge of #9667 - dorublanzeanu:master, r=giraffate
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Oct 25, 2022
2 parents b698a15 + b9b9d6a commit 039af9c
Show file tree
Hide file tree
Showing 10 changed files with 406 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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 @@ -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,
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 @@ -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 }
}
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

0 comments on commit 039af9c

Please sign in to comment.