Skip to content

Commit 90c3d60

Browse files
committed
add excessive_nesting
1 parent acb85c3 commit 90c3d60

File tree

17 files changed

+892
-215
lines changed

17 files changed

+892
-215
lines changed

CHANGELOG.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4630,9 +4630,8 @@ Released 2018-09-13
46304630
[`erasing_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op
46314631
[`err_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#err_expect
46324632
[`eval_order_dependence`]: https://rust-lang.github.io/rust-clippy/master/index.html#eval_order_dependence
4633-
[`excessive_indentation`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_indentation
4633+
[`excessive_nesting`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_nesting
46344634
[`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision
4635-
[`excessive_width`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_width
46364635
[`exhaustive_enums`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_enums
46374636
[`exhaustive_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#exhaustive_structs
46384637
[`exit`]: https://rust-lang.github.io/rust-clippy/master/index.html#exit

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
154154
crate::eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS_INFO,
155155
crate::excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS_INFO,
156156
crate::excessive_bools::STRUCT_EXCESSIVE_BOOLS_INFO,
157-
crate::excessive_width::EXCESSIVE_INDENTATION_INFO,
158-
crate::excessive_width::EXCESSIVE_WIDTH_INFO,
157+
crate::excessive_nesting::EXCESSIVE_NESTING_INFO,
159158
crate::exhaustive_items::EXHAUSTIVE_ENUMS_INFO,
160159
crate::exhaustive_items::EXHAUSTIVE_STRUCTS_INFO,
161160
crate::exit::EXIT_INFO,

clippy_lints/src/excessive_nesting.rs

Lines changed: 324 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,324 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use rustc_ast::{
3+
node_id::NodeId,
4+
ptr::P,
5+
visit::{FnKind, Visitor},
6+
Arm, AssocItemKind, Block, Expr, ExprKind, Inline, Item, ItemKind, Local, LocalKind, ModKind, ModSpans, Pat,
7+
PatKind, Stmt, StmtKind,
8+
};
9+
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
10+
use rustc_middle::lint::in_external_macro;
11+
use rustc_session::{declare_tool_lint, impl_lint_pass};
12+
use rustc_span::Span;
13+
use thin_vec::ThinVec;
14+
15+
declare_clippy_lint! {
16+
/// ### What it does
17+
///
18+
/// Checks for blocks which are indented beyond a certain threshold.
19+
///
20+
/// ### Why is this bad?
21+
///
22+
/// It can severely hinder readability. The default is very generous; if you
23+
/// exceed this, it's a sign you should refactor.
24+
///
25+
/// ### Known issues
26+
///
27+
/// Nested inline modules will all be linted, rather than just the outermost one
28+
/// that applies. This makes the output a bit verbose.
29+
///
30+
/// ### Example
31+
/// An example clippy.toml configuration:
32+
/// ```toml
33+
/// # clippy.toml
34+
/// excessive-nesting-threshold = 3
35+
/// ```
36+
/// lib.rs:
37+
/// ```rust,ignore
38+
/// pub mod a {
39+
/// pub struct X;
40+
/// impl X {
41+
/// pub fn run(&self) {
42+
/// if true {
43+
/// // etc...
44+
/// }
45+
/// }
46+
/// }
47+
/// }
48+
/// Use instead:
49+
/// a.rs:
50+
/// ```rust,ignore
51+
/// fn private_run(x: &X) {
52+
/// if true {
53+
/// // etc...
54+
/// }
55+
/// }
56+
///
57+
/// pub struct X;
58+
/// impl X {
59+
/// pub fn run(&self) {
60+
/// private_run(self);
61+
/// }
62+
/// }
63+
#[clippy::version = "1.70.0"]
64+
pub EXCESSIVE_NESTING,
65+
restriction,
66+
"checks for blocks nested beyond a certain threshold"
67+
}
68+
impl_lint_pass!(ExcessiveNesting => [EXCESSIVE_NESTING]);
69+
70+
#[derive(Clone, Copy)]
71+
pub struct ExcessiveNesting {
72+
pub excessive_nesting_threshold: u64,
73+
}
74+
75+
impl EarlyLintPass for ExcessiveNesting {
76+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
77+
let conf = self;
78+
NestingVisitor {
79+
conf,
80+
cx,
81+
nest_level: 0,
82+
}
83+
.visit_item(item);
84+
}
85+
}
86+
87+
struct NestingVisitor<'conf, 'cx> {
88+
conf: &'conf ExcessiveNesting,
89+
cx: &'cx EarlyContext<'cx>,
90+
nest_level: u64,
91+
}
92+
93+
impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
94+
fn visit_local(&mut self, local: &Local) {
95+
self.visit_pat(&local.pat);
96+
97+
match &local.kind {
98+
LocalKind::Init(expr) => self.visit_expr(expr),
99+
LocalKind::InitElse(expr, block) => {
100+
self.visit_expr(expr);
101+
self.visit_block(block);
102+
},
103+
LocalKind::Decl => (),
104+
}
105+
}
106+
107+
fn visit_block(&mut self, block: &Block) {
108+
self.nest_level += 1;
109+
110+
if !check_indent(self, block.span) {
111+
for stmt in &block.stmts {
112+
self.visit_stmt(stmt);
113+
}
114+
}
115+
116+
self.nest_level -= 1;
117+
}
118+
119+
fn visit_stmt(&mut self, stmt: &Stmt) {
120+
match &stmt.kind {
121+
StmtKind::Local(local) => self.visit_local(local),
122+
StmtKind::Item(item) => self.visit_item(item),
123+
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(expr),
124+
_ => (),
125+
}
126+
}
127+
128+
fn visit_arm(&mut self, arm: &Arm) {
129+
self.visit_pat(&arm.pat);
130+
if let Some(expr) = &arm.guard {
131+
self.visit_expr(expr);
132+
}
133+
self.visit_expr(&arm.body);
134+
}
135+
136+
// TODO: Is this necessary?
137+
fn visit_pat(&mut self, pat: &Pat) {
138+
match &pat.kind {
139+
PatKind::Box(pat) | PatKind::Ref(pat, ..) | PatKind::Paren(pat) => self.visit_pat(pat),
140+
PatKind::Lit(expr) => self.visit_expr(expr),
141+
PatKind::Range(start, end, ..) => {
142+
if let Some(expr) = start {
143+
self.visit_expr(expr);
144+
}
145+
if let Some(expr) = end {
146+
self.visit_expr(expr);
147+
}
148+
},
149+
PatKind::Ident(.., pat) if let Some(pat) = pat => {
150+
self.visit_pat(pat);
151+
},
152+
PatKind::Struct(.., pat_fields, _) => {
153+
for pat_field in pat_fields {
154+
self.visit_pat(&pat_field.pat);
155+
}
156+
},
157+
PatKind::TupleStruct(.., pats) | PatKind::Or(pats) | PatKind::Tuple(pats) | PatKind::Slice(pats) => {
158+
for pat in pats {
159+
self.visit_pat(pat);
160+
}
161+
},
162+
_ => (),
163+
}
164+
}
165+
166+
fn visit_expr(&mut self, expr: &Expr) {
167+
// This is a mess, but really all it does is extract every expression from every applicable variant
168+
// of ExprKind until it finds a Block.
169+
match &expr.kind {
170+
ExprKind::ConstBlock(anon_const) => self.visit_expr(&anon_const.value),
171+
ExprKind::Call(.., args) => {
172+
for expr in args {
173+
self.visit_expr(expr);
174+
}
175+
},
176+
ExprKind::MethodCall(method_call) => {
177+
for expr in &method_call.args {
178+
self.visit_expr(expr);
179+
}
180+
},
181+
ExprKind::Tup(exprs) | ExprKind::Array(exprs) => {
182+
for expr in exprs {
183+
self.visit_expr(expr);
184+
}
185+
},
186+
ExprKind::Binary(.., left, right)
187+
| ExprKind::Assign(left, right, ..)
188+
| ExprKind::AssignOp(.., left, right)
189+
| ExprKind::Index(left, right) => {
190+
self.visit_expr(left);
191+
self.visit_expr(right);
192+
},
193+
ExprKind::Let(pat, expr, ..) => {
194+
self.visit_pat(pat);
195+
self.visit_expr(expr);
196+
},
197+
ExprKind::Unary(.., expr)
198+
| ExprKind::Await(expr)
199+
| ExprKind::Field(expr, ..)
200+
| ExprKind::AddrOf(.., expr)
201+
| ExprKind::Try(expr) => {
202+
self.visit_expr(expr);
203+
},
204+
ExprKind::Repeat(expr, anon_const) => {
205+
self.visit_expr(expr);
206+
self.visit_expr(&anon_const.value);
207+
},
208+
ExprKind::If(expr, block, else_expr) => {
209+
self.visit_expr(expr);
210+
self.visit_block(block);
211+
212+
if let Some(expr) = else_expr {
213+
self.visit_expr(expr);
214+
}
215+
},
216+
ExprKind::While(expr, block, ..) => {
217+
self.visit_expr(expr);
218+
self.visit_block(block);
219+
},
220+
ExprKind::ForLoop(pat, expr, block, ..) => {
221+
self.visit_pat(pat);
222+
self.visit_expr(expr);
223+
self.visit_block(block);
224+
},
225+
ExprKind::Loop(block, ..)
226+
| ExprKind::Block(block, ..)
227+
| ExprKind::Async(.., block)
228+
| ExprKind::TryBlock(block) => {
229+
self.visit_block(block);
230+
},
231+
ExprKind::Match(expr, arms) => {
232+
self.visit_expr(expr);
233+
234+
for arm in arms {
235+
self.visit_arm(arm);
236+
}
237+
},
238+
ExprKind::Closure(closure) => self.visit_expr(&closure.body),
239+
ExprKind::Range(start, end, ..) => {
240+
if let Some(expr) = start {
241+
self.visit_expr(expr);
242+
}
243+
if let Some(expr) = end {
244+
self.visit_expr(expr);
245+
}
246+
},
247+
ExprKind::Break(.., expr) | ExprKind::Ret(expr) | ExprKind::Yield(expr) | ExprKind::Yeet(expr) => {
248+
if let Some(expr) = expr {
249+
self.visit_expr(expr);
250+
}
251+
},
252+
ExprKind::Struct(struct_expr) => {
253+
for field in &struct_expr.fields {
254+
self.visit_expr(&field.expr);
255+
}
256+
},
257+
_ => (),
258+
}
259+
}
260+
261+
fn visit_fn(&mut self, fk: FnKind<'_>, _: Span, _: NodeId) {
262+
match fk {
263+
FnKind::Fn(.., block) if let Some(block) = block => self.visit_block(block),
264+
FnKind::Closure(.., expr) => self.visit_expr(expr),
265+
// :/
266+
FnKind::Fn(..) => (),
267+
}
268+
}
269+
270+
fn visit_item(&mut self, item: &Item) {
271+
match &item.kind {
272+
ItemKind::Static(static_item) if let Some(expr) = static_item.expr.as_ref() => self.visit_expr(expr),
273+
ItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => self.visit_expr(expr),
274+
ItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => self.visit_block(block),
275+
ItemKind::Mod(.., mod_kind)
276+
if let ModKind::Loaded(items, Inline::Yes, ModSpans { inner_span, ..}) = mod_kind =>
277+
{
278+
self.nest_level += 1;
279+
280+
check_indent(self, *inner_span);
281+
282+
self.nest_level -= 1;
283+
}
284+
ItemKind::Trait(trit) => check_trait_and_impl(self, item, &trit.items),
285+
ItemKind::Impl(imp) => check_trait_and_impl(self, item, &imp.items),
286+
_ => (),
287+
}
288+
}
289+
}
290+
291+
fn check_trait_and_impl(visitor: &mut NestingVisitor<'_, '_>, item: &Item, items: &ThinVec<P<Item<AssocItemKind>>>) {
292+
visitor.nest_level += 1;
293+
294+
if !check_indent(visitor, item.span) {
295+
for item in items {
296+
match &item.kind {
297+
AssocItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => {
298+
visitor.visit_expr(expr);
299+
},
300+
AssocItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => visitor.visit_block(block),
301+
_ => (),
302+
}
303+
}
304+
}
305+
306+
visitor.nest_level -= 1;
307+
}
308+
309+
fn check_indent(visitor: &NestingVisitor<'_, '_>, span: Span) -> bool {
310+
if visitor.nest_level > visitor.conf.excessive_nesting_threshold && !in_external_macro(visitor.cx.sess(), span) {
311+
span_lint_and_help(
312+
visitor.cx,
313+
EXCESSIVE_NESTING,
314+
span,
315+
"this block is too nested",
316+
None,
317+
"try refactoring your code, extraction is often both easier to read and less nested",
318+
);
319+
320+
return true;
321+
}
322+
323+
false
324+
}

0 commit comments

Comments
 (0)