Skip to content

Commit 7ecc601

Browse files
Centri3Centri3
committed
decided against reinventing the wheel
1 parent c584823 commit 7ecc601

File tree

4 files changed

+137
-316
lines changed

4 files changed

+137
-316
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/target-ra
2+
13
# Used by CI to be able to push:
24
/.github/deploy_key
35
out

‎clippy_lints/src/excessive_nesting.rs‎

Lines changed: 37 additions & 230 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
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,
3+
visit::{walk_block, walk_item, Visitor},
4+
Block, Crate, Inline, Item, ItemKind, ModKind,
85
};
96
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
107
use rustc_middle::lint::in_external_macro;
118
use rustc_session::{declare_tool_lint, impl_lint_pass};
129
use rustc_span::Span;
13-
use thin_vec::ThinVec;
1410

1511
declare_clippy_lint! {
1612
/// ### What it does
@@ -22,11 +18,6 @@ declare_clippy_lint! {
2218
/// It can severely hinder readability. The default is very generous; if you
2319
/// exceed this, it's a sign you should refactor.
2420
///
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-
///
3021
/// ### Example
3122
/// An example clippy.toml configuration:
3223
/// ```toml
@@ -74,14 +65,17 @@ pub struct ExcessiveNesting {
7465
}
7566

7667
impl EarlyLintPass for ExcessiveNesting {
77-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
68+
fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) {
7869
let conf = self;
79-
NestingVisitor {
70+
let mut visitor = NestingVisitor {
8071
conf,
8172
cx,
8273
nest_level: 0,
74+
};
75+
76+
for item in &krate.items {
77+
visitor.visit_item(item);
8378
}
84-
.visit_item(item);
8579
}
8680
}
8781

@@ -91,239 +85,52 @@ struct NestingVisitor<'conf, 'cx> {
9185
nest_level: u64,
9286
}
9387

94-
impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
95-
fn visit_local(&mut self, local: &Local) {
96-
self.visit_pat(&local.pat);
97-
98-
match &local.kind {
99-
LocalKind::Init(expr) => self.visit_expr(expr),
100-
LocalKind::InitElse(expr, block) => {
101-
self.visit_expr(expr);
102-
self.visit_block(block);
103-
},
104-
LocalKind::Decl => (),
88+
impl NestingVisitor<'_, '_> {
89+
fn check_indent(&self, span: Span) -> bool {
90+
if self.nest_level > self.conf.excessive_nesting_threshold && !in_external_macro(self.cx.sess(), span) {
91+
span_lint_and_help(
92+
self.cx,
93+
EXCESSIVE_NESTING,
94+
span,
95+
"this block is too nested",
96+
None,
97+
"try refactoring your code to minimize nesting",
98+
);
99+
100+
return true;
105101
}
102+
103+
false
106104
}
105+
}
107106

107+
impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
108108
fn visit_block(&mut self, block: &Block) {
109-
// TODO: Can we use some RAII guard instead? Borrow checker seems to hate that
110-
// idea but it would be a lot cleaner.
111109
self.nest_level += 1;
112110

113-
if !check_indent(self, block.span) {
114-
for stmt in &block.stmts {
115-
self.visit_stmt(stmt);
116-
}
111+
if !self.check_indent(block.span) {
112+
walk_block(self, block);
117113
}
118114

119115
self.nest_level -= 1;
120116
}
121117

122-
fn visit_stmt(&mut self, stmt: &Stmt) {
123-
match &stmt.kind {
124-
StmtKind::Local(local) => self.visit_local(local),
125-
StmtKind::Item(item) => self.visit_item(item),
126-
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(expr),
127-
_ => (),
128-
}
129-
}
130-
131-
fn visit_arm(&mut self, arm: &Arm) {
132-
self.visit_pat(&arm.pat);
133-
if let Some(expr) = &arm.guard {
134-
self.visit_expr(expr);
135-
}
136-
self.visit_expr(&arm.body);
137-
}
138-
139-
// TODO: Is this necessary?
140-
fn visit_pat(&mut self, pat: &Pat) {
141-
match &pat.kind {
142-
PatKind::Box(pat) | PatKind::Ref(pat, ..) | PatKind::Paren(pat) => self.visit_pat(pat),
143-
PatKind::Lit(expr) => self.visit_expr(expr),
144-
PatKind::Range(start, end, ..) => {
145-
if let Some(expr) = start {
146-
self.visit_expr(expr);
147-
}
148-
if let Some(expr) = end {
149-
self.visit_expr(expr);
150-
}
151-
},
152-
PatKind::Ident(.., pat) if let Some(pat) = pat => {
153-
self.visit_pat(pat);
154-
},
155-
PatKind::Struct(.., pat_fields, _) => {
156-
for pat_field in pat_fields {
157-
self.visit_pat(&pat_field.pat);
158-
}
159-
},
160-
PatKind::TupleStruct(.., pats) | PatKind::Or(pats) | PatKind::Tuple(pats) | PatKind::Slice(pats) => {
161-
for pat in pats {
162-
self.visit_pat(pat);
163-
}
164-
},
165-
_ => (),
166-
}
167-
}
168-
169-
fn visit_expr(&mut self, expr: &Expr) {
170-
// This is a mess, but really all it does is extract every expression from every applicable variant
171-
// of ExprKind until it finds a Block.
172-
// TODO: clippy_utils has the two functions for_each_expr and for_each_expr_with_closures, can those
173-
// be used here or are they not applicable for this case?
174-
match &expr.kind {
175-
ExprKind::ConstBlock(anon_const) => self.visit_expr(&anon_const.value),
176-
ExprKind::Call(.., args) => {
177-
for expr in args {
178-
self.visit_expr(expr);
179-
}
180-
},
181-
ExprKind::MethodCall(method_call) => {
182-
for expr in &method_call.args {
183-
self.visit_expr(expr);
184-
}
185-
},
186-
ExprKind::Tup(exprs) | ExprKind::Array(exprs) => {
187-
for expr in exprs {
188-
self.visit_expr(expr);
189-
}
190-
},
191-
ExprKind::Binary(.., left, right)
192-
| ExprKind::Assign(left, right, ..)
193-
| ExprKind::AssignOp(.., left, right)
194-
| ExprKind::Index(left, right) => {
195-
self.visit_expr(left);
196-
self.visit_expr(right);
197-
},
198-
ExprKind::Let(pat, expr, ..) => {
199-
self.visit_pat(pat);
200-
self.visit_expr(expr);
201-
},
202-
ExprKind::Unary(.., expr)
203-
| ExprKind::Await(expr)
204-
| ExprKind::Field(expr, ..)
205-
| ExprKind::AddrOf(.., expr)
206-
| ExprKind::Try(expr) => {
207-
self.visit_expr(expr);
208-
},
209-
ExprKind::Repeat(expr, anon_const) => {
210-
self.visit_expr(expr);
211-
self.visit_expr(&anon_const.value);
212-
},
213-
ExprKind::If(expr, block, else_expr) => {
214-
self.visit_expr(expr);
215-
self.visit_block(block);
216-
217-
if let Some(expr) = else_expr {
218-
self.visit_expr(expr);
219-
}
220-
},
221-
ExprKind::While(expr, block, ..) => {
222-
self.visit_expr(expr);
223-
self.visit_block(block);
224-
},
225-
ExprKind::ForLoop(pat, expr, block, ..) => {
226-
self.visit_pat(pat);
227-
self.visit_expr(expr);
228-
self.visit_block(block);
229-
},
230-
ExprKind::Loop(block, ..)
231-
| ExprKind::Block(block, ..)
232-
| ExprKind::Async(.., block)
233-
| ExprKind::TryBlock(block) => {
234-
self.visit_block(block);
235-
},
236-
ExprKind::Match(expr, arms) => {
237-
self.visit_expr(expr);
238-
239-
for arm in arms {
240-
self.visit_arm(arm);
241-
}
242-
},
243-
ExprKind::Closure(closure) => self.visit_expr(&closure.body),
244-
ExprKind::Range(start, end, ..) => {
245-
if let Some(expr) = start {
246-
self.visit_expr(expr);
247-
}
248-
if let Some(expr) = end {
249-
self.visit_expr(expr);
250-
}
251-
},
252-
ExprKind::Break(.., expr) | ExprKind::Ret(expr) | ExprKind::Yield(expr) | ExprKind::Yeet(expr) => {
253-
if let Some(expr) = expr {
254-
self.visit_expr(expr);
255-
}
256-
},
257-
ExprKind::Struct(struct_expr) => {
258-
for field in &struct_expr.fields {
259-
self.visit_expr(&field.expr);
260-
}
261-
},
262-
_ => (),
263-
}
264-
}
265-
266-
fn visit_fn(&mut self, fk: FnKind<'_>, _: Span, _: NodeId) {
267-
match fk {
268-
FnKind::Fn(.., block) if let Some(block) = block => self.visit_block(block),
269-
FnKind::Closure(.., expr) => self.visit_expr(expr),
270-
// :/
271-
FnKind::Fn(..) => (),
272-
}
273-
}
274-
275118
fn visit_item(&mut self, item: &Item) {
276119
match &item.kind {
277-
ItemKind::Static(static_item) if let Some(expr) = static_item.expr.as_ref() => self.visit_expr(expr),
278-
ItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => self.visit_expr(expr),
279-
ItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => self.visit_block(block),
280-
ItemKind::Mod(.., mod_kind)
281-
if let ModKind::Loaded(items, Inline::Yes, ModSpans { inner_span, ..}) = mod_kind =>
282-
{
120+
ItemKind::Trait(_) | ItemKind::Impl(_) | ItemKind::Mod(.., ModKind::Loaded(_, Inline::Yes, _)) => {
283121
self.nest_level += 1;
284122

285-
check_indent(self, *inner_span);
123+
if !self.check_indent(item.span) {
124+
walk_item(self, item);
125+
}
286126

287127
self.nest_level -= 1;
288-
}
289-
ItemKind::Trait(trit) => check_trait_and_impl(self, item, &trit.items),
290-
ItemKind::Impl(imp) => check_trait_and_impl(self, item, &imp.items),
291-
_ => (),
292-
}
293-
}
294-
}
295-
296-
fn check_trait_and_impl(visitor: &mut NestingVisitor<'_, '_>, item: &Item, items: &ThinVec<P<Item<AssocItemKind>>>) {
297-
visitor.nest_level += 1;
298-
299-
if !check_indent(visitor, item.span) {
300-
for item in items {
301-
match &item.kind {
302-
AssocItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => {
303-
visitor.visit_expr(expr);
304-
},
305-
AssocItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => visitor.visit_block(block),
306-
_ => (),
307-
}
128+
},
129+
// Mod: Don't visit non-inline modules
130+
// ForeignMod: I don't think this is necessary, but just incase let's not take any chances (don't want to
131+
// cause any false positives)
132+
ItemKind::Mod(..) | ItemKind::ForeignMod(..) => {},
133+
_ => walk_item(self, item),
308134
}
309135
}
310-
311-
visitor.nest_level -= 1;
312-
}
313-
314-
fn check_indent(visitor: &NestingVisitor<'_, '_>, span: Span) -> bool {
315-
if visitor.nest_level > visitor.conf.excessive_nesting_threshold && !in_external_macro(visitor.cx.sess(), span) {
316-
span_lint_and_help(
317-
visitor.cx,
318-
EXCESSIVE_NESTING,
319-
span,
320-
"this block is too nested",
321-
None,
322-
"try refactoring your code, extraction is often both easier to read and less nested",
323-
);
324-
325-
return true;
326-
}
327-
328-
false
329136
}

‎tests/ui-toml/excessive_nesting/excessive_nesting.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ trait Lol {
7777
fn lmao() {
7878
fn bb() {
7979
fn cc() {
80-
let x = { 1 }; // not a warning
80+
let x = { 1 }; // not a warning, but cc is
8181
}
8282

8383
let x = { 1 }; // warning
@@ -93,8 +93,8 @@ pub mod a {
9393
pub mod d {
9494
pub mod e {
9595
pub mod f {}
96-
}
97-
}
96+
} // not here
97+
} // only warning should be here
9898
}
9999
}
100100
}
@@ -139,6 +139,7 @@ fn main() {
139139
let boo = true;
140140
!{boo as u32 + !{boo as u32 + !{boo as u32}}};
141141

142+
// this is a mess, but that's intentional
142143
let mut y = 1;
143144
y += {{{{{5}}}}};
144145
let z = y + {{{{{{{{{5}}}}}}}}};

0 commit comments

Comments
 (0)