Skip to content

WIP: use_self checking lifetimes & generics #1997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 53 additions & 21 deletions clippy_lints/src/use_self.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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<Ty>,
self_path: &'a Path,
cx: &'a LateContext<'a, 'tcx>,
body_id: Option<BodyId>,
}

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<Ty>`: {:?}", self.self_type);
println!("Self `Path`: {:?}", self.self_path);
println!("Struct Literal `TyS`: {:?}", ty);
println!();
}
}
}

walk_expr(self, expr);
}
}
21 changes: 19 additions & 2 deletions tests/ui/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand All @@ -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"}
Expand All @@ -66,3 +65,21 @@ mod lifetimes {
}
}
}

mod generics {
struct Foo<T> {
value: T,
}

impl<T> Foo<T> {
// `Self` is applicable here
fn foo(value: T) -> Foo<T> {
Foo { value }
}

// `Cannot` use `Self` as a return type as the generic types are different
fn bar(value: i32) -> Foo<i32> {
Foo { value }
}
}
}