Skip to content

Commit 65ae666

Browse files
committed
Auto merge of #9595 - Alexendoo:author-let-chains, r=Jarcho
Replace if_chain with let chains in `clippy::author` output Should help nudge new contributors towards let chains changelog: none
2 parents 3690199 + 9e70a0f commit 65ae666

File tree

10 files changed

+387
-420
lines changed

10 files changed

+387
-420
lines changed

clippy_lints/src/utils/author.rs

Lines changed: 62 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_hir::{
1212
use rustc_lint::{LateContext, LateLintPass, LintContext};
1313
use rustc_session::{declare_lint_pass, declare_tool_lint};
1414
use rustc_span::symbol::{Ident, Symbol};
15+
use std::cell::Cell;
1516
use std::fmt::{Display, Formatter, Write as _};
1617

1718
declare_clippy_lint! {
@@ -37,15 +38,13 @@ declare_clippy_lint! {
3738
///
3839
/// ```rust,ignore
3940
/// // ./tests/ui/new_lint.stdout
40-
/// if_chain! {
41-
/// if let ExprKind::If(ref cond, ref then, None) = item.kind,
42-
/// if let ExprKind::Binary(BinOp::Eq, ref left, ref right) = cond.kind,
43-
/// if let ExprKind::Path(ref path) = left.kind,
44-
/// if let ExprKind::Lit(ref lit) = right.kind,
45-
/// if let LitKind::Int(42, _) = lit.node,
46-
/// then {
47-
/// // report your lint here
48-
/// }
41+
/// if ExprKind::If(ref cond, ref then, None) = item.kind
42+
/// && let ExprKind::Binary(BinOp::Eq, ref left, ref right) = cond.kind
43+
/// && let ExprKind::Path(ref path) = left.kind
44+
/// && let ExprKind::Lit(ref lit) = right.kind
45+
/// && let LitKind::Int(42, _) = lit.node
46+
/// {
47+
/// // report your lint here
4948
/// }
5049
/// ```
5150
pub LINT_AUTHOR,
@@ -91,15 +90,16 @@ macro_rules! field {
9190
};
9291
}
9392

94-
fn prelude() {
95-
println!("if_chain! {{");
96-
}
97-
98-
fn done() {
99-
println!(" then {{");
100-
println!(" // report your lint here");
101-
println!(" }}");
102-
println!("}}");
93+
/// Print a condition of a let chain, `chain!(self, "let Some(x) = y")` will print
94+
/// `if let Some(x) = y` on the first call and ` && let Some(x) = y` thereafter
95+
macro_rules! chain {
96+
($self:ident, $($t:tt)*) => {
97+
if $self.first.take() {
98+
println!("if {}", format_args!($($t)*));
99+
} else {
100+
println!(" && {}", format_args!($($t)*));
101+
}
102+
}
103103
}
104104

105105
impl<'tcx> LateLintPass<'tcx> for Author {
@@ -149,9 +149,10 @@ fn check_item(cx: &LateContext<'_>, hir_id: HirId) {
149149

150150
fn check_node(cx: &LateContext<'_>, hir_id: HirId, f: impl Fn(&PrintVisitor<'_, '_>)) {
151151
if has_attr(cx, hir_id) {
152-
prelude();
153152
f(&PrintVisitor::new(cx));
154-
done();
153+
println!("{{");
154+
println!(" // report your lint here");
155+
println!("}}");
155156
}
156157
}
157158

@@ -195,15 +196,18 @@ struct PrintVisitor<'a, 'tcx> {
195196
cx: &'a LateContext<'tcx>,
196197
/// Fields are the current index that needs to be appended to pattern
197198
/// binding names
198-
ids: std::cell::Cell<FxHashMap<&'static str, u32>>,
199+
ids: Cell<FxHashMap<&'static str, u32>>,
200+
/// Currently at the first condition in the if chain
201+
first: Cell<bool>,
199202
}
200203

201204
#[allow(clippy::unused_self)]
202205
impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
203206
fn new(cx: &'a LateContext<'tcx>) -> Self {
204207
Self {
205208
cx,
206-
ids: std::cell::Cell::default(),
209+
ids: Cell::default(),
210+
first: Cell::new(true),
207211
}
208212
}
209213

@@ -226,20 +230,20 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
226230

227231
fn option<T: Copy>(&self, option: &Binding<Option<T>>, name: &'static str, f: impl Fn(&Binding<T>)) {
228232
match option.value {
229-
None => out!("if {option}.is_none();"),
233+
None => chain!(self, "{option}.is_none()"),
230234
Some(value) => {
231235
let value = &self.bind(name, value);
232-
out!("if let Some({value}) = {option};");
236+
chain!(self, "let Some({value}) = {option}");
233237
f(value);
234238
},
235239
}
236240
}
237241

238242
fn slice<T>(&self, slice: &Binding<&[T]>, f: impl Fn(&Binding<&T>)) {
239243
if slice.value.is_empty() {
240-
out!("if {slice}.is_empty();");
244+
chain!(self, "{slice}.is_empty()");
241245
} else {
242-
out!("if {slice}.len() == {};", slice.value.len());
246+
chain!(self, "{slice}.len() == {}", slice.value.len());
243247
for (i, value) in slice.value.iter().enumerate() {
244248
let name = format!("{slice}[{i}]");
245249
f(&Binding { name, value });
@@ -254,23 +258,23 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
254258
}
255259

256260
fn ident(&self, ident: &Binding<Ident>) {
257-
out!("if {ident}.as_str() == {:?};", ident.value.as_str());
261+
chain!(self, "{ident}.as_str() == {:?}", ident.value.as_str());
258262
}
259263

260264
fn symbol(&self, symbol: &Binding<Symbol>) {
261-
out!("if {symbol}.as_str() == {:?};", symbol.value.as_str());
265+
chain!(self, "{symbol}.as_str() == {:?}", symbol.value.as_str());
262266
}
263267

264268
fn qpath(&self, qpath: &Binding<&QPath<'_>>) {
265269
if let QPath::LangItem(lang_item, ..) = *qpath.value {
266-
out!("if matches!({qpath}, QPath::LangItem(LangItem::{lang_item:?}, _));");
270+
chain!(self, "matches!({qpath}, QPath::LangItem(LangItem::{lang_item:?}, _))");
267271
} else {
268-
out!("if match_qpath({qpath}, &[{}]);", path_to_string(qpath.value));
272+
chain!(self, "match_qpath({qpath}, &[{}])", path_to_string(qpath.value));
269273
}
270274
}
271275

272276
fn lit(&self, lit: &Binding<&Lit>) {
273-
let kind = |kind| out!("if let LitKind::{kind} = {lit}.node;");
277+
let kind = |kind| chain!(self, "let LitKind::{kind} = {lit}.node");
274278
macro_rules! kind {
275279
($($t:tt)*) => (kind(format_args!($($t)*)));
276280
}
@@ -298,7 +302,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
298302
LitKind::ByteStr(ref vec) => {
299303
bind!(self, vec);
300304
kind!("ByteStr(ref {vec})");
301-
out!("if let [{:?}] = **{vec};", vec.value);
305+
chain!(self, "let [{:?}] = **{vec}", vec.value);
302306
},
303307
LitKind::Str(s, _) => {
304308
bind!(self, s);
@@ -311,15 +315,15 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
311315
fn arm(&self, arm: &Binding<&hir::Arm<'_>>) {
312316
self.pat(field!(arm.pat));
313317
match arm.value.guard {
314-
None => out!("if {arm}.guard.is_none();"),
318+
None => chain!(self, "{arm}.guard.is_none()"),
315319
Some(hir::Guard::If(expr)) => {
316320
bind!(self, expr);
317-
out!("if let Some(Guard::If({expr})) = {arm}.guard;");
321+
chain!(self, "let Some(Guard::If({expr})) = {arm}.guard");
318322
self.expr(expr);
319323
},
320324
Some(hir::Guard::IfLet(let_expr)) => {
321325
bind!(self, let_expr);
322-
out!("if let Some(Guard::IfLet({let_expr}) = {arm}.guard;");
326+
chain!(self, "let Some(Guard::IfLet({let_expr}) = {arm}.guard");
323327
self.pat(field!(let_expr.pat));
324328
self.expr(field!(let_expr.init));
325329
},
@@ -331,9 +335,10 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
331335
fn expr(&self, expr: &Binding<&hir::Expr<'_>>) {
332336
if let Some(higher::While { condition, body }) = higher::While::hir(expr.value) {
333337
bind!(self, condition, body);
334-
out!(
335-
"if let Some(higher::While {{ condition: {condition}, body: {body} }}) \
336-
= higher::While::hir({expr});"
338+
chain!(
339+
self,
340+
"let Some(higher::While {{ condition: {condition}, body: {body} }}) \
341+
= higher::While::hir({expr})"
337342
);
338343
self.expr(condition);
339344
self.expr(body);
@@ -347,9 +352,10 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
347352
}) = higher::WhileLet::hir(expr.value)
348353
{
349354
bind!(self, let_pat, let_expr, if_then);
350-
out!(
351-
"if let Some(higher::WhileLet {{ let_pat: {let_pat}, let_expr: {let_expr}, if_then: {if_then} }}) \
352-
= higher::WhileLet::hir({expr});"
355+
chain!(
356+
self,
357+
"let Some(higher::WhileLet {{ let_pat: {let_pat}, let_expr: {let_expr}, if_then: {if_then} }}) \
358+
= higher::WhileLet::hir({expr})"
353359
);
354360
self.pat(let_pat);
355361
self.expr(let_expr);
@@ -359,17 +365,18 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
359365

360366
if let Some(higher::ForLoop { pat, arg, body, .. }) = higher::ForLoop::hir(expr.value) {
361367
bind!(self, pat, arg, body);
362-
out!(
363-
"if let Some(higher::ForLoop {{ pat: {pat}, arg: {arg}, body: {body}, .. }}) \
364-
= higher::ForLoop::hir({expr});"
368+
chain!(
369+
self,
370+
"let Some(higher::ForLoop {{ pat: {pat}, arg: {arg}, body: {body}, .. }}) \
371+
= higher::ForLoop::hir({expr})"
365372
);
366373
self.pat(pat);
367374
self.expr(arg);
368375
self.expr(body);
369376
return;
370377
}
371378

372-
let kind = |kind| out!("if let ExprKind::{kind} = {expr}.kind;");
379+
let kind = |kind| chain!(self, "let ExprKind::{kind} = {expr}.kind");
373380
macro_rules! kind {
374381
($($t:tt)*) => (kind(format_args!($($t)*)));
375382
}
@@ -383,7 +390,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
383390
// if it's a path
384391
if let Some(TyKind::Path(ref qpath)) = let_expr.value.ty.as_ref().map(|ty| &ty.kind) {
385392
bind!(self, qpath);
386-
out!("if let TyKind::Path(ref {qpath}) = {let_expr}.ty.kind;");
393+
chain!(self, "let TyKind::Path(ref {qpath}) = {let_expr}.ty.kind");
387394
self.qpath(qpath);
388395
}
389396
self.expr(field!(let_expr.init));
@@ -419,7 +426,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
419426
ExprKind::Binary(op, left, right) => {
420427
bind!(self, op, left, right);
421428
kind!("Binary({op}, {left}, {right})");
422-
out!("if BinOpKind::{:?} == {op}.node;", op.value.node);
429+
chain!(self, "BinOpKind::{:?} == {op}.node", op.value.node);
423430
self.expr(left);
424431
self.expr(right);
425432
},
@@ -438,7 +445,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
438445
kind!("Cast({expr}, {cast_ty})");
439446
if let TyKind::Path(ref qpath) = cast_ty.value.kind {
440447
bind!(self, qpath);
441-
out!("if let TyKind::Path(ref {qpath}) = {cast_ty}.kind;");
448+
chain!(self, "let TyKind::Path(ref {qpath}) = {cast_ty}.kind");
442449
self.qpath(qpath);
443450
}
444451
self.expr(expr);
@@ -485,7 +492,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
485492

486493
bind!(self, fn_decl, body_id);
487494
kind!("Closure(CaptureBy::{capture_clause:?}, {fn_decl}, {body_id}, _, {movability})");
488-
out!("if let {ret_ty} = {fn_decl}.output;");
495+
chain!(self, "let {ret_ty} = {fn_decl}.output");
489496
self.body(body_id);
490497
},
491498
ExprKind::Yield(sub, source) => {
@@ -509,7 +516,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
509516
ExprKind::AssignOp(op, target, value) => {
510517
bind!(self, op, target, value);
511518
kind!("AssignOp({op}, {target}, {value})");
512-
out!("if BinOpKind::{:?} == {op}.node;", op.value.node);
519+
chain!(self, "BinOpKind::{:?} == {op}.node", op.value.node);
513520
self.expr(target);
514521
self.expr(value);
515522
},
@@ -573,10 +580,10 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
573580
kind!("Repeat({value}, {length})");
574581
self.expr(value);
575582
match length.value {
576-
ArrayLen::Infer(..) => out!("if let ArrayLen::Infer(..) = length;"),
583+
ArrayLen::Infer(..) => chain!(self, "let ArrayLen::Infer(..) = length"),
577584
ArrayLen::Body(anon_const) => {
578585
bind!(self, anon_const);
579-
out!("if let ArrayLen::Body({anon_const}) = {length};");
586+
chain!(self, "let ArrayLen::Body({anon_const}) = {length}");
580587
self.body(field!(anon_const.body));
581588
},
582589
}
@@ -600,12 +607,12 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
600607
fn body(&self, body_id: &Binding<hir::BodyId>) {
601608
let expr = self.cx.tcx.hir().body(body_id.value).value;
602609
bind!(self, expr);
603-
out!("let {expr} = &cx.tcx.hir().body({body_id}).value;");
610+
chain!(self, "{expr} = &cx.tcx.hir().body({body_id}).value");
604611
self.expr(expr);
605612
}
606613

607614
fn pat(&self, pat: &Binding<&hir::Pat<'_>>) {
608-
let kind = |kind| out!("if let PatKind::{kind} = {pat}.kind;");
615+
let kind = |kind| chain!(self, "let PatKind::{kind} = {pat}.kind");
609616
macro_rules! kind {
610617
($($t:tt)*) => (kind(format_args!($($t)*)));
611618
}
@@ -688,7 +695,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
688695
}
689696

690697
fn stmt(&self, stmt: &Binding<&hir::Stmt<'_>>) {
691-
let kind = |kind| out!("if let StmtKind::{kind} = {stmt}.kind;");
698+
let kind = |kind| chain!(self, "let StmtKind::{kind} = {stmt}.kind");
692699
macro_rules! kind {
693700
($($t:tt)*) => (kind(format_args!($($t)*)));
694701
}

tests/ui/author.stdout

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
if_chain! {
2-
if let StmtKind::Local(local) = stmt.kind;
3-
if let Some(init) = local.init;
4-
if let ExprKind::Cast(expr, cast_ty) = init.kind;
5-
if let TyKind::Path(ref qpath) = cast_ty.kind;
6-
if match_qpath(qpath, &["char"]);
7-
if let ExprKind::Lit(ref lit) = expr.kind;
8-
if let LitKind::Int(69, LitIntType::Unsuffixed) = lit.node;
9-
if let PatKind::Binding(BindingAnnotation::NONE, _, name, None) = local.pat.kind;
10-
if name.as_str() == "x";
11-
then {
12-
// report your lint here
13-
}
1+
if let StmtKind::Local(local) = stmt.kind
2+
&& let Some(init) = local.init
3+
&& let ExprKind::Cast(expr, cast_ty) = init.kind
4+
&& let TyKind::Path(ref qpath) = cast_ty.kind
5+
&& match_qpath(qpath, &["char"])
6+
&& let ExprKind::Lit(ref lit) = expr.kind
7+
&& let LitKind::Int(69, LitIntType::Unsuffixed) = lit.node
8+
&& let PatKind::Binding(BindingAnnotation::NONE, _, name, None) = local.pat.kind
9+
&& name.as_str() == "x"
10+
{
11+
// report your lint here
1412
}

0 commit comments

Comments
 (0)