Auto merge of #7484 - camsteffen:author, r=flip1995

Some `clippy::author` improvements

changelog: none

* Use `Debug` instead of re-implementing it for some things
* Fix block trailing expression handing
* Don't double print on stmt/expr with `#[clippy::author]` attribute

6 files changed, 69 insertions, 60 deletions

diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs
index 39ef170ae36..c1e7fd7fe95 100644
--- a/clippy_lints/src/utils/author.rs
+++ b/clippy_lints/src/utils/author.rs
@@ -7,7 +7,7 @@ use rustc_ast::walk_list;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
-use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
+use rustc_hir::{Block, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::hir::map::Map;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -132,6 +132,10 @@ impl<'tcx> LateLintPass<'tcx> for Author {
         if !has_attr(cx, stmt.hir_id) {
             return;
         }
+        match stmt.kind {
+            StmtKind::Expr(e) | StmtKind::Semi(e) if has_attr(cx, e.hir_id) => return,
+            _ => {},
+        }
         prelude();
         PrintVisitor::new("stmt").visit_stmt(stmt);
         done();
@@ -316,11 +320,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
                 self.current = cast_pat;
                 self.visit_expr(expr);
             },
-            ExprKind::Loop(body, _, desugaring, _) => {
+            ExprKind::Loop(body, _, des, _) => {
                 let body_pat = self.next("body");
-                let des = loop_desugaring_name(desugaring);
                 let label_pat = self.next("label");
-                println!("Loop(ref {}, ref {}, {}) = {};", body_pat, label_pat, des, current);
+                println!(
+                    "Loop(ref {}, ref {}, LoopSource::{:?}) = {};",
+                    body_pat, label_pat, des, current
+                );
                 self.current = body_pat;
                 self.visit_block(body);
             },
@@ -343,11 +349,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
                 self.current = then_pat;
                 self.visit_expr(then);
             },
-            ExprKind::Match(expr, arms, desugaring) => {
-                let des = desugaring_name(desugaring);
+            ExprKind::Match(expr, arms, des) => {
                 let expr_pat = self.next("expr");
                 let arms_pat = self.next("arms");
-                println!("Match(ref {}, ref {}, {}) = {};", expr_pat, arms_pat, des, current);
+                println!(
+                    "Match(ref {}, ref {}, MatchSource::{:?}) = {};",
+                    expr_pat, arms_pat, des, current
+                );
                 self.current = expr_pat;
                 self.visit_expr(expr);
                 println!("    if {}.len() == {};", arms_pat, arms.len());
@@ -536,14 +544,19 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
     }
 
     fn visit_block(&mut self, block: &Block<'_>) {
-        let trailing_pat = self.next("trailing_expr");
-        println!("    if let Some({}) = &{}.expr;", trailing_pat, self.current);
         println!("    if {}.stmts.len() == {};", self.current, block.stmts.len());
-        let current = self.current.clone();
+        let block_name = self.current.clone();
         for (i, stmt) in block.stmts.iter().enumerate() {
-            self.current = format!("{}.stmts[{}]", current, i);
+            self.current = format!("{}.stmts[{}]", block_name, i);
             self.visit_stmt(stmt);
         }
+        if let Some(expr) = block.expr {
+            self.current = self.next("trailing_expr");
+            println!("    if let Some({}) = &{}.expr;", self.current, block_name);
+            self.visit_expr(expr);
+        } else {
+            println!("    if {}.expr.is_none();", block_name);
+        }
     }
 
     #[allow(clippy::too_many_lines)]
@@ -553,12 +566,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
         match pat.kind {
             PatKind::Wild => println!("Wild = {};", current),
             PatKind::Binding(anno, .., ident, ref sub) => {
-                let anno_pat = match anno {
-                    BindingAnnotation::Unannotated => "BindingAnnotation::Unannotated",
-                    BindingAnnotation::Mutable => "BindingAnnotation::Mutable",
-                    BindingAnnotation::Ref => "BindingAnnotation::Ref",
-                    BindingAnnotation::RefMut => "BindingAnnotation::RefMut",
-                };
+                let anno_pat = &format!("BindingAnnotation::{:?}", anno);
                 let name_pat = self.next("name");
                 if let Some(sub) = *sub {
                     let sub_pat = self.next("sub");
@@ -723,33 +731,6 @@ fn has_attr(cx: &LateContext<'_>, hir_id: hir::HirId) -> bool {
     get_attr(cx.sess(), attrs, "author").count() > 0
 }
 
-#[must_use]
-fn desugaring_name(des: hir::MatchSource) -> String {
-    match des {
-        hir::MatchSource::ForLoopDesugar => "MatchSource::ForLoopDesugar".to_string(),
-        hir::MatchSource::TryDesugar => "MatchSource::TryDesugar".to_string(),
-        hir::MatchSource::WhileDesugar => "MatchSource::WhileDesugar".to_string(),
-        hir::MatchSource::WhileLetDesugar => "MatchSource::WhileLetDesugar".to_string(),
-        hir::MatchSource::Normal => "MatchSource::Normal".to_string(),
-        hir::MatchSource::IfLetDesugar { contains_else_clause } => format!(
-            "MatchSource::IfLetDesugar {{ contains_else_clause: {} }}",
-            contains_else_clause
-        ),
-        hir::MatchSource::IfLetGuardDesugar => "MatchSource::IfLetGuardDesugar".to_string(),
-        hir::MatchSource::AwaitDesugar => "MatchSource::AwaitDesugar".to_string(),
-    }
-}
-
-#[must_use]
-fn loop_desugaring_name(des: hir::LoopSource) -> &'static str {
-    match des {
-        hir::LoopSource::ForLoop => "LoopSource::ForLoop",
-        hir::LoopSource::Loop => "LoopSource::Loop",
-        hir::LoopSource::While => "LoopSource::While",
-        hir::LoopSource::WhileLet => "LoopSource::WhileLet",
-    }
-}
-
 fn print_path(path: &QPath<'_>, first: &mut bool) {
     match *path {
         QPath::Resolved(_, path) => {
diff --git a/tests/ui/author/blocks.rs b/tests/ui/author/blocks.rs
index a8068436b70..c8465cd59aa 100644
--- a/tests/ui/author/blocks.rs
+++ b/tests/ui/author/blocks.rs
@@ -1,16 +1,15 @@
-#![feature(stmt_expr_attributes)]
 #![allow(redundant_semicolons, clippy::no_effect)]
 
 #[rustfmt::skip]
 fn main() {
     #[clippy::author]
     {
-        ;;;;
-    }
-}
-
-#[clippy::author]
-fn foo() {
-    let x = 42i32;
-    -x;
+        let x = 42i32;
+        -x;
+    };
+    #[clippy::author]
+    {
+        let expr = String::new();
+        drop(expr)
+    };
 }
diff --git a/tests/ui/author/blocks.stdout b/tests/ui/author/blocks.stdout
index c8ef75e48fc..854bc28083a 100644
--- a/tests/ui/author/blocks.stdout
+++ b/tests/ui/author/blocks.stdout
@@ -1,12 +1,39 @@
 if_chain! {
     if let ExprKind::Block(ref block) = expr.kind;
-    if let Some(trailing_expr) = &block.expr;
-    if block.stmts.len() == 0;
+    if block.stmts.len() == 2;
+    if let StmtKind::Local(ref local) = block.stmts[0].kind;
+    if let Some(ref init) = local.init;
+    if let ExprKind::Lit(ref lit) = init.kind;
+    if let LitKind::Int(42, _) = lit.node;
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.kind;
+    if name.as_str() == "x";
+    if let StmtKind::Semi(ref e, _) = block.stmts[1].kind
+    if let ExprKind::Unary(UnOp::Neg, ref inner) = e.kind;
+    if let ExprKind::Path(ref path) = inner.kind;
+    if match_qpath(path, &["x"]);
+    if block.expr.is_none();
     then {
         // report your lint here
     }
 }
 if_chain! {
+    if let ExprKind::Block(ref block) = expr.kind;
+    if block.stmts.len() == 1;
+    if let StmtKind::Local(ref local) = block.stmts[0].kind;
+    if let Some(ref init) = local.init;
+    if let ExprKind::Call(ref func, ref args) = init.kind;
+    if let ExprKind::Path(ref path) = func.kind;
+    if match_qpath(path, &["String", "new"]);
+    if args.len() == 0;
+    if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.kind;
+    if name.as_str() == "expr";
+    if let Some(trailing_expr) = &block.expr;
+    if let ExprKind::Call(ref func1, ref args1) = trailing_expr.kind;
+    if let ExprKind::Path(ref path1) = func1.kind;
+    if match_qpath(path1, &["drop"]);
+    if args1.len() == 1;
+    if let ExprKind::Path(ref path2) = args1[0].kind;
+    if match_qpath(path2, &["expr"]);
     then {
         // report your lint here
     }
diff --git a/tests/ui/author/for_loop.stdout b/tests/ui/author/for_loop.stdout
index 3bf7607c62f..f1b4d4e096e 100644
--- a/tests/ui/author/for_loop.stdout
+++ b/tests/ui/author/for_loop.stdout
@@ -11,7 +11,6 @@ if_chain! {
     // unimplemented: field checks
     if arms.len() == 1;
     if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.kind;
-    if let Some(trailing_expr) = &body.expr;
     if body.stmts.len() == 4;
     if let StmtKind::Local(ref local) = body.stmts[0].kind;
     if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.kind;
@@ -48,7 +47,6 @@ if_chain! {
     if name1.as_str() == "y";
     if let StmtKind::Expr(ref e1, _) = body.stmts[3].kind
     if let ExprKind::Block(ref block) = e1.kind;
-    if let Some(trailing_expr1) = &block.expr;
     if block.stmts.len() == 1;
     if let StmtKind::Local(ref local2) = block.stmts[0].kind;
     if let Some(ref init1) = local2.init;
@@ -56,6 +54,8 @@ if_chain! {
     if match_qpath(path9, &["y"]);
     if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.kind;
     if name2.as_str() == "z";
+    if block.expr.is_none();
+    if body.expr.is_none();
     if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pat.kind;
     if name3.as_str() == "iter";
     then {
diff --git a/tests/ui/author/if.stdout b/tests/ui/author/if.stdout
index cac64a3f40b..502b38545b8 100644
--- a/tests/ui/author/if.stdout
+++ b/tests/ui/author/if.stdout
@@ -3,7 +3,6 @@ if_chain! {
     if let Some(ref init) = local.init;
     if let ExprKind::If(ref cond, ref then, Some(ref else_)) = init.kind;
     if let ExprKind::Block(ref block) = else_.kind;
-    if let Some(trailing_expr) = &block.expr;
     if block.stmts.len() == 1;
     if let StmtKind::Semi(ref e, _) = block.stmts[0].kind
     if let ExprKind::Binary(ref op, ref left, ref right) = e.kind;
@@ -12,10 +11,10 @@ if_chain! {
     if let LitKind::Int(2, _) = lit.node;
     if let ExprKind::Lit(ref lit1) = right.kind;
     if let LitKind::Int(2, _) = lit1.node;
+    if block.expr.is_none();
     if let ExprKind::Lit(ref lit2) = cond.kind;
     if let LitKind::Bool(true) = lit2.node;
     if let ExprKind::Block(ref block1) = then.kind;
-    if let Some(trailing_expr1) = &block1.expr;
     if block1.stmts.len() == 1;
     if let StmtKind::Semi(ref e1, _) = block1.stmts[0].kind
     if let ExprKind::Binary(ref op1, ref left1, ref right1) = e1.kind;
@@ -24,6 +23,7 @@ if_chain! {
     if let LitKind::Int(1, _) = lit3.node;
     if let ExprKind::Lit(ref lit4) = right1.kind;
     if let LitKind::Int(1, _) = lit4.node;
+    if block1.expr.is_none();
     if let PatKind::Wild = local.pat.kind;
     then {
         // report your lint here
diff --git a/tests/ui/author/matches.stdout b/tests/ui/author/matches.stdout
index 2e8f8227dca..68cc2b214eb 100644
--- a/tests/ui/author/matches.stdout
+++ b/tests/ui/author/matches.stdout
@@ -11,7 +11,6 @@ if_chain! {
     if let ExprKind::Lit(ref lit2) = lit_expr.kind;
     if let LitKind::Int(16, _) = lit2.node;
     if let ExprKind::Block(ref block) = arms[1].body.kind;
-    if let Some(trailing_expr) = &block.expr;
     if block.stmts.len() == 1;
     if let StmtKind::Local(ref local1) = block.stmts[0].kind;
     if let Some(ref init1) = local1.init;
@@ -19,6 +18,9 @@ if_chain! {
     if let LitKind::Int(3, _) = lit3.node;
     if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local1.pat.kind;
     if name.as_str() == "x";
+    if let Some(trailing_expr) = &block.expr;
+    if let ExprKind::Path(ref path) = trailing_expr.kind;
+    if match_qpath(path, &["x"]);
     if let PatKind::Lit(ref lit_expr1) = arms[1].pat.kind
     if let ExprKind::Lit(ref lit4) = lit_expr1.kind;
     if let LitKind::Int(17, _) = lit4.node;