diff --git a/CHANGELOG.md b/CHANGELOG.md index dd67bc3cdc18..9dd04af611f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1236,6 +1236,7 @@ Released 2018-09-13 [`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label +[`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self diff --git a/README.md b/README.md index a84ab0e8c252..986d920ac968 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 324 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 325 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/double_comparison.rs b/clippy_lints/src/double_comparison.rs index 48d2fd341f81..a1e4be260b49 100644 --- a/clippy_lints/src/double_comparison.rs +++ b/clippy_lints/src/double_comparison.rs @@ -39,7 +39,7 @@ declare_lint_pass!(DoubleComparisons => [DOUBLE_COMPARISONS]); impl<'a, 'tcx> DoubleComparisons { #[allow(clippy::similar_names)] - fn check_binop(self, cx: &LateContext<'a, 'tcx>, op: BinOpKind, lhs: &'tcx Expr, rhs: &'tcx Expr, span: Span) { + fn check_binop(cx: &LateContext<'a, 'tcx>, op: BinOpKind, lhs: &'tcx Expr, rhs: &'tcx Expr, span: Span) { let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (&lhs.kind, &rhs.kind) { (ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => { (lb.node, llhs, lrhs, rb.node, rlhs, rrhs) @@ -89,7 +89,7 @@ impl<'a, 'tcx> DoubleComparisons { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DoubleComparisons { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = expr.kind { - self.check_binop(cx, kind.node, lhs, rhs, expr.span); + Self::check_binop(cx, kind.node, lhs, rhs, expr.span); } } } diff --git a/clippy_lints/src/enum_glob_use.rs b/clippy_lints/src/enum_glob_use.rs index bb71bb54472d..64f340dffa13 100644 --- a/clippy_lints/src/enum_glob_use.rs +++ b/clippy_lints/src/enum_glob_use.rs @@ -32,20 +32,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EnumGlobUse { let map = cx.tcx.hir(); // only check top level `use` statements for item in &m.item_ids { - self.lint_item(cx, map.expect_item(item.id)); + lint_item(cx, map.expect_item(item.id)); } } } -impl EnumGlobUse { - fn lint_item(self, cx: &LateContext<'_, '_>, item: &Item) { - if item.vis.node.is_pub() { - return; // re-exports are fine - } - if let ItemKind::Use(ref path, UseKind::Glob) = item.kind { - if let Res::Def(DefKind::Enum, _) = path.res { - span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); - } +fn lint_item(cx: &LateContext<'_, '_>, item: &Item) { + if item.vis.node.is_pub() { + return; // re-exports are fine + } + if let ItemKind::Use(ref path, UseKind::Glob) = item.kind { + if let Res::Def(DefKind::Enum, _) = path.res { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); } } } diff --git a/clippy_lints/src/excessive_precision.rs b/clippy_lints/src/excessive_precision.rs index 763770c74efd..fcc247974c57 100644 --- a/clippy_lints/src/excessive_precision.rs +++ b/clippy_lints/src/excessive_precision.rs @@ -44,7 +44,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision { if let ty::Float(fty) = ty.kind; if let hir::ExprKind::Lit(ref lit) = expr.kind; if let LitKind::Float(sym, _) | LitKind::FloatUnsuffixed(sym) = lit.node; - if let Some(sugg) = self.check(sym, fty); + if let Some(sugg) = Self::check(sym, fty); then { span_lint_and_sugg( cx, @@ -63,7 +63,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision { impl ExcessivePrecision { // None if nothing to lint, Some(suggestion) if lint necessary #[must_use] - fn check(self, sym: Symbol, fty: FloatTy) -> Option { + fn check(sym: Symbol, fty: FloatTy) -> Option { let max = max_digits(fty); let sym_str = sym.as_str(); if dot_zero_exclusion(&sym_str) { diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 6052f9361091..5b2619aa9ba7 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -222,7 +222,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { } } - self.check_raw_ptr(cx, unsafety, decl, body, hir_id); + Self::check_raw_ptr(cx, unsafety, decl, body, hir_id); self.check_line_number(cx, span, body); } @@ -282,7 +282,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions { } if let hir::TraitMethod::Provided(eid) = *eid { let body = cx.tcx.hir().body(eid); - self.check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id); + Self::check_raw_ptr(cx, sig.header.unsafety, &sig.decl, body, item.hir_id); if attr.is_none() && cx.access_levels.is_exported(item.hir_id) { check_must_use_candidate( @@ -368,7 +368,6 @@ impl<'a, 'tcx> Functions { } fn check_raw_ptr( - self, cx: &LateContext<'a, 'tcx>, unsafety: hir::Unsafety, decl: &'tcx hir::FnDecl, diff --git a/clippy_lints/src/int_plus_one.rs b/clippy_lints/src/int_plus_one.rs index 4d588f253e89..93e09315f86c 100644 --- a/clippy_lints/src/int_plus_one.rs +++ b/clippy_lints/src/int_plus_one.rs @@ -53,25 +53,25 @@ enum Side { impl IntPlusOne { #[allow(clippy::cast_sign_loss)] - fn check_lit(self, lit: &Lit, target_value: i128) -> bool { + fn check_lit(lit: &Lit, target_value: i128) -> bool { if let LitKind::Int(value, ..) = lit.kind { return value == (target_value as u128); } false } - fn check_binop(self, cx: &EarlyContext<'_>, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option { + fn check_binop(cx: &EarlyContext<'_>, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option { match (binop, &lhs.kind, &rhs.kind) { // case where `x - 1 >= ...` or `-1 + x >= ...` (BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => { match (lhskind.node, &lhslhs.kind, &lhsrhs.kind) { // `-1 + x` - (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => { - self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) + (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => { + Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) }, // `x - 1` - (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) + (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) }, _ => None, } @@ -82,11 +82,11 @@ impl IntPlusOne { { match (&rhslhs.kind, &rhsrhs.kind) { // `y + 1` and `1 + y` - (&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) + (&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) }, - (_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) + (_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) }, _ => None, } @@ -97,11 +97,11 @@ impl IntPlusOne { { match (&lhslhs.kind, &lhsrhs.kind) { // `1 + x` and `x + 1` - (&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) + (&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) }, - (_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) + (_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) }, _ => None, } @@ -110,12 +110,12 @@ impl IntPlusOne { (BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => { match (rhskind.node, &rhslhs.kind, &rhsrhs.kind) { // `-1 + y` - (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => { - self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) + (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => { + Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) }, // `y - 1` - (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { - self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) + (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => { + Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) }, _ => None, } @@ -125,7 +125,6 @@ impl IntPlusOne { } fn generate_recommendation( - self, cx: &EarlyContext<'_>, binop: BinOpKind, node: &Expr, @@ -149,7 +148,7 @@ impl IntPlusOne { None } - fn emit_warning(self, cx: &EarlyContext<'_>, block: &Expr, recommendation: String) { + fn emit_warning(cx: &EarlyContext<'_>, block: &Expr, recommendation: String) { span_lint_and_then( cx, INT_PLUS_ONE, @@ -170,8 +169,8 @@ impl IntPlusOne { impl EarlyLintPass for IntPlusOne { fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) { if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = item.kind { - if let Some(ref rec) = self.check_binop(cx, kind.node, lhs, rhs) { - self.emit_warning(cx, item, rec.clone()); + if let Some(ref rec) = Self::check_binop(cx, kind.node, lhs, rhs) { + Self::emit_warning(cx, item, rec.clone()); } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5d428221e699..e8962c6e2071 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -277,6 +277,7 @@ pub mod unicode; pub mod unsafe_removed_from_name; pub mod unused_io_amount; pub mod unused_label; +pub mod unused_self; pub mod unwrap; pub mod use_self; pub mod vec; @@ -606,6 +607,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con reg.register_late_lint_pass(box trait_bounds::TraitBounds); reg.register_late_lint_pass(box comparison_chain::ComparisonChain); reg.register_late_lint_pass(box mul_add::MulAddCheck); + reg.register_late_lint_pass(box unused_self::UnusedSelf); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -683,6 +685,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con types::LINKEDLIST, unicode::NON_ASCII_LITERAL, unicode::UNICODE_NOT_NFC, + unused_self::UNUSED_SELF, use_self::USE_SELF, ]); diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 4b2bb69fa794..badd2237073c 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -350,13 +350,13 @@ impl EarlyLintPass for LiteralDigitGrouping { } if let ExprKind::Lit(ref lit) = expr.kind { - self.check_lit(cx, lit) + Self::check_lit(cx, lit) } } } impl LiteralDigitGrouping { - fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { + fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) { let in_macro = in_macro(lit.span); match lit.kind { LitKind::Int(..) => { diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index c06eec950281..2f43daf4caf7 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -437,7 +437,7 @@ impl EarlyLintPass for MiscEarlyLints { ); } }, - ExprKind::Lit(ref lit) => self.check_lit(cx, lit), + ExprKind::Lit(ref lit) => Self::check_lit(cx, lit), _ => (), } } @@ -469,7 +469,7 @@ impl EarlyLintPass for MiscEarlyLints { } impl MiscEarlyLints { - fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { + fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) { // We test if first character in snippet is a number, because the snippet could be an expansion // from a built-in macro like `line!()` or a proc-macro like `#[wasm_bindgen]`. // Note that this check also covers special case that `line!()` is eagerly expanded by compiler. diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index a53235012077..5ed95a674b73 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -117,7 +117,7 @@ impl Return { ast::ExprKind::Ret(ref inner) => { // allow `#[cfg(a)] return a; #[cfg(b)] return b;` if !expr.attrs.iter().any(attr_is_cfg) { - self.emit_return_lint( + Self::emit_return_lint( cx, span.expect("`else return` is not possible"), inner.as_ref().map(|i| i.span), @@ -146,13 +146,7 @@ impl Return { } } - fn emit_return_lint( - &mut self, - cx: &EarlyContext<'_>, - ret_span: Span, - inner_span: Option, - replacement: RetReplacement, - ) { + fn emit_return_lint(cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option, replacement: RetReplacement) { match inner_span { Some(inner_span) => { if in_external_macro(cx.sess(), inner_span) || inner_span.from_expansion() { @@ -191,7 +185,7 @@ impl Return { } // Check for "let x = EXPR; x" - fn check_let_return(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { + fn check_let_return(cx: &EarlyContext<'_>, block: &ast::Block) { let mut it = block.stmts.iter(); // we need both a let-binding stmt and an expr @@ -275,7 +269,7 @@ impl EarlyLintPass for Return { } fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) { - self.check_let_return(cx, block); + Self::check_let_return(cx, block); if_chain! { if let Some(ref stmt) = block.stmts.last(); if let ast::StmtKind::Expr(ref expr) = stmt.kind; diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 3b9985a42451..41d8980e7463 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -244,7 +244,7 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> { // Check that take is applied to `repeat(0)` if let Some(ref repeat_expr) = take_args.get(0); - if self.is_repeat_zero(repeat_expr); + if Self::is_repeat_zero(repeat_expr); // Check that len expression is equals to `with_capacity` expression if let Some(ref len_arg) = take_args.get(1); @@ -259,7 +259,7 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> { } /// Returns `true` if given expression is `repeat(0)` - fn is_repeat_zero(&self, expr: &Expr) -> bool { + fn is_repeat_zero(expr: &Expr) -> bool { if_chain! { if let ExprKind::Call(ref fn_expr, ref repeat_args) = expr.kind; if let ExprKind::Path(ref qpath_repeat) = fn_expr.kind; diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs new file mode 100644 index 000000000000..06b11b6c38c1 --- /dev/null +++ b/clippy_lints/src/unused_self.rs @@ -0,0 +1,102 @@ +use if_chain::if_chain; +use rustc::hir::def::Res; +use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor}; +use rustc::hir::{AssocItemKind, HirId, ImplItemKind, ImplItemRef, Item, ItemKind, Path}; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::{declare_lint_pass, declare_tool_lint}; + +use crate::utils::span_help_and_lint; + +declare_clippy_lint! { + /// **What it does:** Checks methods that contain a `self` argument but don't use it + /// + /// **Why is this bad?** It may be clearer to define the method as an associated function instead + /// of an instance method if it doesn't require `self`. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// struct A; + /// impl A { + /// fn method(&self) {} + /// } + /// ``` + /// + /// Could be written: + /// + /// ```rust,ignore + /// struct A; + /// impl A { + /// fn method() {} + /// } + /// ``` + pub UNUSED_SELF, + pedantic, + "methods that contain a `self` argument but don't use it" +} + +declare_lint_pass!(UnusedSelf => [UNUSED_SELF]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &Item) { + if item.span.from_expansion() { + return; + } + if let ItemKind::Impl(_, _, _, _, None, _, ref impl_item_refs) = item.kind { + for impl_item_ref in impl_item_refs { + if_chain! { + if let ImplItemRef { + kind: AssocItemKind::Method { has_self: true }, + .. + } = impl_item_ref; + let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id); + if let ImplItemKind::Method(_, body_id) = &impl_item.kind; + then { + let body = cx.tcx.hir().body(*body_id); + let self_param = &body.params[0]; + let self_hir_id = self_param.pat.hir_id; + let mut visitor = UnusedSelfVisitor { + cx, + uses_self: false, + self_hir_id: &self_hir_id, + }; + visitor.visit_body(body); + if !visitor.uses_self { + span_help_and_lint( + cx, + UNUSED_SELF, + self_param.span, + "unused `self` argument", + "consider refactoring to a associated function", + ) + } + } + } + } + }; + } +} + +struct UnusedSelfVisitor<'a, 'tcx> { + cx: &'a LateContext<'a, 'tcx>, + uses_self: bool, + self_hir_id: &'a HirId, +} + +impl<'a, 'tcx> Visitor<'tcx> for UnusedSelfVisitor<'a, 'tcx> { + fn visit_path(&mut self, path: &'tcx Path, _id: HirId) { + if self.uses_self { + // This function already uses `self` + return; + } + if let Res::Local(hir_id) = &path.res { + self.uses_self = self.self_hir_id == hir_id + } + walk_path(self, path); + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir()) + } +} diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 9bfd1b38ffa0..0e8fcf8fd0fc 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -169,13 +169,13 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { fn eq_generic_arg(&mut self, left: &GenericArg, right: &GenericArg) -> bool { match (left, right) { - (GenericArg::Lifetime(l_lt), GenericArg::Lifetime(r_lt)) => self.eq_lifetime(l_lt, r_lt), + (GenericArg::Lifetime(l_lt), GenericArg::Lifetime(r_lt)) => Self::eq_lifetime(l_lt, r_lt), (GenericArg::Type(l_ty), GenericArg::Type(r_ty)) => self.eq_ty(l_ty, r_ty), _ => false, } } - fn eq_lifetime(&mut self, left: &Lifetime, right: &Lifetime) -> bool { + fn eq_lifetime(left: &Lifetime, right: &Lifetime) -> bool { left.name == right.name } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 66ee41402eab..d9ae1f70d42b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 324] = [ +pub const ALL_LINTS: [Lint; 325] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -2086,6 +2086,13 @@ pub const ALL_LINTS: [Lint; 324] = [ deprecation: None, module: "unused_label", }, + Lint { + name: "unused_self", + group: "pedantic", + desc: "methods that contain a `self` argument but don\'t use it", + deprecation: None, + module: "unused_self", + }, Lint { name: "unused_unit", group: "style", diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index e5a6cba9a253..54a58e0c86a8 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -14,6 +14,7 @@ clippy::use_self, clippy::useless_format, clippy::wrong_self_convention, + clippy::unused_self, unused )] diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 28da35bff3e0..c3dc08be00b9 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name - --> $DIR/methods.rs:37:5 + --> $DIR/methods.rs:38:5 | LL | / pub fn add(self, other: T) -> T { LL | | self @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::should-implement-trait` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/methods.rs:153:5 + --> $DIR/methods.rs:154:5 | LL | / fn new() -> i32 { LL | | 0 @@ -19,7 +19,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:175:13 + --> $DIR/methods.rs:176:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -31,7 +31,7 @@ LL | | .unwrap_or(0); = note: replace `map(|x| x + 1).unwrap_or(0)` with `map_or(0, |x| x + 1)` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:179:13 + --> $DIR/methods.rs:180:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -41,7 +41,7 @@ LL | | ).unwrap_or(0); | |____________________________^ error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:183:13 + --> $DIR/methods.rs:184:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -51,7 +51,7 @@ LL | | }); | |__________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:188:13 + --> $DIR/methods.rs:189:13 | LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:190:13 + --> $DIR/methods.rs:191:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -69,7 +69,7 @@ LL | | ).unwrap_or(None); | |_____________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:194:13 + --> $DIR/methods.rs:195:13 | LL | let _ = opt | _____________^ @@ -80,7 +80,7 @@ LL | | .unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:205:13 + --> $DIR/methods.rs:206:13 | LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); = note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:209:13 + --> $DIR/methods.rs:210:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -100,7 +100,7 @@ LL | | .unwrap_or_else(|| 0); = note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:213:13 + --> $DIR/methods.rs:214:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -110,7 +110,7 @@ LL | | ).unwrap_or_else(|| 0); | |____________________________________^ error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:217:13 + --> $DIR/methods.rs:218:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -120,7 +120,7 @@ LL | | ); | |_________________^ error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:247:13 + --> $DIR/methods.rs:248:13 | LL | let _ = v.iter().filter(|&x| *x < 0).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -129,7 +129,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next(); = note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:250:13 + --> $DIR/methods.rs:251:13 | LL | let _ = v.iter().filter(|&x| { | _____________^ @@ -139,7 +139,7 @@ LL | | ).next(); | |___________________________^ error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:267:22 + --> $DIR/methods.rs:268:22 | LL | let _ = v.iter().find(|&x| *x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` @@ -147,25 +147,25 @@ LL | let _ = v.iter().find(|&x| *x < 0).is_some(); = note: `-D clippy::search-is-some` implied by `-D warnings` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:268:20 + --> $DIR/methods.rs:269:20 | LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:269:20 + --> $DIR/methods.rs:270:20 | LL | let _ = (0..1).find(|x| *x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:270:22 + --> $DIR/methods.rs:271:22 | LL | let _ = v.iter().find(|x| **x == 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:273:13 + --> $DIR/methods.rs:274:13 | LL | let _ = v.iter().find(|&x| { | _____________^ @@ -175,13 +175,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:279:22 + --> $DIR/methods.rs:280:22 | LL | let _ = v.iter().position(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:282:13 + --> $DIR/methods.rs:283:13 | LL | let _ = v.iter().position(|&x| { | _____________^ @@ -191,13 +191,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:288:22 + --> $DIR/methods.rs:289:22 | LL | let _ = v.iter().rposition(|&x| x < 0).is_some(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:291:13 + --> $DIR/methods.rs:292:13 | LL | let _ = v.iter().rposition(|&x| { | _____________^ @@ -207,7 +207,7 @@ LL | | ).is_some(); | |______________________________^ error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message - --> $DIR/methods.rs:306:13 + --> $DIR/methods.rs:307:13 | LL | let _ = opt.unwrap(); | ^^^^^^^^^^^^ diff --git a/tests/ui/unused_self.rs b/tests/ui/unused_self.rs new file mode 100644 index 000000000000..9119c43c082c --- /dev/null +++ b/tests/ui/unused_self.rs @@ -0,0 +1,111 @@ +#![warn(clippy::unused_self)] +#![allow(clippy::boxed_local)] + +mod unused_self { + use std::pin::Pin; + use std::sync::{Arc, Mutex}; + + struct A {} + + impl A { + fn unused_self_move(self) {} + fn unused_self_ref(&self) {} + fn unused_self_mut_ref(&mut self) {} + fn unused_self_pin_ref(self: Pin<&Self>) {} + fn unused_self_pin_mut_ref(self: Pin<&mut Self>) {} + fn unused_self_pin_nested(self: Pin>) {} + fn unused_self_box(self: Box) {} + fn unused_with_other_used_args(&self, x: u8, y: u8) -> u8 { + x + y + } + fn unused_self_class_method(&self) { + Self::static_method(); + } + + fn static_method() {} + } +} + +mod used_self { + use std::pin::Pin; + + struct A { + x: u8, + } + + impl A { + fn used_self_move(self) -> u8 { + self.x + } + fn used_self_ref(&self) -> u8 { + self.x + } + fn used_self_mut_ref(&mut self) { + self.x += 1 + } + fn used_self_pin_ref(self: Pin<&Self>) -> u8 { + self.x + } + fn used_self_box(self: Box) -> u8 { + self.x + } + fn used_self_with_other_unused_args(&self, x: u8, y: u8) -> u8 { + self.x + } + fn used_in_nested_closure(&self) -> u8 { + let mut a = || -> u8 { self.x }; + a() + } + + #[allow(clippy::collapsible_if)] + fn used_self_method_nested_conditions(&self, a: bool, b: bool, c: bool, d: bool) { + if a { + if b { + if c { + if d { + self.used_self_ref(); + } + } + } + } + } + + fn foo(&self) -> u32 { + let mut sum = 0u32; + for i in 0..self.x { + sum += i as u32; + } + sum + } + + fn bar(&mut self, x: u8) -> u32 { + let mut y = 0u32; + for i in 0..x { + y += self.foo() + } + y + } + } +} + +mod not_applicable { + use std::fmt; + + struct A {} + + impl fmt::Debug for A { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "A") + } + } + + impl A { + fn method(x: u8, y: u8) {} + } + + trait B { + fn method(&self) {} + } +} + +fn main() {} diff --git a/tests/ui/unused_self.stderr b/tests/ui/unused_self.stderr new file mode 100644 index 000000000000..0534b40eabb7 --- /dev/null +++ b/tests/ui/unused_self.stderr @@ -0,0 +1,75 @@ +error: unused `self` argument + --> $DIR/unused_self.rs:11:29 + | +LL | fn unused_self_move(self) {} + | ^^^^ + | + = note: `-D clippy::unused-self` implied by `-D warnings` + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:12:28 + | +LL | fn unused_self_ref(&self) {} + | ^^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:13:32 + | +LL | fn unused_self_mut_ref(&mut self) {} + | ^^^^^^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:14:32 + | +LL | fn unused_self_pin_ref(self: Pin<&Self>) {} + | ^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:15:36 + | +LL | fn unused_self_pin_mut_ref(self: Pin<&mut Self>) {} + | ^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:16:35 + | +LL | fn unused_self_pin_nested(self: Pin>) {} + | ^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:17:28 + | +LL | fn unused_self_box(self: Box) {} + | ^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:18:40 + | +LL | fn unused_with_other_used_args(&self, x: u8, y: u8) -> u8 { + | ^^^^^ + | + = help: consider refactoring to a associated function + +error: unused `self` argument + --> $DIR/unused_self.rs:21:37 + | +LL | fn unused_self_class_method(&self) { + | ^^^^^ + | + = help: consider refactoring to a associated function + +error: aborting due to 9 previous errors +