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

Fix #2894

Browse source
This commit is contained in:
Michael Wright 2018-07-14 12:18:50 +02:00
parent 06d6710147
commit b90fc5edfa
5 changed files with 303 additions and 116 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

117
clippy_lints/src/use_self.rs
View file

@ -1,8 +1,10 @@
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::hir::*;
use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor};
use crate::utils::{in_macro, span_lint_and_then};
use rustc::hir::intravisit::{walk_path, walk_ty, NestedVisitorMap, Visitor};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::ty;
use syntax::ast::NodeId;
use syntax::symbol::keywords;
use syntax_pos::symbol::keywords::SelfType;
/// **What it does:** Checks for unnecessary repetition of structure name when a
@ -49,13 +51,93 @@ impl LintPass for UseSelf {
const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
fn span_use_self_lint(cx: &LateContext, path: &Path) {
span_lint_and_then(cx, USE_SELF, path.span, "unnecessary structure name repetition", |db| {
db.span_suggestion(path.span, "use the applicable keyword", "Self".to_owned());
});
}
struct TraitImplTyVisitor<'a, 'tcx: 'a> {
cx: &'a LateContext<'a, 'tcx>,
type_walker: ty::walk::TypeWalker<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
fn visit_ty(&mut self, t: &'tcx Ty) {
let trait_ty = self.type_walker.next();
if let TyPath(QPath::Resolved(_, path)) = &t.node {
let impl_is_self_ty = if let def::Def::SelfTy(..) = path.def {
true
} else {
false
};
if !impl_is_self_ty {
let trait_is_self_ty = if let Some(ty::TyParam(ty::ParamTy { name, .. })) = trait_ty.map(|ty| &ty.sty) {
*name == keywords::SelfType.name().as_str()
} else {
false
};
if trait_is_self_ty {
span_use_self_lint(self.cx, path);
}
}
}
walk_ty(self, t)
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}
fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
cx: &'a LateContext<'a, 'tcx>,
impl_item: &ImplItem,
impl_decl: &'tcx FnDecl,
impl_trait_ref: &ty::TraitRef,
) {
let trait_method = cx
.tcx
.associated_items(impl_trait_ref.def_id)
.find(|assoc_item| {
assoc_item.kind == ty::AssociatedKind::Method
&& cx
.tcx
.hygienic_eq(impl_item.ident, assoc_item.ident, impl_trait_ref.def_id)
})
.expect("impl method matches a trait method");
let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);
let output_ty = if let FunctionRetTy::Return(ty) = &impl_decl.output {
Some(&**ty)
} else {
None
};
for (impl_ty, trait_ty) in impl_decl
.inputs
.iter()
.chain(output_ty)
.zip(trait_method_sig.inputs_and_output)
{
let mut visitor = TraitImplTyVisitor {
cx,
type_walker: trait_ty.walk(),
};
visitor.visit_ty(&impl_ty);
}
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
if in_macro(item.span) {
return;
}
if_chain! {
if let ItemImpl(.., ref item_type, ref refs) = item.node;
if let ItemImpl(.., item_type, refs) = &item.node;
if let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node;
then {
let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
@ -67,13 +149,32 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
} else {
true
};
if should_check {
let visitor = &mut UseSelfVisitor {
item_path,
cx,
};
for impl_item_ref in refs {
visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id));
let impl_def_id = cx.tcx.hir.local_def_id(item.id);
let impl_trait_ref = cx.tcx.impl_trait_ref(impl_def_id);
if let Some(impl_trait_ref) = impl_trait_ref {
for impl_item_ref in refs {
let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
if let ImplItemKind::Method(MethodSig{ decl: impl_decl, .. }, impl_body_id)
= &impl_item.node {
check_trait_method_impl_decl(cx, impl_item, impl_decl, &impl_trait_ref);
let body = cx.tcx.hir.body(*impl_body_id);
visitor.visit_body(body);
} else {
visitor.visit_impl_item(impl_item);
}
}
} else {
for impl_item_ref in refs {
let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
visitor.visit_impl_item(impl_item);
}
}
}
}
@ -89,9 +190,7 @@ struct UseSelfVisitor<'a, 'tcx: 'a> {
impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
fn visit_path(&mut self, path: &'tcx Path, _id: NodeId) {
if self.item_path.def == path.def && path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfType.name() {
span_lint_and_then(self.cx, USE_SELF, path.span, "unnecessary structure name repetition", |db| {
db.span_suggestion(path.span, "use the applicable keyword", "Self".to_owned());
});
span_use_self_lint(self.cx, path);
}
walk_path(self, path);

2
tests/ui/methods.rs
View file

@ -4,7 +4,7 @@
#![warn(clippy, clippy_pedantic, option_unwrap_used)]
#![allow(blacklisted_name, unused, print_stdout, non_ascii_literal, new_without_default,
new_without_default_derive, missing_docs_in_private_items, needless_pass_by_value,
default_trait_access)]
default_trait_access, use_self)]
use std::collections::BTreeMap;
use std::collections::HashMap;

94
tests/ui/methods.stderr
View file

@ -1,47 +1,3 @@
error: unnecessary structure name repetition
--> $DIR/methods.rs:21:29
|
21 | pub fn add(self, other: T) -> T { self }
| ^ help: use the applicable keyword: `Self`
|
= note: `-D use-self` implied by `-D warnings`
error: unnecessary structure name repetition
--> $DIR/methods.rs:21:35
|
21 | pub fn add(self, other: T) -> T { self }
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:25:25
|
25 | fn eq(&self, other: T) -> bool { true } // no error, private function
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:27:26
|
27 | fn sub(&self, other: T) -> &T { self } // no error, self is a ref
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:27:33
|
27 | fn sub(&self, other: T) -> &T { self } // no error, self is a ref
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:28:21
|
28 | fn div(self) -> T { self } // no error, different #arguments
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:29:25
|
29 | fn rem(self, other: T) { } // no error, wrong return type
| ^ help: use the applicable keyword: `Self`
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:21:5
|
@ -78,30 +34,6 @@ error: methods called `new` usually return `Self`
|
= note: `-D new-ret-no-self` implied by `-D warnings`
error: unnecessary structure name repetition
--> $DIR/methods.rs:80:24
|
80 | fn new() -> Option<V<T>> { None }
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:84:19
|
84 | type Output = T;
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:85:25
|
85 | fn mul(self, other: T) -> T { self } // no error, obviously
| ^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:85:31
|
85 | fn mul(self, other: T) -> T { self } // no error, obviously
| ^ help: use the applicable keyword: `Self`
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:104:13
|
@ -251,24 +183,6 @@ error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done mor
174 | | );
| |_________________^
error: unnecessary structure name repetition
--> $DIR/methods.rs:200:24
|
200 | fn filter(self) -> IteratorFalsePositives {
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:204:22
|
204 | fn next(self) -> IteratorFalsePositives {
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/methods.rs:224:32
|
224 | fn skip(self, _: usize) -> IteratorFalsePositives {
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
--> $DIR/methods.rs:234:13
|
@ -343,12 +257,6 @@ error: called `is_some()` after searching an `Iterator` with rposition. This is
276 | | ).is_some();
| |______________________________^
error: unnecessary structure name repetition
--> $DIR/methods.rs:290:21
|
290 | fn new() -> Foo { Foo }
| ^^^ help: use the applicable keyword: `Self`
error: use of `unwrap_or` followed by a function call
--> $DIR/methods.rs:308:22
|
@ -527,5 +435,5 @@ error: used unwrap() on an Option value. If you don't want to handle the None ca
|
= note: `-D option-unwrap-used` implied by `-D warnings`
error: aborting due to 70 previous errors
error: aborting due to 55 previous errors

114
tests/ui/use_self.rs
View file

@ -3,6 +3,7 @@
#![warn(use_self)]
#![allow(dead_code)]
#![allow(should_implement_trait)]
#![allow(boxed_local)]
fn main() {}
@ -66,3 +67,116 @@ mod lifetimes {
}
}
}
mod traits {
#![cfg_attr(feature = "cargo-clippy", allow(boxed_local))]
trait SelfTrait {
fn refs(p1: &Self) -> &Self;
fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self;
fn mut_refs(p1: &mut Self) -> &mut Self;
fn nested(p1: Box<Self>, p2: (&u8, &Self));
fn vals(r: Self) -> Self;
}
#[derive(Default)]
struct Bad;
impl SelfTrait for Bad {
fn refs(p1: &Bad) -> &Bad {
p1
}
fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
p1
}
fn mut_refs(p1: &mut Bad) -> &mut Bad {
p1
}
fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
}
fn vals(_: Bad) -> Bad {
Bad::default()
}
}
#[derive(Default)]
struct Good;
impl SelfTrait for Good {
fn refs(p1: &Self) -> &Self {
p1
}
fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
p1
}
fn mut_refs(p1: &mut Self) -> &mut Self {
p1
}
fn nested(_p1: Box<Self>, _p2: (&u8, &Self)) {
}
fn vals(_: Self) -> Self {
Self::default()
}
}
trait NameTrait {
fn refs(p1: &u8) -> &u8;
fn ref_refs<'a>(p1: &'a &'a u8) -> &'a &'a u8;
fn mut_refs(p1: &mut u8) -> &mut u8;
fn nested(p1: Box<u8>, p2: (&u8, &u8));
fn vals(p1: u8) -> u8;
}
// Using `Self` instead of the type name is OK
impl NameTrait for u8 {
fn refs(p1: &Self) -> &Self {
p1
}
fn ref_refs<'a>(p1: &'a &'a Self) -> &'a &'a Self {
p1
}
fn mut_refs(p1: &mut Self) -> &mut Self {
p1
}
fn nested(_p1: Box<Self>, _p2: (&Self, &Self)) {
}
fn vals(_: Self) -> Self {
Self::default()
}
}
// Check that self arg isn't linted
impl Clone for Good {
fn clone(&self) -> Self {
// Note: Not linted and it wouldn't be valid
// because "can't use `Self` as a constructor`
Good
}
}
}
mod issue2894 {
trait IntoBytes {
fn into_bytes(&self) -> Vec<u8>;
}
// This should not be linted
impl IntoBytes for u8 {
fn into_bytes(&self) -> Vec<u8> {
vec![*self]
}
}
}

92
tests/ui/use_self.stderr
View file

@ -1,40 +1,106 @@
error: unnecessary structure name repetition
--> $DIR/use_self.rs:14:21
--> $DIR/use_self.rs:15:21
|
14 | fn new() -> Foo {
15 | fn new() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
|
= note: `-D use-self` implied by `-D warnings`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:15:13
--> $DIR/use_self.rs:16:13
|
15 | Foo {}
16 | Foo {}
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:17:22
--> $DIR/use_self.rs:18:22
|
17 | fn test() -> Foo {
18 | fn test() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:18:13
--> $DIR/use_self.rs:19:13
|
18 | Foo::new()
19 | Foo::new()
| ^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:23:25
--> $DIR/use_self.rs:24:25
|
23 | fn default() -> Foo {
24 | fn default() -> Foo {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:24:13
--> $DIR/use_self.rs:25:13
|
24 | Foo::new()
25 | Foo::new()
| ^^^^^^^^ help: use the applicable keyword: `Self`
error: aborting due to 6 previous errors
error: unnecessary structure name repetition
--> $DIR/use_self.rs:87:22
|
87 | fn refs(p1: &Bad) -> &Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:87:31
|
87 | fn refs(p1: &Bad) -> &Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:91:37
|
91 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:91:53
|
91 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:95:30
|
95 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:95:43
|
95 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:99:28
|
99 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:99:46
|
99 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:102:20
|
102 | fn vals(_: Bad) -> Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:102:28
|
102 | fn vals(_: Bad) -> Bad {
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:103:13
|
103 | Bad::default()
| ^^^^^^^^^^^^ help: use the applicable keyword: `Self`
error: aborting due to 17 previous errors