Skip to content

Commit 2be6c4a

Browse files
borsbors
committed
Auto merge of #8762 - Jarcho:visitor, r=Jarcho
Replace `expr_visitor` with `for_each_expr` This is a minor change which uses `ControlFlow` rather than a boolean. This also runs the visitor rather than returning the visitor, which results in a small readability win as well. changelog: None
2 parents bef93d3 + 26bb366 commit 2be6c4a

19 files changed

+413
-595
lines changed

‎clippy_lints/src/blocks_in_if_conditions.rs‎

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ use clippy_utils::get_parent_expr;
33
use clippy_utils::higher;
44
use clippy_utils::source::snippet_block_with_applicability;
55
use clippy_utils::ty::implements_trait;
6+
use clippy_utils::visitors::{for_each_expr, Descend};
7+
use core::ops::ControlFlow;
68
use if_chain::if_chain;
79
use rustc_errors::Applicability;
8-
use rustc_hir::intravisit::{walk_expr, Visitor};
9-
use rustc_hir::{BlockCheckMode, Closure, Expr, ExprKind};
10+
use rustc_hir::{BlockCheckMode, Expr, ExprKind};
1011
use rustc_lint::{LateContext, LateLintPass, LintContext};
1112
use rustc_middle::lint::in_external_macro;
1213
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -44,39 +45,6 @@ declare_clippy_lint! {
4445

4546
declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]);
4647

47-
struct ExVisitor<'a, 'tcx> {
48-
found_block: Option<&'tcx Expr<'tcx>>,
49-
cx: &'a LateContext<'tcx>,
50-
}
51-
52-
impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
53-
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
54-
if let ExprKind::Closure(&Closure { body, .. }) = expr.kind {
55-
// do not lint if the closure is called using an iterator (see #1141)
56-
if_chain! {
57-
if let Some(parent) = get_parent_expr(self.cx, expr);
58-
if let ExprKind::MethodCall(_, self_arg, ..) = &parent.kind;
59-
let caller = self.cx.typeck_results().expr_ty(self_arg);
60-
if let Some(iter_id) = self.cx.tcx.get_diagnostic_item(sym::Iterator);
61-
if implements_trait(self.cx, caller, iter_id, &[]);
62-
then {
63-
return;
64-
}
65-
}
66-
67-
let body = self.cx.tcx.hir().body(body);
68-
let ex = &body.value;
69-
if let ExprKind::Block(block, _) = ex.kind {
70-
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
71-
self.found_block = Some(ex);
72-
return;
73-
}
74-
}
75-
}
76-
walk_expr(self, expr);
77-
}
78-
}
79-
8048
const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition";
8149
const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \
8250
instead, move the block or closure higher and bind it with a `let`";
@@ -144,11 +112,31 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions {
144112
}
145113
}
146114
} else {
147-
let mut visitor = ExVisitor { found_block: None, cx };
148-
walk_expr(&mut visitor, cond);
149-
if let Some(block) = visitor.found_block {
150-
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE);
151-
}
115+
let _: Option<!> = for_each_expr(cond, |e| {
116+
if let ExprKind::Closure(closure) = e.kind {
117+
// do not lint if the closure is called using an iterator (see #1141)
118+
if_chain! {
119+
if let Some(parent) = get_parent_expr(cx, e);
120+
if let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind;
121+
let caller = cx.typeck_results().expr_ty(self_arg);
122+
if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
123+
if implements_trait(cx, caller, iter_id, &[]);
124+
then {
125+
return ControlFlow::Continue(Descend::No);
126+
}
127+
}
128+
129+
let body = cx.tcx.hir().body(closure.body);
130+
let ex = &body.value;
131+
if let ExprKind::Block(block, _) = ex.kind {
132+
if !body.value.span.from_expansion() && !block.stmts.is_empty() {
133+
span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE);
134+
return ControlFlow::Continue(Descend::No);
135+
}
136+
}
137+
}
138+
ControlFlow::Continue(Descend::Yes)
139+
});
152140
}
153141
}
154142
}

‎clippy_lints/src/cognitive_complexity.rs‎

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
use clippy_utils::diagnostics::span_lint_and_help;
44
use clippy_utils::source::snippet_opt;
55
use clippy_utils::ty::is_type_diagnostic_item;
6+
use clippy_utils::visitors::for_each_expr;
67
use clippy_utils::LimitStack;
8+
use core::ops::ControlFlow;
79
use rustc_ast::ast::Attribute;
8-
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
9-
use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId};
10+
use rustc_hir::intravisit::FnKind;
11+
use rustc_hir::{Body, ExprKind, FnDecl, HirId};
1012
use rustc_lint::{LateContext, LateLintPass, LintContext};
1113
use rustc_session::{declare_tool_lint, impl_lint_pass};
1214
use rustc_span::source_map::Span;
@@ -61,11 +63,27 @@ impl CognitiveComplexity {
6163
return;
6264
}
6365

64-
let expr = &body.value;
66+
let expr = body.value;
67+
68+
let mut cc = 1u64;
69+
let mut returns = 0u64;
70+
let _: Option<!> = for_each_expr(expr, |e| {
71+
match e.kind {
72+
ExprKind::If(_, _, _) => {
73+
cc += 1;
74+
},
75+
ExprKind::Match(_, arms, _) => {
76+
if arms.len() > 1 {
77+
cc += 1;
78+
}
79+
cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
80+
},
81+
ExprKind::Ret(_) => returns += 1,
82+
_ => {},
83+
}
84+
ControlFlow::Continue(())
85+
});
6586

66-
let mut helper = CcHelper { cc: 1, returns: 0 };
67-
helper.visit_expr(expr);
68-
let CcHelper { cc, returns } = helper;
6987
let ret_ty = cx.typeck_results().node_type(expr.hir_id);
7088
let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) {
7189
returns
@@ -74,13 +92,12 @@ impl CognitiveComplexity {
7492
(returns / 2)
7593
};
7694

77-
let mut rust_cc = cc;
7895
// prevent degenerate cases where unreachable code contains `return` statements
79-
if rust_cc >= ret_adjust {
80-
rust_cc -= ret_adjust;
96+
if cc >= ret_adjust {
97+
cc -= ret_adjust;
8198
}
8299

83-
if rust_cc > self.limit.limit() {
100+
if cc > self.limit.limit() {
84101
let fn_span = match kind {
85102
FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span,
86103
FnKind::Closure => {
@@ -107,7 +124,7 @@ impl CognitiveComplexity {
107124
COGNITIVE_COMPLEXITY,
108125
fn_span,
109126
&format!(
110-
"the function has a cognitive complexity of ({rust_cc}/{})",
127+
"the function has a cognitive complexity of ({cc}/{})",
111128
self.limit.limit()
112129
),
113130
None,
@@ -140,27 +157,3 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity {
140157
self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity");
141158
}
142159
}
143-
144-
struct CcHelper {
145-
cc: u64,
146-
returns: u64,
147-
}
148-
149-
impl<'tcx> Visitor<'tcx> for CcHelper {
150-
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
151-
walk_expr(self, e);
152-
match e.kind {
153-
ExprKind::If(_, _, _) => {
154-
self.cc += 1;
155-
},
156-
ExprKind::Match(_, arms, _) => {
157-
if arms.len() > 1 {
158-
self.cc += 1;
159-
}
160-
self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
161-
},
162-
ExprKind::Ret(_) => self.returns += 1,
163-
_ => {},
164-
}
165-
}
166-
}

‎clippy_lints/src/functions/must_use.rs‎

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use rustc_ast::ast::Attribute;
22
use rustc_errors::Applicability;
33
use rustc_hir::def_id::{DefIdSet, LocalDefId};
4-
use rustc_hir::{self as hir, def::Res, intravisit, QPath};
4+
use rustc_hir::{self as hir, def::Res, QPath};
55
use rustc_lint::{LateContext, LintContext};
66
use rustc_middle::{
77
lint::in_external_macro,
@@ -13,8 +13,11 @@ use clippy_utils::attrs::is_proc_macro;
1313
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then};
1414
use clippy_utils::source::snippet_opt;
1515
use clippy_utils::ty::is_must_use_ty;
16+
use clippy_utils::visitors::for_each_expr;
1617
use clippy_utils::{match_def_path, return_ty, trait_ref_of_method};
1718

19+
use core::ops::ControlFlow;
20+
1821
use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT};
1922

2023
pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
@@ -200,79 +203,65 @@ fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &m
200203
}
201204
}
202205

203-
struct StaticMutVisitor<'a, 'tcx> {
204-
cx: &'a LateContext<'tcx>,
205-
mutates_static: bool,
206+
fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
207+
use hir::ExprKind::{Field, Index, Path};
208+
209+
match e.kind {
210+
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
211+
Path(_) => true,
212+
Field(inner, _) | Index(inner, _) => is_mutated_static(inner),
213+
_ => false,
214+
}
206215
}
207216

208-
impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> {
209-
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
217+
fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
218+
for_each_expr(body.value, |e| {
210219
use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall};
211220

212-
if self.mutates_static {
213-
return;
214-
}
215-
match expr.kind {
221+
match e.kind {
216222
Call(_, args) => {
217223
let mut tys = DefIdSet::default();
218224
for arg in args {
219-
if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
225+
if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
220226
&& is_mutable_ty(
221-
self.cx,
222-
self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
227+
cx,
228+
cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
223229
arg.span,
224230
&mut tys,
225231
)
226232
&& is_mutated_static(arg)
227233
{
228-
self.mutates_static = true;
229-
return;
234+
return ControlFlow::Break(());
230235
}
231236
tys.clear();
232237
}
238+
ControlFlow::Continue(())
233239
},
234240
MethodCall(_, receiver, args, _) => {
235241
let mut tys = DefIdSet::default();
236242
for arg in std::iter::once(receiver).chain(args.iter()) {
237-
if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
243+
if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id())
238244
&& is_mutable_ty(
239-
self.cx,
240-
self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
245+
cx,
246+
cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg),
241247
arg.span,
242248
&mut tys,
243249
)
244250
&& is_mutated_static(arg)
245251
{
246-
self.mutates_static = true;
247-
return;
252+
return ControlFlow::Break(());
248253
}
249254
tys.clear();
250255
}
256+
ControlFlow::Continue(())
251257
},
252-
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => {
253-
self.mutates_static |= is_mutated_static(target);
258+
Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target)
259+
if is_mutated_static(target) =>
260+
{
261+
ControlFlow::Break(())
254262
},
255-
_ => {},
263+
_ => ControlFlow::Continue(()),
256264
}
257-
}
258-
}
259-
260-
fn is_mutated_static(e: &hir::Expr<'_>) -> bool {
261-
use hir::ExprKind::{Field, Index, Path};
262-
263-
match e.kind {
264-
Path(QPath::Resolved(_, path)) => !matches!(path.res, Res::Local(_)),
265-
Path(_) => true,
266-
Field(inner, _) | Index(inner, _) => is_mutated_static(inner),
267-
_ => false,
268-
}
269-
}
270-
271-
fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool {
272-
let mut v = StaticMutVisitor {
273-
cx,
274-
mutates_static: false,
275-
};
276-
intravisit::walk_expr(&mut v, body.value);
277-
v.mutates_static
265+
})
266+
.is_some()
278267
}

0 commit comments

Comments
 (0)