]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/methods/mod.rs
Auto merge of #9258 - Serial-ATA:unused-peekable, r=Alexendoo
[rust.git] / clippy_lints / src / methods / mod.rs
index 545cf7918af097e59ba49c1de8511e99ea36e4e6..1cfe8c4191efab8bae143d30f57d090e6ab92e17 100644 (file)
 mod or_fun_call;
 mod or_then_unwrap;
 mod path_buf_push_overwrite;
+mod range_zip_with_len;
+mod repeat_once;
 mod search_is_some;
 mod single_char_add_str;
 mod single_char_insert_string;
 mod single_char_pattern;
 mod single_char_push_string;
 mod skip_while_next;
+mod stable_sort_primitive;
 mod str_splitn;
 mod string_extend_chars;
 mod suspicious_map;
 mod suspicious_splitn;
 mod uninit_assumed_init;
+mod unit_hash;
 mod unnecessary_filter_map;
 mod unnecessary_fold;
 mod unnecessary_iter_cloned;
 mod unnecessary_join;
 mod unnecessary_lazy_eval;
+mod unnecessary_sort_by;
 mod unnecessary_to_owned;
 mod unwrap_or_else_default;
 mod unwrap_used;
 mod useless_asref;
 mod utils;
+mod vec_resize_to_zero;
+mod verbose_file_reads;
 mod wrong_self_convention;
 mod zst_offset;
 
     "calling `push` with file system root on `PathBuf` can overwrite it"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for zipping a collection with the range of
+    /// `0.._.len()`.
+    ///
+    /// ### Why is this bad?
+    /// The code is better expressed with `.enumerate()`.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let x = vec![1];
+    /// let _ = x.iter().zip(0..x.len());
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// # let x = vec![1];
+    /// let _ = x.iter().enumerate();
+    /// ```
+    #[clippy::version = "pre 1.29.0"]
+    pub RANGE_ZIP_WITH_LEN,
+    complexity,
+    "zipping iterator with a range when `enumerate()` would do"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `.repeat(1)` and suggest the following method for each types.
+    /// - `.to_string()` for `str`
+    /// - `.clone()` for `String`
+    /// - `.to_vec()` for `slice`
+    ///
+    /// The lint will evaluate constant expressions and values as arguments of `.repeat(..)` and emit a message if
+    /// they are equivalent to `1`. (Related discussion in [rust-clippy#7306](https://github.com/rust-lang/rust-clippy/issues/7306))
+    ///
+    /// ### Why is this bad?
+    /// For example, `String.repeat(1)` is equivalent to `.clone()`. If cloning
+    /// the string is the intention behind this, `clone()` should be used.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn main() {
+    ///     let x = String::from("hello world").repeat(1);
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn main() {
+    ///     let x = String::from("hello world").clone();
+    /// }
+    /// ```
+    #[clippy::version = "1.47.0"]
+    pub REPEAT_ONCE,
+    complexity,
+    "using `.repeat(1)` instead of `String.clone()`, `str.to_string()` or `slice.to_vec()` "
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// When sorting primitive values (integers, bools, chars, as well
+    /// as arrays, slices, and tuples of such items), it is typically better to
+    /// use an unstable sort than a stable sort.
+    ///
+    /// ### Why is this bad?
+    /// Typically, using a stable sort consumes more memory and cpu cycles.
+    /// Because values which compare equal are identical, preserving their
+    /// relative order (the guarantee that a stable sort provides) means
+    /// nothing, while the extra costs still apply.
+    ///
+    /// ### Known problems
+    ///
+    /// As pointed out in
+    /// [issue #8241](https://github.com/rust-lang/rust-clippy/issues/8241),
+    /// a stable sort can instead be significantly faster for certain scenarios
+    /// (eg. when a sorted vector is extended with new data and resorted).
+    ///
+    /// For more information and benchmarking results, please refer to the
+    /// issue linked above.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let mut vec = vec![2, 1, 3];
+    /// vec.sort();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut vec = vec![2, 1, 3];
+    /// vec.sort_unstable();
+    /// ```
+    #[clippy::version = "1.47.0"]
+    pub STABLE_SORT_PRIMITIVE,
+    pedantic,
+    "use of sort() when sort_unstable() is equivalent"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects `().hash(_)`.
+    ///
+    /// ### Why is this bad?
+    /// Hashing a unit value doesn't do anything as the implementation of `Hash` for `()` is a no-op.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::hash::Hash;
+    /// # use std::collections::hash_map::DefaultHasher;
+    /// # enum Foo { Empty, WithValue(u8) }
+    /// # use Foo::*;
+    /// # let mut state = DefaultHasher::new();
+    /// # let my_enum = Foo::Empty;
+    /// match my_enum {
+    ///        Empty => ().hash(&mut state),
+    ///        WithValue(x) => x.hash(&mut state),
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::hash::Hash;
+    /// # use std::collections::hash_map::DefaultHasher;
+    /// # enum Foo { Empty, WithValue(u8) }
+    /// # use Foo::*;
+    /// # let mut state = DefaultHasher::new();
+    /// # let my_enum = Foo::Empty;
+    /// match my_enum {
+    ///        Empty => 0_u8.hash(&mut state),
+    ///        WithValue(x) => x.hash(&mut state),
+    /// }
+    /// ```
+    #[clippy::version = "1.58.0"]
+    pub UNIT_HASH,
+    correctness,
+    "hashing a unit value, which does nothing"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects uses of `Vec::sort_by` passing in a closure
+    /// which compares the two arguments, either directly or indirectly.
+    ///
+    /// ### Why is this bad?
+    /// It is more clear to use `Vec::sort_by_key` (or `Vec::sort` if
+    /// possible) than to use `Vec::sort_by` and a more complicated
+    /// closure.
+    ///
+    /// ### Known problems
+    /// If the suggested `Vec::sort_by_key` uses Reverse and it isn't already
+    /// imported by a use statement, then it will need to be added manually.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # struct A;
+    /// # impl A { fn foo(&self) {} }
+    /// # let mut vec: Vec<A> = Vec::new();
+    /// vec.sort_by(|a, b| a.foo().cmp(&b.foo()));
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # struct A;
+    /// # impl A { fn foo(&self) {} }
+    /// # let mut vec: Vec<A> = Vec::new();
+    /// vec.sort_by_key(|a| a.foo());
+    /// ```
+    #[clippy::version = "1.46.0"]
+    pub UNNECESSARY_SORT_BY,
+    complexity,
+    "Use of `Vec::sort_by` when `Vec::sort_by_key` or `Vec::sort` would be clearer"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Finds occurrences of `Vec::resize(0, an_int)`
+    ///
+    /// ### Why is this bad?
+    /// This is probably an argument inversion mistake.
+    ///
+    /// ### Example
+    /// ```rust
+    /// vec!(1, 2, 3, 4, 5).resize(0, 5)
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// vec!(1, 2, 3, 4, 5).clear()
+    /// ```
+    #[clippy::version = "1.46.0"]
+    pub VEC_RESIZE_TO_ZERO,
+    correctness,
+    "emptying a vector with `resize(0, an_int)` instead of `clear()` is probably an argument inversion mistake"
+}
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for use of File::read_to_end and File::read_to_string.
+    ///
+    /// ### Why is this bad?
+    /// `fs::{read, read_to_string}` provide the same functionality when `buf` is empty with fewer imports and no intermediate values.
+    /// See also: [fs::read docs](https://doc.rust-lang.org/std/fs/fn.read.html), [fs::read_to_string docs](https://doc.rust-lang.org/std/fs/fn.read_to_string.html)
+    ///
+    /// ### Example
+    /// ```rust,no_run
+    /// # use std::io::Read;
+    /// # use std::fs::File;
+    /// let mut f = File::open("foo.txt").unwrap();
+    /// let mut bytes = Vec::new();
+    /// f.read_to_end(&mut bytes).unwrap();
+    /// ```
+    /// Can be written more concisely as
+    /// ```rust,no_run
+    /// # use std::fs;
+    /// let mut bytes = fs::read("foo.txt").unwrap();
+    /// ```
+    #[clippy::version = "1.44.0"]
+    pub VERBOSE_FILE_READS,
+    restriction,
+    "use of `File::read_to_end` or `File::read_to_string`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -2848,6 +3072,13 @@ pub fn new(
     MUT_MUTEX_LOCK,
     NONSENSICAL_OPEN_OPTIONS,
     PATH_BUF_PUSH_OVERWRITE,
+    RANGE_ZIP_WITH_LEN,
+    REPEAT_ONCE,
+    STABLE_SORT_PRIMITIVE,
+    UNIT_HASH,
+    UNNECESSARY_SORT_BY,
+    VEC_RESIZE_TO_ZERO,
+    VERBOSE_FILE_READS,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -3157,6 +3388,9 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                     get_last_with_len::check(cx, expr, recv, arg);
                 },
                 ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"),
+                ("hash", [arg]) => {
+                    unit_hash::check(cx, expr, recv, arg);
+                },
                 ("is_file", []) => filetype_is_file::check(cx, expr, recv),
                 ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv),
                 ("is_none", []) => check_is_some_is_none(cx, expr, recv, false),
@@ -3236,6 +3470,27 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 ("push", [arg]) => {
                     path_buf_push_overwrite::check(cx, expr, arg);
                 },
+                ("read_to_end", [_]) => {
+                    verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_END_MSG);
+                },
+                ("read_to_string", [_]) => {
+                    verbose_file_reads::check(cx, expr, recv, verbose_file_reads::READ_TO_STRING_MSG);
+                },
+                ("repeat", [arg]) => {
+                    repeat_once::check(cx, expr, recv, arg);
+                },
+                ("resize", [count_arg, default_arg]) => {
+                    vec_resize_to_zero::check(cx, expr, count_arg, default_arg, span);
+                },
+                ("sort", []) => {
+                    stable_sort_primitive::check(cx, expr, recv);
+                },
+                ("sort_by", [arg]) => {
+                    unnecessary_sort_by::check(cx, expr, recv, arg, false);
+                },
+                ("sort_unstable_by", [arg]) => {
+                    unnecessary_sort_by::check(cx, expr, recv, arg, true);
+                },
                 ("splitn" | "rsplitn", [count_arg, pat_arg]) => {
                     if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) {
                         suspicious_splitn::check(cx, name, expr, recv, count);
@@ -3304,6 +3559,13 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
                 ("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => {
                     no_effect_replace::check(cx, expr, arg1, arg2);
                 },
+                ("zip", [arg]) => {
+                    if let ExprKind::MethodCall(name, [iter_recv], _) = recv.kind
+                        && name.ident.name == sym::iter
+                    {
+                        range_zip_with_len::check(cx, expr, iter_recv, arg);
+                    }
+                },
                 _ => {},
             }
         }