Skip to content

Commit eaa835d

Browse files
committed
write tests, description and fix false negatives
1 parent e174fa1 commit eaa835d

File tree

7 files changed

+633
-152
lines changed

7 files changed

+633
-152
lines changed

clippy_lints/src/excessive_nesting.rs

Lines changed: 72 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use rustc_ast::{
33
node_id::NodeId,
4+
ptr::P,
45
visit::{FnKind, Visitor},
5-
Arm, AssocItemKind, Block, Expr, ExprKind, Item, ItemKind, Local, LocalKind, ModKind, Pat, PatKind, Stmt, StmtKind,
6+
Arm, AssocItemKind, Block, Expr, ExprKind, Inline, Item, ItemKind, Local, LocalKind, ModKind, ModSpans, Pat,
7+
PatKind, Stmt, StmtKind,
68
};
7-
use rustc_lint::{EarlyContext, EarlyLintPass};
8-
// TODO: Use this? Not sure if this is necessary here. After all, this is pre macro expansion...
9-
// use rustc_middle::lint::in_external_macro;
9+
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
10+
use rustc_middle::lint::in_external_macro;
1011
use rustc_session::{declare_tool_lint, impl_lint_pass};
1112
use rustc_span::Span;
13+
use thin_vec::ThinVec;
1214

1315
declare_clippy_lint! {
1416
/// ### What it does
@@ -20,10 +22,44 @@ declare_clippy_lint! {
2022
/// It can severely hinder readability. The default is very generous; if you
2123
/// exceed this, it's a sign you should refactor.
2224
///
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+
///
2330
/// ### Example
24-
/// TODO
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+
/// }
2548
/// Use instead:
26-
/// TODO
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+
/// }
2763
#[clippy::version = "1.70.0"]
2864
pub EXCESSIVE_NESTING,
2965
restriction,
@@ -58,14 +94,13 @@ impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
5894
fn visit_local(&mut self, local: &Local) {
5995
self.visit_pat(&local.pat);
6096

61-
#[expect(clippy::match_wildcard_for_single_variants, reason = "this is intentional")]
6297
match &local.kind {
6398
LocalKind::Init(expr) => self.visit_expr(expr),
6499
LocalKind::InitElse(expr, block) => {
65100
self.visit_expr(expr);
66101
self.visit_block(block);
67102
},
68-
_ => (),
103+
LocalKind::Decl => (),
69104
}
70105
}
71106

@@ -163,10 +198,13 @@ impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
163198
| ExprKind::Await(expr)
164199
| ExprKind::Field(expr, ..)
165200
| ExprKind::AddrOf(.., expr)
166-
| ExprKind::Repeat(expr, ..)
167201
| ExprKind::Try(expr) => {
168202
self.visit_expr(expr);
169203
},
204+
ExprKind::Repeat(expr, anon_const) => {
205+
self.visit_expr(expr);
206+
self.visit_expr(&anon_const.value);
207+
}
170208
ExprKind::If(expr, block, else_expr) => {
171209
self.visit_expr(expr);
172210
self.visit_block(block);
@@ -221,11 +259,11 @@ impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
221259
}
222260

223261
fn visit_fn(&mut self, fk: FnKind<'_>, _: Span, _: NodeId) {
224-
#[expect(clippy::match_wildcard_for_single_variants, reason = "this is intentional")]
225262
match fk {
226263
FnKind::Fn(.., block) if let Some(block) = block => self.visit_block(block),
227264
FnKind::Closure(.., expr) => self.visit_expr(expr),
228-
_ => (),
265+
// :/
266+
FnKind::Fn(..) => (),
229267
}
230268
}
231269

@@ -234,55 +272,42 @@ impl<'conf, 'cx> Visitor<'_> for NestingVisitor<'conf, 'cx> {
234272
ItemKind::Static(static_item) if let Some(expr) = static_item.expr.as_ref() => self.visit_expr(expr),
235273
ItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => self.visit_expr(expr),
236274
ItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => self.visit_block(block),
237-
ItemKind::Mod(.., mod_kind) if let ModKind::Loaded(items, inline, ..) = mod_kind => {
275+
ItemKind::Mod(.., mod_kind)
276+
if let ModKind::Loaded(items, Inline::Yes, ModSpans { inner_span, ..}) = mod_kind =>
277+
{
238278
self.nest_level += 1;
239279

240-
for item in items {
241-
self.visit_item(item);
242-
}
280+
check_indent(self, *inner_span);
243281

244282
self.nest_level -= 1;
245283
}
246-
// TODO: These 2 are duplicated
247-
ItemKind::Trait(trit) => {
248-
self.nest_level += 1;
249-
250-
for item in &trit.items {
251-
match &item.kind {
252-
// TODO: This is copied from above, is this necessary?
253-
AssocItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => {
254-
self.visit_expr(expr);
255-
},
256-
AssocItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => self.visit_block(block),
257-
_ => (),
258-
}
259-
}
260-
261-
self.nest_level -= 1;
262-
}
263-
ItemKind::Impl(imp) => {
264-
self.nest_level += 1;
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+
}
265290

266-
for item in &imp.items {
267-
match &item.kind {
268-
// TODO: This is copied from above, is this necessary?
269-
AssocItemKind::Const(const_item) if let Some(expr) = const_item.expr.as_ref() => {
270-
self.visit_expr(expr);
271-
},
272-
AssocItemKind::Fn(fk) if let Some(block) = fk.body.as_ref() => self.visit_block(block),
273-
_ => (),
274-
}
275-
}
291+
fn check_trait_and_impl(visitor: &mut NestingVisitor<'_, '_>, item: &Item, items: &ThinVec<P<Item<AssocItemKind>>>) {
292+
visitor.nest_level += 1;
276293

277-
self.nest_level -= 1;
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+
_ => (),
278302
}
279-
_ => (),
280303
}
281304
}
305+
306+
visitor.nest_level -= 1;
282307
}
283308

284309
fn check_indent(visitor: &NestingVisitor<'_, '_>, span: Span) -> bool {
285-
if visitor.nest_level > visitor.conf.excessive_nesting_threshold {
310+
if visitor.nest_level > visitor.conf.excessive_nesting_threshold && !in_external_macro(visitor.cx.sess(), span) {
286311
span_lint_and_help(
287312
visitor.cx,
288313
EXCESSIVE_NESTING,
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#[rustfmt::skip]
2+
#[macro_export]
3+
macro_rules! excessive_nesting {
4+
() => {{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{
5+
println!("hi!!")
6+
}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
7+
}

tests/ui-toml/excessive_nesting/excessive_nesting.rs

Lines changed: 176 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,173 @@
1+
// aux-build:macro_rules.rs
2+
#![rustfmt::skip]
3+
#![feature(custom_inner_attributes)]
14
#![allow(unused)]
5+
#![allow(clippy::let_and_return)]
6+
#![allow(clippy::redundant_closure_call)]
7+
#![allow(clippy::no_effect)]
8+
#![allow(clippy::unnecessary_operation)]
29
#![warn(clippy::excessive_nesting)]
10+
#![allow(clippy::collapsible_if)]
11+
12+
#[macro_use]
13+
extern crate macro_rules;
14+
15+
static X: u32 = {
16+
let x = {
17+
let y = {
18+
let z = {
19+
let w = { 3 };
20+
w
21+
};
22+
z
23+
};
24+
y
25+
};
26+
x
27+
};
28+
29+
macro_rules! xx {
30+
() => {{
31+
{
32+
{
33+
{
34+
{
35+
{
36+
{
37+
{
38+
{
39+
{
40+
{
41+
println!("ehe");
42+
}
43+
}
44+
}
45+
}
46+
}
47+
}
48+
}
49+
}
50+
}
51+
}
52+
}};
53+
}
54+
55+
struct A;
56+
57+
impl A {
58+
pub fn a(&self, v: u32) {
59+
struct B;
60+
61+
impl B {
62+
pub fn b() {
63+
struct C;
64+
65+
impl C {
66+
pub fn c() {}
67+
}
68+
}
69+
}
70+
}
71+
}
72+
73+
struct D { d: u32 }
74+
75+
trait Lol {
76+
fn lmao() {
77+
fn bb() {
78+
fn cc() {
79+
let x = { 1 }; // not a warning
80+
}
81+
82+
let x = { 1 }; // warning
83+
}
84+
}
85+
}
86+
87+
use a::{b::{c::{d::{e::{f::{}}}}}}; // should not lint
88+
89+
pub mod a {
90+
pub mod b {
91+
pub mod c {
92+
pub mod d {
93+
pub mod e {
94+
pub mod f {}
95+
}
96+
}
97+
}
98+
}
99+
}
100+
101+
fn a_but_not(v: u32) {}
3102

4-
#[rustfmt::skip]
5103
fn main() {
104+
let a = A;
105+
106+
a_but_not({{{{{{{{0}}}}}}}});
107+
a.a({{{{{{{{{0}}}}}}}}});
108+
(0, {{{{{{{1}}}}}}});
109+
110+
if true {
111+
if true {
112+
if true {
113+
if true {
114+
if true {
115+
116+
}
117+
}
118+
}
119+
}
120+
}
121+
122+
let y = (|| {
123+
let x = (|| {
124+
let y = (|| {
125+
let z = (|| {
126+
let w = { 3 };
127+
w
128+
})();
129+
z
130+
})();
131+
y
132+
})();
133+
x
134+
})();
135+
136+
excessive_nesting!(); // ensure this isn't linted in external macros
137+
xx!();
138+
let boo = true;
139+
!{boo as u32 + !{boo as u32 + !{boo as u32}}};
140+
141+
let mut y = 1;
142+
y += {{{{{5}}}}};
143+
let z = y + {{{{{{{{{5}}}}}}}}};
144+
[0, {{{{{{{{{{0}}}}}}}}}}];
145+
let mut xx = [0; {{{{{{{{1}}}}}}}}];
146+
xx[{{{{{{{{{{{{{{{{{{{{{{{{3}}}}}}}}}}}}}}}}}}}}}}}}];
147+
&mut {{{{{{{{{{y}}}}}}}}}};
148+
149+
for i in {{{{xx}}}} {{{{{{{{}}}}}}}}
150+
151+
while let Some(i) = {{{{{xx.pop()}}}}} {{{{{{{}}}}}}}
152+
153+
while {{{{{{{{true}}}}}}}} {{{{{{{{{}}}}}}}}}
154+
155+
let d = D { d: {{{{{{{{{{{{{{{{{{{{{{{3}}}}}}}}}}}}}}}}}}}}}}} };
156+
157+
{{{{1}}}}..{{{{{{3}}}}}};
158+
{{{{1}}}}..={{{{{{{{{{{{{{{{{{{{{{{{{{6}}}}}}}}}}}}}}}}}}}}}}}}}};
159+
..{{{{{{{5}}}}}}};
160+
..={{{{{3}}}}};
161+
{{{{{1}}}}}..;
162+
163+
loop { break {{{{1}}}} }
164+
loop {{{{{{}}}}}}
165+
166+
match {{{{{{true}}}}}} {
167+
true => {{{{}}}},
168+
false => {{{{}}}},
169+
}
170+
6171
{
7172
{
8173
{
@@ -13,3 +178,13 @@ fn main() {
13178
}
14179
}
15180
}
181+
182+
async fn b() -> u32 {
183+
async fn c() -> u32 {{{{{{{0}}}}}}}
184+
185+
c().await
186+
}
187+
188+
async fn a() {
189+
{{{{b().await}}}};
190+
}

0 commit comments

Comments
 (0)