Skip to content

Commit 8cf7d9b

Browse files
borsbors
committed
Auto merge of rust-lang#7010 - camsteffen:if-chain-lint, r=llogiq
Internal `if_chain!` lints changelog: none We use `if_chain!` a lot. So this enforces some style rules around it, internal only. Lints when... * Nested `if`/`if_chain!` can be collapsed * An `if_chain!` starts with `let` or ends with `let ..; then {..}` * An `if_chain!` has only one `if` * An `if_chain!` contains `if .. && ..;` that spans multiple lines
2 parents 459bca8 + 827d6aa commit 8cf7d9b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+822
-537
lines changed

‎clippy_lints/src/attrs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,8 +371,8 @@ fn extract_clippy_lint(lint: &NestedMetaItem) -> Option<SymbolStr> {
371371
if meta_item.path.segments.len() > 1;
372372
if let tool_name = meta_item.path.segments[0].ident;
373373
if tool_name.name == sym::clippy;
374-
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
375374
then {
375+
let lint_name = meta_item.path.segments.last().unwrap().ident.name;
376376
return Some(lint_name.as_str());
377377
}
378378
}

‎clippy_lints/src/bytecount.rs

Lines changed: 39 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -45,52 +45,48 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
4545
if filter.ident.name == sym!(filter);
4646
if filter_args.len() == 2;
4747
if let ExprKind::Closure(_, _, body_id, _, _) = filter_args[1].kind;
48+
let body = cx.tcx.hir().body(body_id);
49+
if body.params.len() == 1;
50+
if let Some(argname) = get_pat_name(&body.params[0].pat);
51+
if let ExprKind::Binary(ref op, ref l, ref r) = body.value.kind;
52+
if op.node == BinOpKind::Eq;
53+
if match_type(cx,
54+
cx.typeck_results().expr_ty(&filter_args[0]).peel_refs(),
55+
&paths::SLICE_ITER);
4856
then {
49-
let body = cx.tcx.hir().body(body_id);
50-
if_chain! {
51-
if body.params.len() == 1;
52-
if let Some(argname) = get_pat_name(&body.params[0].pat);
53-
if let ExprKind::Binary(ref op, ref l, ref r) = body.value.kind;
54-
if op.node == BinOpKind::Eq;
55-
if match_type(cx,
56-
cx.typeck_results().expr_ty(&filter_args[0]).peel_refs(),
57-
&paths::SLICE_ITER);
58-
then {
59-
let needle = match get_path_name(l) {
60-
Some(name) if check_arg(name, argname, r) => r,
61-
_ => match get_path_name(r) {
62-
Some(name) if check_arg(name, argname, l) => l,
63-
_ => { return; }
64-
}
65-
};
66-
if ty::Uint(UintTy::U8) != *cx.typeck_results().expr_ty(needle).peel_refs().kind() {
67-
return;
68-
}
69-
let haystack = if let ExprKind::MethodCall(ref path, _, ref args, _) =
70-
filter_args[0].kind {
71-
let p = path.ident.name;
72-
if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 {
73-
&args[0]
74-
} else {
75-
&filter_args[0]
76-
}
77-
} else {
78-
&filter_args[0]
79-
};
80-
let mut applicability = Applicability::MaybeIncorrect;
81-
span_lint_and_sugg(
82-
cx,
83-
NAIVE_BYTECOUNT,
84-
expr.span,
85-
"you appear to be counting bytes the naive way",
86-
"consider using the bytecount crate",
87-
format!("bytecount::count({}, {})",
88-
snippet_with_applicability(cx, haystack.span, "..", &mut applicability),
89-
snippet_with_applicability(cx, needle.span, "..", &mut applicability)),
90-
applicability,
91-
);
57+
let needle = match get_path_name(l) {
58+
Some(name) if check_arg(name, argname, r) => r,
59+
_ => match get_path_name(r) {
60+
Some(name) if check_arg(name, argname, l) => l,
61+
_ => { return; }
9262
}
9363
};
64+
if ty::Uint(UintTy::U8) != *cx.typeck_results().expr_ty(needle).peel_refs().kind() {
65+
return;
66+
}
67+
let haystack = if let ExprKind::MethodCall(ref path, _, ref args, _) =
68+
filter_args[0].kind {
69+
let p = path.ident.name;
70+
if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 {
71+
&args[0]
72+
} else {
73+
&filter_args[0]
74+
}
75+
} else {
76+
&filter_args[0]
77+
};
78+
let mut applicability = Applicability::MaybeIncorrect;
79+
span_lint_and_sugg(
80+
cx,
81+
NAIVE_BYTECOUNT,
82+
expr.span,
83+
"you appear to be counting bytes the naive way",
84+
"consider using the bytecount crate",
85+
format!("bytecount::count({}, {})",
86+
snippet_with_applicability(cx, haystack.span, "..", &mut applicability),
87+
snippet_with_applicability(cx, needle.span, "..", &mut applicability)),
88+
applicability,
89+
);
9490
}
9591
};
9692
}

‎clippy_lints/src/checked_conversions.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,8 @@ fn get_implementing_type<'a>(path: &QPath<'_>, candidates: &'a [&str], function:
321321
if path.ident.name.as_str() == function;
322322
if let TyKind::Path(QPath::Resolved(None, ref tp)) = &ty.kind;
323323
if let [int] = &*tp.segments;
324-
let name = &int.ident.name.as_str();
325-
326324
then {
325+
let name = &int.ident.name.as_str();
327326
candidates.iter().find(|c| name == *c).cloned()
328327
} else {
329328
None
@@ -336,9 +335,8 @@ fn int_ty_to_sym<'tcx>(path: &QPath<'_>) -> Option<&'tcx str> {
336335
if_chain! {
337336
if let QPath::Resolved(_, ref path) = *path;
338337
if let [ty] = &*path.segments;
339-
let name = &ty.ident.name.as_str();
340-
341338
then {
339+
let name = &ty.ident.name.as_str();
342340
INTS.iter().find(|c| name == *c).cloned()
343341
} else {
344342
None

‎clippy_lints/src/collapsible_match.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ impl<'tcx> LateLintPass<'tcx> for CollapsibleMatch {
6262
}
6363

6464
fn check_arm<'tcx>(arm: &Arm<'tcx>, wild_outer_arm: &Arm<'tcx>, cx: &LateContext<'tcx>) {
65+
let expr = strip_singleton_blocks(arm.body);
6566
if_chain! {
66-
let expr = strip_singleton_blocks(arm.body);
6767
if let ExprKind::Match(expr_in, arms_inner, _) = expr.kind;
6868
// the outer arm pattern and the inner match
6969
if expr_in.span.ctxt() == arm.pat.span.ctxt();

‎clippy_lints/src/default.rs

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -84,22 +84,21 @@ impl LateLintPass<'_> for Default {
8484
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
8585
// Detect and ignore <Foo as Default>::default() because these calls do explicitly name the type.
8686
if let QPath::Resolved(None, _path) = qpath;
87+
let expr_ty = cx.typeck_results().expr_ty(expr);
88+
if let ty::Adt(def, ..) = expr_ty.kind();
8789
then {
88-
let expr_ty = cx.typeck_results().expr_ty(expr);
89-
if let ty::Adt(def, ..) = expr_ty.kind() {
90-
// TODO: Work out a way to put "whatever the imported way of referencing
91-
// this type in this file" rather than a fully-qualified type.
92-
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did));
93-
span_lint_and_sugg(
94-
cx,
95-
DEFAULT_TRAIT_ACCESS,
96-
expr.span,
97-
&format!("calling `{}` is more clear than this expression", replacement),
98-
"try",
99-
replacement,
100-
Applicability::Unspecified, // First resolve the TODO above
101-
);
102-
}
90+
// TODO: Work out a way to put "whatever the imported way of referencing
91+
// this type in this file" rather than a fully-qualified type.
92+
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did));
93+
span_lint_and_sugg(
94+
cx,
95+
DEFAULT_TRAIT_ACCESS,
96+
expr.span,
97+
&format!("calling `{}` is more clear than this expression", replacement),
98+
"try",
99+
replacement,
100+
Applicability::Unspecified, // First resolve the TODO above
101+
);
103102
}
104103
}
105104
}
@@ -202,14 +201,14 @@ impl LateLintPass<'_> for Default {
202201
let binding_type = if_chain! {
203202
if let ty::Adt(adt_def, substs) = binding_type.kind();
204203
if !substs.is_empty();
205-
let adt_def_ty_name = cx.tcx.item_name(adt_def.did);
206-
let generic_args = substs.iter().collect::<Vec<_>>();
207-
let tys_str = generic_args
208-
.iter()
209-
.map(ToString::to_string)
210-
.collect::<Vec<_>>()
211-
.join(", ");
212204
then {
205+
let adt_def_ty_name = cx.tcx.item_name(adt_def.did);
206+
let generic_args = substs.iter().collect::<Vec<_>>();
207+
let tys_str = generic_args
208+
.iter()
209+
.map(ToString::to_string)
210+
.collect::<Vec<_>>()
211+
.join(", ");
213212
format!("{}::<{}>", adt_def_ty_name, &tys_str)
214213
} else {
215214
binding_type.to_string()

‎clippy_lints/src/default_numeric_fallback.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
130130
},
131131

132132
ExprKind::Struct(_, fields, base) => {
133+
let ty = self.cx.typeck_results().expr_ty(expr);
133134
if_chain! {
134-
let ty = self.cx.typeck_results().expr_ty(expr);
135135
if let Some(adt_def) = ty.ty_adt_def();
136136
if adt_def.is_struct();
137137
if let Some(variant) = adt_def.variants.iter().next();

‎clippy_lints/src/exit.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,14 @@ impl<'tcx> LateLintPass<'tcx> for Exit {
3232
if let ExprKind::Path(ref path) = path_expr.kind;
3333
if let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id();
3434
if match_def_path(cx, def_id, &paths::EXIT);
35+
let parent = cx.tcx.hir().get_parent_item(e.hir_id);
36+
if let Some(Node::Item(Item{kind: ItemKind::Fn(..), ..})) = cx.tcx.hir().find(parent);
37+
// If the next item up is a function we check if it is an entry point
38+
// and only then emit a linter warning
39+
let def_id = cx.tcx.hir().local_def_id(parent);
40+
if !is_entrypoint_fn(cx, def_id.to_def_id());
3541
then {
36-
let parent = cx.tcx.hir().get_parent_item(e.hir_id);
37-
if let Some(Node::Item(Item{kind: ItemKind::Fn(..), ..})) = cx.tcx.hir().find(parent) {
38-
// If the next item up is a function we check if it is an entry point
39-
// and only then emit a linter warning
40-
let def_id = cx.tcx.hir().local_def_id(parent);
41-
if !is_entrypoint_fn(cx, def_id.to_def_id()) {
42-
span_lint(cx, EXIT, e.span, "usage of `process::exit`");
43-
}
44-
}
42+
span_lint(cx, EXIT, e.span, "usage of `process::exit`");
4543
}
4644
}
4745
}

‎clippy_lints/src/float_literal.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ declare_lint_pass!(FloatLiteral => [EXCESSIVE_PRECISION, LOSSY_FLOAT_LITERAL]);
6161

6262
impl<'tcx> LateLintPass<'tcx> for FloatLiteral {
6363
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
64+
let ty = cx.typeck_results().expr_ty(expr);
6465
if_chain! {
65-
let ty = cx.typeck_results().expr_ty(expr);
6666
if let ty::Float(fty) = *ty.kind();
6767
if let hir::ExprKind::Lit(ref lit) = expr.kind;
6868
if let LitKind::Float(sym, lit_float_ty) = lit.node;

‎clippy_lints/src/formatting.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,8 @@ fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
217217
if let Some(else_snippet) = snippet_opt(cx, else_span);
218218
if let Some(else_pos) = else_snippet.find("else");
219219
if else_snippet[else_pos..].contains('\n');
220-
let else_desc = if is_if(else_) { "if" } else { "{..}" };
221-
222220
then {
221+
let else_desc = if is_if(else_) { "if" } else { "{..}" };
223222
span_lint_and_note(
224223
cx,
225224
SUSPICIOUS_ELSE_FORMATTING,

‎clippy_lints/src/if_let_mutex.rs

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,10 @@ impl<'tcx> Visitor<'tcx> for OppVisitor<'_, 'tcx> {
9494
type Map = Map<'tcx>;
9595

9696
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
97-
if_chain! {
98-
if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
99-
then {
100-
self.found_mutex = Some(mutex);
101-
self.mutex_lock_called = true;
102-
return;
103-
}
97+
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
98+
self.found_mutex = Some(mutex);
99+
self.mutex_lock_called = true;
100+
return;
104101
}
105102
visit::walk_expr(self, expr);
106103
}
@@ -121,13 +118,10 @@ impl<'tcx> Visitor<'tcx> for ArmVisitor<'_, 'tcx> {
121118
type Map = Map<'tcx>;
122119

123120
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
124-
if_chain! {
125-
if let Some(mutex) = is_mutex_lock_call(self.cx, expr);
126-
then {
127-
self.found_mutex = Some(mutex);
128-
self.mutex_lock_called = true;
129-
return;
130-
}
121+
if let Some(mutex) = is_mutex_lock_call(self.cx, expr) {
122+
self.found_mutex = Some(mutex);
123+
self.mutex_lock_called = true;
124+
return;
131125
}
132126
visit::walk_expr(self, expr);
133127
}

‎clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
550550
#[cfg(feature = "internal-lints")]
551551
&utils::internal_lints::DEFAULT_LINT,
552552
#[cfg(feature = "internal-lints")]
553+
&utils::internal_lints::IF_CHAIN_STYLE,
554+
#[cfg(feature = "internal-lints")]
553555
&utils::internal_lints::INTERNING_DEFINED_SYMBOL,
554556
#[cfg(feature = "internal-lints")]
555557
&utils::internal_lints::INVALID_PATHS,
@@ -1026,6 +1028,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10261028
store.register_late_pass(|| box utils::inspector::DeepCodeInspector);
10271029
store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
10281030
store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
1031+
store.register_late_pass(|| box utils::internal_lints::IfChainStyle);
10291032
store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
10301033
store.register_late_pass(|| box utils::internal_lints::InterningDefinedSymbol::default());
10311034
store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
@@ -1442,6 +1445,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
14421445
LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS),
14431446
LintId::of(&utils::internal_lints::COMPILER_LINT_FUNCTIONS),
14441447
LintId::of(&utils::internal_lints::DEFAULT_LINT),
1448+
LintId::of(&utils::internal_lints::IF_CHAIN_STYLE),
14451449
LintId::of(&utils::internal_lints::INTERNING_DEFINED_SYMBOL),
14461450
LintId::of(&utils::internal_lints::INVALID_PATHS),
14471451
LintId::of(&utils::internal_lints::LINT_WITHOUT_LINT_PASS),

‎clippy_lints/src/loops/manual_flatten.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ pub(super) fn check<'tcx>(
4646
let some_ctor = is_some_ctor(cx, path.res);
4747
let ok_ctor = is_ok_ctor(cx, path.res);
4848
if some_ctor || ok_ctor;
49-
let if_let_type = if some_ctor { "Some" } else { "Ok" };
50-
5149
then {
50+
let if_let_type = if some_ctor { "Some" } else { "Ok" };
5251
// Prepare the error message
5352
let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
5453

‎clippy_lints/src/loops/manual_memcpy.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ pub(super) fn check<'tcx>(
6161
if_chain! {
6262
if let ExprKind::Index(base_left, idx_left) = lhs.kind;
6363
if let ExprKind::Index(base_right, idx_right) = rhs.kind;
64-
if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
65-
&& is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
64+
if is_slice_like(cx, cx.typeck_results().expr_ty(base_left));
65+
if is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
6666
if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts);
6767
if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts);
6868

0 commit comments

Comments
 (0)