From 8494f57c82f6a1ff79a1065c8025f7e68dbe26de Mon Sep 17 00:00:00 2001 From: Guillem Nieto Date: Sat, 24 Feb 2018 02:02:48 +0100 Subject: [PATCH 1/2] Fix author lint The author lint was generating invalid code as shown on issue: https://github.com/rust-lang-nursery/rust-clippy/issues/2442 I've changed the generated code to properly track cast expressions. Unfortunatelly, I've had to rewrite the `visit_decl` method, to avoid that last if of the chain will be added. After looking at the code, this last line was being added because of the `let x: char` part, but not because of the `0x45df as char` expression. It seems that let statements should not generate code on the author lint, but I'm not sure that this is true or if I'm breaking something on other code generation parts. Finally, I've added a test for the author lint, but I'm not sure that this needs to be added to the testsuite. --- clippy_lints/src/utils/author.rs | 30 ++++++++++++++++++++++++++---- tests/ui/author.rs | 7 +++++++ tests/ui/author.stdout | 10 ++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 tests/ui/author.rs create mode 100644 tests/ui/author.stdout diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index c824c8906a7..d7d29ceda6e 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -5,8 +5,9 @@ use rustc::lint::*; use rustc::hir; -use rustc::hir::{Expr, Expr_, QPath}; -use rustc::hir::intravisit::{NestedVisitorMap, Visitor}; +use rustc::hir::{Expr, Expr_, QPath, Ty_}; +use rustc::hir::intravisit::{NestedVisitorMap, Visitor, walk_decl}; +use rustc::hir::Decl; use syntax::ast::{self, Attribute, LitKind, NodeId, DUMMY_NODE_ID}; use syntax::codemap::Span; use std::collections::HashMap; @@ -79,6 +80,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { return; } prelude(); + PrintVisitor::new("item").visit_impl_item(item); done(); } @@ -182,6 +184,18 @@ struct PrintVisitor { } impl<'tcx> Visitor<'tcx> for PrintVisitor { + fn visit_decl(&mut self, d: &'tcx Decl) { + match d.node { + hir::DeclLocal(ref local) => { + self.visit_pat(&local.pat); + if let Some(ref e) = local.init { + self.visit_expr(e); + } + }, + _ => walk_decl(self, d) + } + } + fn visit_expr(&mut self, expr: &Expr) { print!(" if let Expr_::Expr"); let current = format!("{}.node", self.current); @@ -260,9 +274,17 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { }, } }, - Expr_::ExprCast(ref expr, ref _ty) => { + Expr_::ExprCast(ref expr, ref ty) => { let cast_pat = self.next("expr"); - println!("Cast(ref {}, _) = {};", cast_pat, current); + let cast_ty = self.next("cast_ty"); + let qp_label = self.next("qp"); + + println!("Cast(ref {}, ref {}) = {};", cast_pat, cast_ty, current); + if let Ty_::TyPath(ref qp) = ty.node { + println!(" if let Ty_::TyPath(ref {}) = {}.node;", qp_label, cast_ty); + self.current = qp_label; + self.visit_qpath(&qp, ty.id, ty.span); + } self.current = cast_pat; self.visit_expr(expr); }, diff --git a/tests/ui/author.rs b/tests/ui/author.rs new file mode 100644 index 00000000000..3a819872bc5 --- /dev/null +++ b/tests/ui/author.rs @@ -0,0 +1,7 @@ +#![feature(plugin, custom_attribute)] + +fn main() { + + #[clippy(author)] + let x: char = 0x45 as char; +} diff --git a/tests/ui/author.stdout b/tests/ui/author.stdout new file mode 100644 index 00000000000..0efb3e8b272 --- /dev/null +++ b/tests/ui/author.stdout @@ -0,0 +1,10 @@ +if_chain! { + if let Expr_::ExprCast(ref expr, ref cast_ty) = stmt.node; + if let Ty_::TyPath(ref qp) = cast_ty.node; + if match_qpath(qp, &["char"]); + if let Expr_::ExprLit(ref lit) = expr.node; + if let LitKind::Int(69, _) = lit.node; + then { + // report your lint here + } +} From 3ac84b2542ec1c4caeab54239c67202113c82ea0 Mon Sep 17 00:00:00 2001 From: Guillem Nieto Date: Sat, 24 Feb 2018 19:34:51 +0100 Subject: [PATCH 2/2] Remove explicit visit_qpath method Instead of replacing the default behaviour of the visit_qpath method, I've moved the printing code to private method of PrintVisitor (print_qpath). --- clippy_lints/src/utils/author.rs | 36 ++++++++++---------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index d7d29ceda6e..beae98f81f6 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -6,10 +6,8 @@ use rustc::lint::*; use rustc::hir; use rustc::hir::{Expr, Expr_, QPath, Ty_}; -use rustc::hir::intravisit::{NestedVisitorMap, Visitor, walk_decl}; -use rustc::hir::Decl; -use syntax::ast::{self, Attribute, LitKind, NodeId, DUMMY_NODE_ID}; -use syntax::codemap::Span; +use rustc::hir::intravisit::{NestedVisitorMap, Visitor}; +use syntax::ast::{self, Attribute, LitKind, DUMMY_NODE_ID}; use std::collections::HashMap; /// **What it does:** Generates clippy code that detects the offending pattern @@ -80,7 +78,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { return; } prelude(); - PrintVisitor::new("item").visit_impl_item(item); done(); } @@ -173,6 +170,12 @@ impl PrintVisitor { }, } } + + fn print_qpath(&mut self, path: &QPath) { + print!(" if match_qpath({}, &[", self.current); + print_path(path, &mut true); + println!("]);"); + } } struct PrintVisitor { @@ -184,18 +187,6 @@ struct PrintVisitor { } impl<'tcx> Visitor<'tcx> for PrintVisitor { - fn visit_decl(&mut self, d: &'tcx Decl) { - match d.node { - hir::DeclLocal(ref local) => { - self.visit_pat(&local.pat); - if let Some(ref e) = local.init { - self.visit_expr(e); - } - }, - _ => walk_decl(self, d) - } - } - fn visit_expr(&mut self, expr: &Expr) { print!(" if let Expr_::Expr"); let current = format!("{}.node", self.current); @@ -283,7 +274,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { if let Ty_::TyPath(ref qp) = ty.node { println!(" if let Ty_::TyPath(ref {}) = {}.node;", qp_label, cast_ty); self.current = qp_label; - self.visit_qpath(&qp, ty.id, ty.span); + self.print_qpath(&qp); } self.current = cast_pat; self.visit_expr(expr); @@ -398,7 +389,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { let path_pat = self.next("path"); println!("Path(ref {}) = {};", path_pat, current); self.current = path_pat; - self.visit_qpath(path, expr.id, expr.span); + self.print_qpath(path); }, Expr_::ExprAddrOf(mutability, ref inner) => { let inner_pat = self.next("inner"); @@ -453,7 +444,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { println!("Struct(ref {}, ref {}, None) = {};", path_pat, fields_pat, current); } self.current = path_pat; - self.visit_qpath(path, expr.id, expr.span); + self.print_qpath(path); println!(" if {}.len() == {};", fields_pat, fields.len()); println!(" // unimplemented: field checks"); }, @@ -468,11 +459,6 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { } } - fn visit_qpath(&mut self, path: &QPath, _: NodeId, _: Span) { - print!(" if match_qpath({}, &[", self.current); - print_path(path, &mut true); - println!("]);"); - } fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::None }