Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 07:00:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

wrong_self_convention: fix lint in case of to_*_mut method

Browse source 
When a method starts with `to_` and ends with `_mut`, it should expect a `&mut self` parameter,
otherwise `&self`.
This commit is contained in:
Mateusz Gacek 2021-03-11 23:47:15 -08:00
parent 6ed6f1e6a1
commit 2547edb842
6 changed files with 189 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
clippy_lints/src/methods/mod.rs
View file

@ -191,13 +191,14 @@ declare_clippy_lint! {
/// **What it does:** Checks for methods with certain name prefixes and which
/// doesn't match how self is taken. The actual rules are:
///
/// |Prefix |`self` taken |
/// |-------|----------------------|
/// |`as_` |`&self` or `&mut self`|
/// |`from_`| none |
/// |`into_`|`self` |
/// |`is_` |`&self` or none |
/// |`to_` |`&self` |
/// |Prefix |Postfix |`self` taken |
/// |-------|------------|----------------------|
/// |`as_` | none |`&self` or `&mut self`|
/// |`from_`| none | none |
/// |`into_`| none |`self` |
/// |`is_` | none |`&self` or none |
/// |`to_` | `_mut` |`&mut &self` |
/// |`to_` | not `_mut` |`&self` |
///
/// **Why is this bad?** Consistency breeds readability. If you follow the
/// conventions, your users won't be surprised that they, e.g., need to supply a

72
clippy_lints/src/methods/wrong_self_convention.rs
View file

@ -1,5 +1,5 @@
use crate::methods::SelfKind;
use crate::utils::span_lint;
use crate::utils::span_lint_and_help;
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
use rustc_span::source_map::Span;
@ -9,18 +9,22 @@ use super::WRONG_PUB_SELF_CONVENTION;
use super::WRONG_SELF_CONVENTION;
#[rustfmt::skip]
const CONVENTIONS: [(Convention, &[SelfKind]); 7] = [
(Convention::Eq("new"), &[SelfKind::No]),
(Convention::StartsWith("as_"), &[SelfKind::Ref, SelfKind::RefMut]),
(Convention::StartsWith("from_"), &[SelfKind::No]),
(Convention::StartsWith("into_"), &[SelfKind::Value]),
(Convention::StartsWith("is_"), &[SelfKind::Ref, SelfKind::No]),
(Convention::Eq("to_mut"), &[SelfKind::RefMut]),
(Convention::StartsWith("to_"), &[SelfKind::Ref]),
const CONVENTIONS: [(&[Convention], &[SelfKind]); 8] = [
(&[Convention::Eq("new")], &[SelfKind::No]),
(&[Convention::StartsWith("as_")], &[SelfKind::Ref, SelfKind::RefMut]),
(&[Convention::StartsWith("from_")], &[SelfKind::No]),
(&[Convention::StartsWith("into_")], &[SelfKind::Value]),
(&[Convention::StartsWith("is_")], &[SelfKind::Ref, SelfKind::No]),
(&[Convention::Eq("to_mut")], &[SelfKind::RefMut]),
(&[Convention::StartsWith("to_"), Convention::EndsWith("_mut")], &[SelfKind::RefMut]),
(&[Convention::StartsWith("to_"), Convention::NotEndsWith("_mut")], &[SelfKind::Ref]),
];
enum Convention {
Eq(&'static str),
StartsWith(&'static str),
EndsWith(&'static str),
NotEndsWith(&'static str),
}
impl Convention {
@ -29,6 +33,8 @@ impl Convention {
match *self {
Self::Eq(this) => this == other,
Self::StartsWith(this) => other.starts_with(this) && this != other,
Self::EndsWith(this) => other.ends_with(this) && this != other,
Self::NotEndsWith(this) => !Self::EndsWith(this).check(other),
}
}
}
@ -38,6 +44,8 @@ impl fmt::Display for Convention {
match *self {
Self::Eq(this) => this.fmt(f),
Self::StartsWith(this) => this.fmt(f).and_then(|_| '*'.fmt(f)),
Self::EndsWith(this) => '*'.fmt(f).and_then(|_| this.fmt(f)),
Self::NotEndsWith(this) => '~'.fmt(f).and_then(|_| this.fmt(f)),
}
}
}
@ -55,21 +63,59 @@ pub(super) fn check<'tcx>(
} else {
WRONG_SELF_CONVENTION
};
if let Some((ref conv, self_kinds)) = &CONVENTIONS.iter().find(|(ref conv, _)| conv.check(item_name)) {
if let Some((conventions, self_kinds)) = &CONVENTIONS
.iter()
.find(|(convs, _)| convs.iter().all(|conv| conv.check(item_name)))
{
if !self_kinds.iter().any(|k| k.matches(cx, self_ty, first_arg_ty)) {
span_lint(
let suggestion = {
if conventions.len() > 1 {
let special_case = {
// Don't mention `NotEndsWith` when there is also `StartsWith` convention present
if conventions.len() == 2 {
match conventions {
[Convention::StartsWith(starts_with), Convention::NotEndsWith(_)]
| [Convention::NotEndsWith(_), Convention::StartsWith(starts_with)] => {
Some(format!("methods called `{}`", Convention::StartsWith(starts_with)))
},
_ => None,
}
} else {
None
}
};
if let Some(suggestion) = special_case {
suggestion
} else {
let s = conventions
.iter()
.map(|c| format!("`{}`", &c.to_string()))
.collect::<Vec<_>>()
.join(" and ");
format!("methods called like this: ({})", &s)
}
} else {
format!("methods called `{}`", &conventions[0])
}
};
span_lint_and_help(
cx,
lint,
first_arg_span,
&format!(
"methods called `{}` usually take {}; consider choosing a less ambiguous name",
conv,
"{} usually take {}",
suggestion,
&self_kinds
.iter()
.map(|k| k.description())
.collect::<Vec<_>>()
.join(" or ")
),
None,
"consider choosing a less ambiguous name",
);
}
}

3
tests/ui/def_id_nocore.stderr
View file

@ -1,10 +1,11 @@
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference
--> $DIR/def_id_nocore.rs:26:19
|
LL | pub fn as_ref(self) -> &'static str {
| ^^^^
|
= note: `-D clippy::wrong-self-convention` implied by `-D warnings`
= help: consider choosing a less ambiguous name
error: aborting due to previous error

95
tests/ui/wrong_self_convention.stderr
View file

@ -1,148 +1,195 @@
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:18:17
|
LL | fn from_i32(self) {}
| ^^^^
|
= note: `-D clippy::wrong-self-convention` implied by `-D warnings`
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:24:21
|
LL | pub fn from_i64(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference
--> $DIR/wrong_self_convention.rs:36:15
|
LL | fn as_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value
--> $DIR/wrong_self_convention.rs:38:17
|
LL | fn into_i32(&self) {}
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self
--> $DIR/wrong_self_convention.rs:40:15
|
LL | fn is_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference
--> $DIR/wrong_self_convention.rs:42:15
|
LL | fn to_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:44:17
|
LL | fn from_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference
--> $DIR/wrong_self_convention.rs:46:19
|
LL | pub fn as_i64(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value
--> $DIR/wrong_self_convention.rs:47:21
|
LL | pub fn into_i64(&self) {}
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self
--> $DIR/wrong_self_convention.rs:48:19
|
LL | pub fn is_i64(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference
--> $DIR/wrong_self_convention.rs:49:19
|
LL | pub fn to_i64(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:50:21
|
LL | pub fn from_i64(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference
--> $DIR/wrong_self_convention.rs:95:19
|
LL | fn as_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value
--> $DIR/wrong_self_convention.rs:98:25
|
LL | fn into_i32_ref(&self) {}
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self
--> $DIR/wrong_self_convention.rs:100:19
|
LL | fn is_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference
--> $DIR/wrong_self_convention.rs:102:19
|
LL | fn to_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:104:21
|
LL | fn from_i32(self) {}
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
error: methods called `as_*` usually take self by reference or self by mutable reference
--> $DIR/wrong_self_convention.rs:119:19
|
LL | fn as_i32(self);
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value
--> $DIR/wrong_self_convention.rs:122:25
|
LL | fn into_i32_ref(&self);
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
error: methods called `is_*` usually take self by reference or no self
--> $DIR/wrong_self_convention.rs:124:19
|
LL | fn is_i32(self);
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
error: methods called `to_*` usually take self by reference
--> $DIR/wrong_self_convention.rs:126:19
|
LL | fn to_i32(self);
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:128:21
|
LL | fn from_i32(self);
| ^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
error: methods called `into_*` usually take self by value
--> $DIR/wrong_self_convention.rs:146:25
|
LL | fn into_i32_ref(&self);
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
error: methods called `from_*` usually take no self
--> $DIR/wrong_self_convention.rs:152:21
|
LL | fn from_i32(self);
| ^^^^
|
= help: consider choosing a less ambiguous name
error: aborting due to 24 previous errors

30
tests/ui/wrong_self_conventions_mut.rs Normal file
View file

@ -0,0 +1,30 @@
// edition:2018
#![warn(clippy::wrong_self_convention)]
#![allow(dead_code)]
fn main() {}
mod issue6758 {
pub enum Test<T> {
One(T),
Many(Vec<T>),
}
impl<T> Test<T> {
// If a method starts with `to_` and not ends with `_mut` it should expect `&self`
pub fn to_many(&mut self) -> Option<&mut [T]> {
match self {
Self::Many(data) => Some(data),
_ => None,
}
}
// If a method starts with `to_` and ends with `_mut` it should expect `&mut self`
pub fn to_many_mut(&self) -> Option<&[T]> {
match self {
Self::Many(data) => Some(data),
_ => None,
}
}
}
}

19
tests/ui/wrong_self_conventions_mut.stderr Normal file
View file

@ -0,0 +1,19 @@
error: methods called `to_*` usually take self by reference
--> $DIR/wrong_self_conventions_mut.rs:15:24
|
LL | pub fn to_many(&mut self) -> Option<&mut [T]> {
| ^^^^^^^^^
|
= note: `-D clippy::wrong-self-convention` implied by `-D warnings`
= help: consider choosing a less ambiguous name
error: methods called like this: (`to_*` and `*_mut`) usually take self by mutable reference
--> $DIR/wrong_self_conventions_mut.rs:23:28
|
LL | pub fn to_many_mut(&self) -> Option<&[T]> {
| ^^^^^
|
= help: consider choosing a less ambiguous name
error: aborting due to 2 previous errors