Skip to content

Commit 6d9036b

Browse files
borsbors
committed
Auto merge of rust-lang#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
2 parents ea69a9d + d3492a0 commit 6d9036b

File tree

6 files changed

+69
-60
lines changed

6 files changed

+69
-60
lines changed

‎clippy_lints/src/utils/author.rs

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use rustc_ast::walk_list;
77
use rustc_data_structures::fx::FxHashMap;
88
use rustc_hir as hir;
99
use rustc_hir::intravisit::{NestedVisitorMap, Visitor};
10-
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
10+
use rustc_hir::{Block, Expr, ExprKind, Pat, PatKind, QPath, Stmt, StmtKind, TyKind};
1111
use rustc_lint::{LateContext, LateLintPass, LintContext};
1212
use rustc_middle::hir::map::Map;
1313
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -132,6 +132,10 @@ impl<'tcx> LateLintPass<'tcx> for Author {
132132
if !has_attr(cx, stmt.hir_id) {
133133
return;
134134
}
135+
match stmt.kind {
136+
StmtKind::Expr(e) | StmtKind::Semi(e) if has_attr(cx, e.hir_id) => return,
137+
_ => {},
138+
}
135139
prelude();
136140
PrintVisitor::new("stmt").visit_stmt(stmt);
137141
done();
@@ -316,11 +320,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
316320
self.current = cast_pat;
317321
self.visit_expr(expr);
318322
},
319-
ExprKind::Loop(body, _, desugaring, _) => {
323+
ExprKind::Loop(body, _, des, _) => {
320324
let body_pat = self.next("body");
321-
let des = loop_desugaring_name(desugaring);
322325
let label_pat = self.next("label");
323-
println!("Loop(ref {}, ref {}, {}) = {};", body_pat, label_pat, des, current);
326+
println!(
327+
"Loop(ref {}, ref {}, LoopSource::{:?}) = {};",
328+
body_pat, label_pat, des, current
329+
);
324330
self.current = body_pat;
325331
self.visit_block(body);
326332
},
@@ -343,11 +349,13 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
343349
self.current = then_pat;
344350
self.visit_expr(then);
345351
},
346-
ExprKind::Match(expr, arms, desugaring) => {
347-
let des = desugaring_name(desugaring);
352+
ExprKind::Match(expr, arms, des) => {
348353
let expr_pat = self.next("expr");
349354
let arms_pat = self.next("arms");
350-
println!("Match(ref {}, ref {}, {}) = {};", expr_pat, arms_pat, des, current);
355+
println!(
356+
"Match(ref {}, ref {}, MatchSource::{:?}) = {};",
357+
expr_pat, arms_pat, des, current
358+
);
351359
self.current = expr_pat;
352360
self.visit_expr(expr);
353361
println!(" if {}.len() == {};", arms_pat, arms.len());
@@ -536,14 +544,19 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
536544
}
537545

538546
fn visit_block(&mut self, block: &Block<'_>) {
539-
let trailing_pat = self.next("trailing_expr");
540-
println!(" if let Some({}) = &{}.expr;", trailing_pat, self.current);
541547
println!(" if {}.stmts.len() == {};", self.current, block.stmts.len());
542-
let current = self.current.clone();
548+
let block_name = self.current.clone();
543549
for (i, stmt) in block.stmts.iter().enumerate() {
544-
self.current = format!("{}.stmts[{}]", current, i);
550+
self.current = format!("{}.stmts[{}]", block_name, i);
545551
self.visit_stmt(stmt);
546552
}
553+
if let Some(expr) = block.expr {
554+
self.current = self.next("trailing_expr");
555+
println!(" if let Some({}) = &{}.expr;", self.current, block_name);
556+
self.visit_expr(expr);
557+
} else {
558+
println!(" if {}.expr.is_none();", block_name);
559+
}
547560
}
548561

549562
#[allow(clippy::too_many_lines)]
@@ -553,12 +566,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
553566
match pat.kind {
554567
PatKind::Wild => println!("Wild = {};", current),
555568
PatKind::Binding(anno, .., ident, ref sub) => {
556-
let anno_pat = match anno {
557-
BindingAnnotation::Unannotated => "BindingAnnotation::Unannotated",
558-
BindingAnnotation::Mutable => "BindingAnnotation::Mutable",
559-
BindingAnnotation::Ref => "BindingAnnotation::Ref",
560-
BindingAnnotation::RefMut => "BindingAnnotation::RefMut",
561-
};
569+
let anno_pat = &format!("BindingAnnotation::{:?}", anno);
562570
let name_pat = self.next("name");
563571
if let Some(sub) = *sub {
564572
let sub_pat = self.next("sub");
@@ -723,33 +731,6 @@ fn has_attr(cx: &LateContext<'_>, hir_id: hir::HirId) -> bool {
723731
get_attr(cx.sess(), attrs, "author").count() > 0
724732
}
725733

726-
#[must_use]
727-
fn desugaring_name(des: hir::MatchSource) -> String {
728-
match des {
729-
hir::MatchSource::ForLoopDesugar => "MatchSource::ForLoopDesugar".to_string(),
730-
hir::MatchSource::TryDesugar => "MatchSource::TryDesugar".to_string(),
731-
hir::MatchSource::WhileDesugar => "MatchSource::WhileDesugar".to_string(),
732-
hir::MatchSource::WhileLetDesugar => "MatchSource::WhileLetDesugar".to_string(),
733-
hir::MatchSource::Normal => "MatchSource::Normal".to_string(),
734-
hir::MatchSource::IfLetDesugar { contains_else_clause } => format!(
735-
"MatchSource::IfLetDesugar {{ contains_else_clause: {} }}",
736-
contains_else_clause
737-
),
738-
hir::MatchSource::IfLetGuardDesugar => "MatchSource::IfLetGuardDesugar".to_string(),
739-
hir::MatchSource::AwaitDesugar => "MatchSource::AwaitDesugar".to_string(),
740-
}
741-
}
742-
743-
#[must_use]
744-
fn loop_desugaring_name(des: hir::LoopSource) -> &'static str {
745-
match des {
746-
hir::LoopSource::ForLoop => "LoopSource::ForLoop",
747-
hir::LoopSource::Loop => "LoopSource::Loop",
748-
hir::LoopSource::While => "LoopSource::While",
749-
hir::LoopSource::WhileLet => "LoopSource::WhileLet",
750-
}
751-
}
752-
753734
fn print_path(path: &QPath<'_>, first: &mut bool) {
754735
match *path {
755736
QPath::Resolved(_, path) => {

‎tests/ui/author/blocks.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
1-
#![feature(stmt_expr_attributes)]
21
#![allow(redundant_semicolons, clippy::no_effect)]
32

43
#[rustfmt::skip]
54
fn main() {
65
#[clippy::author]
76
{
8-
;;;;
9-
}
10-
}
11-
12-
#[clippy::author]
13-
fn foo() {
14-
let x = 42i32;
15-
-x;
7+
let x = 42i32;
8+
-x;
9+
};
10+
#[clippy::author]
11+
{
12+
let expr = String::new();
13+
drop(expr)
14+
};
1615
}

‎tests/ui/author/blocks.stdout

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,39 @@
11
if_chain! {
22
if let ExprKind::Block(ref block) = expr.kind;
3-
if let Some(trailing_expr) = &block.expr;
4-
if block.stmts.len() == 0;
3+
if block.stmts.len() == 2;
4+
if let StmtKind::Local(ref local) = block.stmts[0].kind;
5+
if let Some(ref init) = local.init;
6+
if let ExprKind::Lit(ref lit) = init.kind;
7+
if let LitKind::Int(42, _) = lit.node;
8+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.kind;
9+
if name.as_str() == "x";
10+
if let StmtKind::Semi(ref e, _) = block.stmts[1].kind
11+
if let ExprKind::Unary(UnOp::Neg, ref inner) = e.kind;
12+
if let ExprKind::Path(ref path) = inner.kind;
13+
if match_qpath(path, &["x"]);
14+
if block.expr.is_none();
515
then {
616
// report your lint here
717
}
818
}
919
if_chain! {
20+
if let ExprKind::Block(ref block) = expr.kind;
21+
if block.stmts.len() == 1;
22+
if let StmtKind::Local(ref local) = block.stmts[0].kind;
23+
if let Some(ref init) = local.init;
24+
if let ExprKind::Call(ref func, ref args) = init.kind;
25+
if let ExprKind::Path(ref path) = func.kind;
26+
if match_qpath(path, &["String", "new"]);
27+
if args.len() == 0;
28+
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local.pat.kind;
29+
if name.as_str() == "expr";
30+
if let Some(trailing_expr) = &block.expr;
31+
if let ExprKind::Call(ref func1, ref args1) = trailing_expr.kind;
32+
if let ExprKind::Path(ref path1) = func1.kind;
33+
if match_qpath(path1, &["drop"]);
34+
if args1.len() == 1;
35+
if let ExprKind::Path(ref path2) = args1[0].kind;
36+
if match_qpath(path2, &["expr"]);
1037
then {
1138
// report your lint here
1239
}

‎tests/ui/author/for_loop.stdout

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ if_chain! {
1111
// unimplemented: field checks
1212
if arms.len() == 1;
1313
if let ExprKind::Loop(ref body, ref label, LoopSource::ForLoop) = arms[0].body.kind;
14-
if let Some(trailing_expr) = &body.expr;
1514
if body.stmts.len() == 4;
1615
if let StmtKind::Local(ref local) = body.stmts[0].kind;
1716
if let PatKind::Binding(BindingAnnotation::Mutable, _, name, None) = local.pat.kind;
@@ -48,14 +47,15 @@ if_chain! {
4847
if name1.as_str() == "y";
4948
if let StmtKind::Expr(ref e1, _) = body.stmts[3].kind
5049
if let ExprKind::Block(ref block) = e1.kind;
51-
if let Some(trailing_expr1) = &block.expr;
5250
if block.stmts.len() == 1;
5351
if let StmtKind::Local(ref local2) = block.stmts[0].kind;
5452
if let Some(ref init1) = local2.init;
5553
if let ExprKind::Path(ref path9) = init1.kind;
5654
if match_qpath(path9, &["y"]);
5755
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.kind;
5856
if name2.as_str() == "z";
57+
if block.expr.is_none();
58+
if body.expr.is_none();
5959
if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pat.kind;
6060
if name3.as_str() == "iter";
6161
then {

‎tests/ui/author/if.stdout

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ if_chain! {
33
if let Some(ref init) = local.init;
44
if let ExprKind::If(ref cond, ref then, Some(ref else_)) = init.kind;
55
if let ExprKind::Block(ref block) = else_.kind;
6-
if let Some(trailing_expr) = &block.expr;
76
if block.stmts.len() == 1;
87
if let StmtKind::Semi(ref e, _) = block.stmts[0].kind
98
if let ExprKind::Binary(ref op, ref left, ref right) = e.kind;
@@ -12,10 +11,10 @@ if_chain! {
1211
if let LitKind::Int(2, _) = lit.node;
1312
if let ExprKind::Lit(ref lit1) = right.kind;
1413
if let LitKind::Int(2, _) = lit1.node;
14+
if block.expr.is_none();
1515
if let ExprKind::Lit(ref lit2) = cond.kind;
1616
if let LitKind::Bool(true) = lit2.node;
1717
if let ExprKind::Block(ref block1) = then.kind;
18-
if let Some(trailing_expr1) = &block1.expr;
1918
if block1.stmts.len() == 1;
2019
if let StmtKind::Semi(ref e1, _) = block1.stmts[0].kind
2120
if let ExprKind::Binary(ref op1, ref left1, ref right1) = e1.kind;
@@ -24,6 +23,7 @@ if_chain! {
2423
if let LitKind::Int(1, _) = lit3.node;
2524
if let ExprKind::Lit(ref lit4) = right1.kind;
2625
if let LitKind::Int(1, _) = lit4.node;
26+
if block1.expr.is_none();
2727
if let PatKind::Wild = local.pat.kind;
2828
then {
2929
// report your lint here

‎tests/ui/author/matches.stdout

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ if_chain! {
1111
if let ExprKind::Lit(ref lit2) = lit_expr.kind;
1212
if let LitKind::Int(16, _) = lit2.node;
1313
if let ExprKind::Block(ref block) = arms[1].body.kind;
14-
if let Some(trailing_expr) = &block.expr;
1514
if block.stmts.len() == 1;
1615
if let StmtKind::Local(ref local1) = block.stmts[0].kind;
1716
if let Some(ref init1) = local1.init;
1817
if let ExprKind::Lit(ref lit3) = init1.kind;
1918
if let LitKind::Int(3, _) = lit3.node;
2019
if let PatKind::Binding(BindingAnnotation::Unannotated, _, name, None) = local1.pat.kind;
2120
if name.as_str() == "x";
21+
if let Some(trailing_expr) = &block.expr;
22+
if let ExprKind::Path(ref path) = trailing_expr.kind;
23+
if match_qpath(path, &["x"]);
2224
if let PatKind::Lit(ref lit_expr1) = arms[1].pat.kind
2325
if let ExprKind::Lit(ref lit4) = lit_expr1.kind;
2426
if let LitKind::Int(17, _) = lit4.node;

0 commit comments

Comments
 (0)