Auto merge of #4619 - james9909:unused-self, r=flip1995

Add a lint for unused self

changelog: Adds a new lint: `unused_self`

Closes #4550.
This commit is contained in:
bors 2019-10-15 08:16:39 +00:00
commit 55e7818a06
19 changed files with 373 additions and 83 deletions

View file

@ -1236,6 +1236,7 @@ Released 2018-09-13
[`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`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_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_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 [`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_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 [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self

View file

@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. 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: We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

View file

@ -39,7 +39,7 @@ declare_lint_pass!(DoubleComparisons => [DOUBLE_COMPARISONS]);
impl<'a, 'tcx> DoubleComparisons { impl<'a, 'tcx> DoubleComparisons {
#[allow(clippy::similar_names)] #[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) { let (lkind, llhs, lrhs, rkind, rlhs, rrhs) = match (&lhs.kind, &rhs.kind) {
(ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => { (ExprKind::Binary(lb, llhs, lrhs), ExprKind::Binary(rb, rlhs, rrhs)) => {
(lb.node, llhs, lrhs, rb.node, 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 { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DoubleComparisons {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = expr.kind { 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);
} }
} }
} }

View file

@ -32,20 +32,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EnumGlobUse {
let map = cx.tcx.hir(); let map = cx.tcx.hir();
// only check top level `use` statements // only check top level `use` statements
for item in &m.item_ids { 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(cx: &LateContext<'_, '_>, item: &Item) {
fn lint_item(self, cx: &LateContext<'_, '_>, item: &Item) { if item.vis.node.is_pub() {
if item.vis.node.is_pub() { return; // re-exports are fine
return; // re-exports are fine }
} if let ItemKind::Use(ref path, UseKind::Glob) = item.kind {
if let ItemKind::Use(ref path, UseKind::Glob) = item.kind { if let Res::Def(DefKind::Enum, _) = path.res {
if let Res::Def(DefKind::Enum, _) = path.res { span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants");
span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants");
}
} }
} }
} }

View file

@ -44,7 +44,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
if let ty::Float(fty) = ty.kind; if let ty::Float(fty) = ty.kind;
if let hir::ExprKind::Lit(ref lit) = expr.kind; if let hir::ExprKind::Lit(ref lit) = expr.kind;
if let LitKind::Float(sym, _) | LitKind::FloatUnsuffixed(sym) = lit.node; 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 { then {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
@ -63,7 +63,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
impl ExcessivePrecision { impl ExcessivePrecision {
// None if nothing to lint, Some(suggestion) if lint necessary // None if nothing to lint, Some(suggestion) if lint necessary
#[must_use] #[must_use]
fn check(self, sym: Symbol, fty: FloatTy) -> Option<String> { fn check(sym: Symbol, fty: FloatTy) -> Option<String> {
let max = max_digits(fty); let max = max_digits(fty);
let sym_str = sym.as_str(); let sym_str = sym.as_str();
if dot_zero_exclusion(&sym_str) { if dot_zero_exclusion(&sym_str) {

View file

@ -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); 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 { if let hir::TraitMethod::Provided(eid) = *eid {
let body = cx.tcx.hir().body(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) { if attr.is_none() && cx.access_levels.is_exported(item.hir_id) {
check_must_use_candidate( check_must_use_candidate(
@ -368,7 +368,6 @@ impl<'a, 'tcx> Functions {
} }
fn check_raw_ptr( fn check_raw_ptr(
self,
cx: &LateContext<'a, 'tcx>, cx: &LateContext<'a, 'tcx>,
unsafety: hir::Unsafety, unsafety: hir::Unsafety,
decl: &'tcx hir::FnDecl, decl: &'tcx hir::FnDecl,

View file

@ -53,25 +53,25 @@ enum Side {
impl IntPlusOne { impl IntPlusOne {
#[allow(clippy::cast_sign_loss)] #[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 { if let LitKind::Int(value, ..) = lit.kind {
return value == (target_value as u128); return value == (target_value as u128);
} }
false false
} }
fn check_binop(self, cx: &EarlyContext<'_>, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<String> { fn check_binop(cx: &EarlyContext<'_>, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<String> {
match (binop, &lhs.kind, &rhs.kind) { match (binop, &lhs.kind, &rhs.kind) {
// case where `x - 1 >= ...` or `-1 + x >= ...` // case where `x - 1 >= ...` or `-1 + x >= ...`
(BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => { (BinOpKind::Ge, &ExprKind::Binary(ref lhskind, ref lhslhs, ref lhsrhs), _) => {
match (lhskind.node, &lhslhs.kind, &lhsrhs.kind) { match (lhskind.node, &lhslhs.kind, &lhsrhs.kind) {
// `-1 + x` // `-1 + x`
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => { (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS)
}, },
// `x - 1` // `x - 1`
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS)
}, },
_ => None, _ => None,
} }
@ -82,11 +82,11 @@ impl IntPlusOne {
{ {
match (&rhslhs.kind, &rhsrhs.kind) { match (&rhslhs.kind, &rhsrhs.kind) {
// `y + 1` and `1 + y` // `y + 1` and `1 + y`
(&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => { (&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS)
}, },
(_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { (_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS)
}, },
_ => None, _ => None,
} }
@ -97,11 +97,11 @@ impl IntPlusOne {
{ {
match (&lhslhs.kind, &lhsrhs.kind) { match (&lhslhs.kind, &lhsrhs.kind) {
// `1 + x` and `x + 1` // `1 + x` and `x + 1`
(&ExprKind::Lit(ref lit), _) if self.check_lit(lit, 1) => { (&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS) Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS)
}, },
(_, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { (_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS) Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS)
}, },
_ => None, _ => None,
} }
@ -110,12 +110,12 @@ impl IntPlusOne {
(BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => { (BinOpKind::Le, _, &ExprKind::Binary(ref rhskind, ref rhslhs, ref rhsrhs)) => {
match (rhskind.node, &rhslhs.kind, &rhsrhs.kind) { match (rhskind.node, &rhslhs.kind, &rhsrhs.kind) {
// `-1 + y` // `-1 + y`
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if self.check_lit(lit, -1) => { (BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
self.generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS) Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS)
}, },
// `y - 1` // `y - 1`
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if self.check_lit(lit, 1) => { (BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
self.generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS) Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS)
}, },
_ => None, _ => None,
} }
@ -125,7 +125,6 @@ impl IntPlusOne {
} }
fn generate_recommendation( fn generate_recommendation(
self,
cx: &EarlyContext<'_>, cx: &EarlyContext<'_>,
binop: BinOpKind, binop: BinOpKind,
node: &Expr, node: &Expr,
@ -149,7 +148,7 @@ impl IntPlusOne {
None None
} }
fn emit_warning(self, cx: &EarlyContext<'_>, block: &Expr, recommendation: String) { fn emit_warning(cx: &EarlyContext<'_>, block: &Expr, recommendation: String) {
span_lint_and_then( span_lint_and_then(
cx, cx,
INT_PLUS_ONE, INT_PLUS_ONE,
@ -170,8 +169,8 @@ impl IntPlusOne {
impl EarlyLintPass for IntPlusOne { impl EarlyLintPass for IntPlusOne {
fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) { fn check_expr(&mut self, cx: &EarlyContext<'_>, item: &Expr) {
if let ExprKind::Binary(ref kind, ref lhs, ref rhs) = item.kind { 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) { if let Some(ref rec) = Self::check_binop(cx, kind.node, lhs, rhs) {
self.emit_warning(cx, item, rec.clone()); Self::emit_warning(cx, item, rec.clone());
} }
} }
} }

View file

@ -277,6 +277,7 @@ pub mod unicode;
pub mod unsafe_removed_from_name; pub mod unsafe_removed_from_name;
pub mod unused_io_amount; pub mod unused_io_amount;
pub mod unused_label; pub mod unused_label;
pub mod unused_self;
pub mod unwrap; pub mod unwrap;
pub mod use_self; pub mod use_self;
pub mod vec; 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 trait_bounds::TraitBounds);
reg.register_late_lint_pass(box comparison_chain::ComparisonChain); reg.register_late_lint_pass(box comparison_chain::ComparisonChain);
reg.register_late_lint_pass(box mul_add::MulAddCheck); 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![ reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
arithmetic::FLOAT_ARITHMETIC, arithmetic::FLOAT_ARITHMETIC,
@ -683,6 +685,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
types::LINKEDLIST, types::LINKEDLIST,
unicode::NON_ASCII_LITERAL, unicode::NON_ASCII_LITERAL,
unicode::UNICODE_NOT_NFC, unicode::UNICODE_NOT_NFC,
unused_self::UNUSED_SELF,
use_self::USE_SELF, use_self::USE_SELF,
]); ]);

View file

@ -350,13 +350,13 @@ impl EarlyLintPass for LiteralDigitGrouping {
} }
if let ExprKind::Lit(ref lit) = expr.kind { if let ExprKind::Lit(ref lit) = expr.kind {
self.check_lit(cx, lit) Self::check_lit(cx, lit)
} }
} }
} }
impl LiteralDigitGrouping { impl LiteralDigitGrouping {
fn check_lit(self, cx: &EarlyContext<'_>, lit: &Lit) { fn check_lit(cx: &EarlyContext<'_>, lit: &Lit) {
let in_macro = in_macro(lit.span); let in_macro = in_macro(lit.span);
match lit.kind { match lit.kind {
LitKind::Int(..) => { LitKind::Int(..) => {

View file

@ -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 { 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 // 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]`. // 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. // Note that this check also covers special case that `line!()` is eagerly expanded by compiler.

View file

@ -117,7 +117,7 @@ impl Return {
ast::ExprKind::Ret(ref inner) => { ast::ExprKind::Ret(ref inner) => {
// allow `#[cfg(a)] return a; #[cfg(b)] return b;` // allow `#[cfg(a)] return a; #[cfg(b)] return b;`
if !expr.attrs.iter().any(attr_is_cfg) { if !expr.attrs.iter().any(attr_is_cfg) {
self.emit_return_lint( Self::emit_return_lint(
cx, cx,
span.expect("`else return` is not possible"), span.expect("`else return` is not possible"),
inner.as_ref().map(|i| i.span), inner.as_ref().map(|i| i.span),
@ -146,13 +146,7 @@ impl Return {
} }
} }
fn emit_return_lint( fn emit_return_lint(cx: &EarlyContext<'_>, ret_span: Span, inner_span: Option<Span>, replacement: RetReplacement) {
&mut self,
cx: &EarlyContext<'_>,
ret_span: Span,
inner_span: Option<Span>,
replacement: RetReplacement,
) {
match inner_span { match inner_span {
Some(inner_span) => { Some(inner_span) => {
if in_external_macro(cx.sess(), inner_span) || inner_span.from_expansion() { if in_external_macro(cx.sess(), inner_span) || inner_span.from_expansion() {
@ -191,7 +185,7 @@ impl Return {
} }
// Check for "let x = EXPR; x" // 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(); let mut it = block.stmts.iter();
// we need both a let-binding stmt and an expr // 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) { 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_chain! {
if let Some(ref stmt) = block.stmts.last(); if let Some(ref stmt) = block.stmts.last();
if let ast::StmtKind::Expr(ref expr) = stmt.kind; if let ast::StmtKind::Expr(ref expr) = stmt.kind;

View file

@ -244,7 +244,7 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> {
// Check that take is applied to `repeat(0)` // Check that take is applied to `repeat(0)`
if let Some(ref repeat_expr) = take_args.get(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 // Check that len expression is equals to `with_capacity` expression
if let Some(ref len_arg) = take_args.get(1); 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)` /// 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_chain! {
if let ExprKind::Call(ref fn_expr, ref repeat_args) = expr.kind; if let ExprKind::Call(ref fn_expr, ref repeat_args) = expr.kind;
if let ExprKind::Path(ref qpath_repeat) = fn_expr.kind; if let ExprKind::Path(ref qpath_repeat) = fn_expr.kind;

View file

@ -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())
}
}

View file

@ -169,13 +169,13 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> {
fn eq_generic_arg(&mut self, left: &GenericArg, right: &GenericArg) -> bool { fn eq_generic_arg(&mut self, left: &GenericArg, right: &GenericArg) -> bool {
match (left, right) { 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), (GenericArg::Type(l_ty), GenericArg::Type(r_ty)) => self.eq_ty(l_ty, r_ty),
_ => false, _ => false,
} }
} }
fn eq_lifetime(&mut self, left: &Lifetime, right: &Lifetime) -> bool { fn eq_lifetime(left: &Lifetime, right: &Lifetime) -> bool {
left.name == right.name left.name == right.name
} }

View file

@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS; pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, its used in `update_lints` // begin lint list, do not remove this comment, its used in `update_lints`
pub const ALL_LINTS: [Lint; 324] = [ pub const ALL_LINTS: [Lint; 325] = [
Lint { Lint {
name: "absurd_extreme_comparisons", name: "absurd_extreme_comparisons",
group: "correctness", group: "correctness",
@ -2086,6 +2086,13 @@ pub const ALL_LINTS: [Lint; 324] = [
deprecation: None, deprecation: None,
module: "unused_label", 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 { Lint {
name: "unused_unit", name: "unused_unit",
group: "style", group: "style",

View file

@ -14,6 +14,7 @@
clippy::use_self, clippy::use_self,
clippy::useless_format, clippy::useless_format,
clippy::wrong_self_convention, clippy::wrong_self_convention,
clippy::unused_self,
unused unused
)] )]

View file

@ -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 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 | / pub fn add(self, other: T) -> T {
LL | | self LL | | self
@ -9,7 +9,7 @@ LL | | }
= note: `-D clippy::should-implement-trait` implied by `-D warnings` = note: `-D clippy::should-implement-trait` implied by `-D warnings`
error: methods called `new` usually return `Self` error: methods called `new` usually return `Self`
--> $DIR/methods.rs:153:5 --> $DIR/methods.rs:154:5
| |
LL | / fn new() -> i32 { LL | / fn new() -> i32 {
LL | | 0 LL | | 0
@ -19,7 +19,7 @@ LL | | }
= note: `-D clippy::new-ret-no-self` implied by `-D warnings` = 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 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) 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)` = 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 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| { 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 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) 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 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); 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))` = 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 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| { 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 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 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))` = 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 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); 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))` = 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 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) 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)` = 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 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| { 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 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) 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. 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(); 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)` = 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. 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| { 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()`. 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(); LL | let _ = v.iter().find(|&x| *x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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` = 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()`. 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 LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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()`. 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(); LL | let _ = (0..1).find(|x| *x == 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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()`. 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(); LL | let _ = v.iter().find(|x| **x == 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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()`. 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| { 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()`. 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(); LL | let _ = v.iter().position(|&x| x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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()`. 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| { 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()`. 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(); LL | let _ = v.iter().rposition(|&x| x < 0).is_some();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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()`. 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| { 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 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(); LL | let _ = opt.unwrap();
| ^^^^^^^^^^^^ | ^^^^^^^^^^^^

111
tests/ui/unused_self.rs Normal file
View file

@ -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<Arc<Self>>) {}
fn unused_self_box(self: Box<Self>) {}
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<Self>) -> 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() {}

View file

@ -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<Arc<Self>>) {}
| ^^^^
|
= help: consider refactoring to a associated function
error: unused `self` argument
--> $DIR/unused_self.rs:17:28
|
LL | fn unused_self_box(self: Box<Self>) {}
| ^^^^
|
= 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