dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Check for AsRef/AsMut arguments in wrong_self_convention 

This fixes #451
This commit is contained in:
Andre Bogus 2017-06-06 19:26:50 +02:00
parent 329ddb98e1
commit 8122d3e8cb
4 changed files with 72 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
clippy_lints/src/methods.rs
View File

@ -10,7 +10,7 @@ use syntax::codemap::Span;
use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, match_path, match_trait_method,
match_type, method_chain_args, return_ty, same_tys, snippet, span_lint, span_lint_and_then,
span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, last_path_segment, single_segment_path,
match_def_path, is_self, is_self_ty, iter_input_pats};
match_def_path, is_self, is_self_ty, iter_input_pats, match_path_old};
use utils::paths;
use utils::sugg;
@ -649,7 +649,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
if name == method_name &&
sig.decl.inputs.len() == n_args &&
out_type.matches(&sig.decl.output) &&
self_kind.matches(first_arg_ty, first_arg, self_ty, false) {
self_kind.matches(first_arg_ty, first_arg, self_ty, false, &sig.generics) {
span_lint(cx, SHOULD_IMPLEMENT_TRAIT, implitem.span, &format!(
"defining a method called `{}` on this type; consider implementing \
the `{}` trait or choosing a less ambiguous name", name, trait_name));
@ -663,7 +663,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
for &(ref conv, self_kinds) in &CONVENTIONS {
if_let_chain! {[
conv.check(&name.as_str()),
!self_kinds.iter().any(|k| k.matches(first_arg_ty, first_arg, self_ty, is_copy)),
!self_kinds.iter().any(|k| k.matches(first_arg_ty, first_arg, self_ty, is_copy, &sig.generics)),
], {
let lint = if item.vis == hir::Visibility::Public {
WRONG_PUB_SELF_CONVENTION
@ -1353,7 +1353,7 @@ enum SelfKind {
}
impl SelfKind {
fn matches(self, ty: &hir::Ty, arg: &hir::Arg, self_ty: &hir::Ty, allow_value_for_ref: bool) -> bool {
fn matches(self, ty: &hir::Ty, arg: &hir::Arg, self_ty: &hir::Ty, allow_value_for_ref: bool, generics: &hir::Generics) -> bool {
// Self types in the HIR are desugared to explicit self types. So it will always be `self:
// SomeType`,
// where SomeType can be `Self` or an explicit impl self type (e.g. `Foo` if the impl is on `Foo`)
@ -1384,7 +1384,12 @@ impl SelfKind {
_ => false,
}
} else {
self == SelfKind::No
match self {
SelfKind::Value => false,
SelfKind::Ref => is_astrait(ty, self_ty, generics, &paths::ASREF_TRAIT),
SelfKind::RefMut => is_astrait(ty, self_ty, generics, &paths::ASMUT_TRAIT),
SelfKind::No => true
}
}
}
@ -1398,6 +1403,45 @@ impl SelfKind {
}
}
fn is_astrait(ty: &hir::Ty, self_ty: &hir::Ty, generics: &hir::Generics, name: &[&str]) -> bool {
single_segment_ty(ty).map_or(false, |seg| {
generics.ty_params.iter().any(|param| {
param.name == seg.name && param.bounds.iter().any(|bound| {
if let hir::TyParamBound::TraitTyParamBound(ref ptr, ..) = *bound {
let path = &ptr.trait_ref.path;
match_path_old(path, name) && path.segments.last().map_or(false, |s| {
if let hir::PathParameters::AngleBracketedParameters(ref data) = s.parameters {
data.types.len() == 1 && (is_self_ty(&data.types[0]) || is_ty(&*data.types[0], self_ty))
} else {
false
}
})
} else {
false
}
})
})
})
}
fn is_ty(ty: &hir::Ty, self_ty: &hir::Ty) -> bool {
match (&ty.node, &self_ty.node) {
(&hir::TyPath(hir::QPath::Resolved(_, ref ty_path)), &hir::TyPath(hir::QPath::Resolved(_, ref self_ty_path))) => {
ty_path.segments.iter().rev().map(|seg| seg.name).zip(
self_ty_path.segments.iter().rev().map(|seg| seg.name)).all(|(l, r)| l == r)
}
_ => false
}
}
fn single_segment_ty(ty: &hir::Ty) -> Option<&hir::PathSegment> {
if let hir::TyPath(ref path) = ty.node {
single_segment_path(path)
} else {
None
}
}
impl Convention {
fn check(&self, other: &str) -> bool {
match *self {

1
clippy_lints/src/utils/paths.rs
View File

@ -1,5 +1,6 @@
//! This module contains paths to types and functions Clippy needs to know about.
pub const ASMUT_TRAIT: [&'static str; 3] = ["core", "convert", "AsMut"];
pub const ASREF_TRAIT: [&'static str; 3] = ["core", "convert", "AsRef"];
pub const BEGIN_PANIC: [&'static str; 3] = ["std", "panicking", "begin_panic"];
pub const BINARY_HEAP: [&'static str; 3] = ["collections", "binary_heap", "BinaryHeap"];

2
clippy_tests/examples/wrong_self_convention.rs
View File

@ -29,6 +29,8 @@ impl Foo {
#[allow(wrong_self_convention)]
pub fn from_cake(self) {}
fn as_x<F: AsRef<Self>>(_: F) { }
fn as_y<F: AsRef<Foo>>(_: F) { }
}
struct Bar;

40
clippy_tests/examples/wrong_self_convention.stderr
View File

@ -15,81 +15,81 @@ error: methods called `from_*` usually take no self; consider choosing a less am
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
--> wrong_self_convention.rs:38:15
--> wrong_self_convention.rs:40:15
|
38 | fn as_i32(self) {}
40 | fn as_i32(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
--> wrong_self_convention.rs:40:17
--> wrong_self_convention.rs:42:17
|
40 | fn into_i32(&self) {}
42 | fn into_i32(&self) {}
| ^^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
--> wrong_self_convention.rs:42:15
--> wrong_self_convention.rs:44:15
|
42 | fn is_i32(self) {}
44 | fn is_i32(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
--> wrong_self_convention.rs:44:15
--> wrong_self_convention.rs:46:15
|
44 | fn to_i32(self) {}
46 | fn to_i32(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
--> wrong_self_convention.rs:46:17
--> wrong_self_convention.rs:48:17
|
46 | fn from_i32(self) {}
48 | fn from_i32(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `as_*` usually take self by reference or self by mutable reference; consider choosing a less ambiguous name
--> wrong_self_convention.rs:48:19
--> wrong_self_convention.rs:50:19
|
48 | pub fn as_i64(self) {}
50 | pub fn as_i64(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
--> wrong_self_convention.rs:49:21
--> wrong_self_convention.rs:51:21
|
49 | pub fn into_i64(&self) {}
51 | pub fn into_i64(&self) {}
| ^^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `is_*` usually take self by reference or no self; consider choosing a less ambiguous name
--> wrong_self_convention.rs:50:19
--> wrong_self_convention.rs:52:19
|
50 | pub fn is_i64(self) {}
52 | pub fn is_i64(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `to_*` usually take self by reference; consider choosing a less ambiguous name
--> wrong_self_convention.rs:51:19
--> wrong_self_convention.rs:53:19
|
51 | pub fn to_i64(self) {}
53 | pub fn to_i64(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`
error: methods called `from_*` usually take no self; consider choosing a less ambiguous name
--> wrong_self_convention.rs:52:21
--> wrong_self_convention.rs:54:21
|
52 | pub fn from_i64(self) {}
54 | pub fn from_i64(self) {}
| ^^^^
|
= note: `-D wrong-self-convention` implied by `-D warnings`