diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 8203377e465f..842d1130f4c2 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -1,9 +1,10 @@ use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::hir::*; -use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor}; +use rustc::hir::intravisit::{walk_path, walk_expr, walk_body, walk_impl_item, NestedVisitorMap, Visitor}; use utils::{in_macro, span_lint_and_then}; use syntax::ast::NodeId; use syntax_pos::symbol::keywords::SelfType; +use syntax::ptr::P; /// **What it does:** Checks for unnecessary repetition of structure name when a /// replacement with `Self` is applicable. @@ -56,45 +57,76 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { } if_chain! { if let ItemImpl(.., ref item_type, ref refs) = item.node; - if let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node; + if let Ty_::TyPath(QPath::Resolved(_, ref self_path)) = item_type.node; then { - let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).parameters; - let should_check = if let Some(ref params) = *parameters { - !params.parenthesized && params.lifetimes.len() == 0 - } else { - true - }; - if should_check { + let parameters = &self_path.segments.last().expect(SEGMENTS_MSG).parameters; let visitor = &mut UseSelfVisitor { - item_path: item_path, - cx: cx, + self_path, + cx, + body_id: None, + self_type: item_type, }; for impl_item_ref in refs { visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id)); } - } } } } } struct UseSelfVisitor<'a, 'tcx: 'a> { - item_path: &'a Path, + self_type: &'a P, + self_path: &'a Path, cx: &'a LateContext<'a, 'tcx>, + body_id: Option, } 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).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()); - }); - } + // fn visit_path(&mut self, path: &'tcx Path, _id: NodeId) { + // if self.self_path.def == path.def && path.segments.last().expect(SEGMENTS_MSG).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()); + // }); + // } - walk_path(self, path); - } + // walk_path(self, path); + // } + + // fn visit_impl_item(&mut self, impl_item: &'tcx ImplItem) { + // println!("{:?}", impl_item); + // walk_impl_item(self, impl_item); + // } fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::OnlyBodies(&self.cx.tcx.hir) } + + //TODO: This is just a hack to see if having the body id is useful + fn visit_body(&mut self, body: &'tcx Body) { + self.body_id = Some(body.id()); + walk_body(self, body); + } + + fn visit_expr(&mut self, expr: &'tcx Expr) { + if let Expr_::ExprStruct(ref path, _, _) = expr.node { + let segment = match *path { + QPath::Resolved(_, ref path) => path.segments + .last() + .expect(SEGMENTS_MSG), + QPath::TypeRelative(_, ref segment) => segment, + }; + + if let Some(body_id) = self.body_id { + if segment.name != SelfType.name() { + let ty = self.cx.tcx.body_tables(body_id).expr_ty(expr); + println!("Self `P`: {:?}", self.self_type); + println!("Self `Path`: {:?}", self.self_path); + println!("Struct Literal `TyS`: {:?}", ty); + println!(); + } + } + } + + walk_expr(self, expr); + } } diff --git a/tests/ui/use_self.rs b/tests/ui/use_self.rs index b12900b7691a..a98def32208e 100644 --- a/tests/ui/use_self.rs +++ b/tests/ui/use_self.rs @@ -45,8 +45,6 @@ mod better { } } -//todo the lint does not handle lifetimed struct -//the following module should trigger the lint on the third method only mod lifetimes { struct Foo<'a>{foo_str: &'a str} @@ -55,6 +53,7 @@ mod lifetimes { fn foo(s: &str) -> Foo { Foo { foo_str: s } } + // cannot replace with `Self`, because that's `Foo<'a>` fn bar() -> Foo<'static> { Foo { foo_str: "foo"} @@ -66,3 +65,21 @@ mod lifetimes { } } } + +mod generics { + struct Foo { + value: T, + } + + impl Foo { + // `Self` is applicable here + fn foo(value: T) -> Foo { + Foo { value } + } + + // `Cannot` use `Self` as a return type as the generic types are different + fn bar(value: i32) -> Foo { + Foo { value } + } + } +}