Enable Clippy nesting limit

[Nilirad]

Objective

Some areas of the codebase are excessively nested. I think we should at least put a limit to it to prevent exacerbating the already critical situation.

Solution

Update clippy.toml to set the excessive-nesting-threshold option to 9, which is the highest level of nesting so far.

Tested by doing cargo +stable clippy --all-features --all-targets

[alice-i-cecile]

This seems sensible as a style guidance (ideally we can bring it down a bit further later). Config is correct.

Note: Even though this lint is warn-by-default, it will only trigger if a maximum nesting level is defined in the clippy.toml file.

From the Clippy docs.

[Nilirad]

Yes, the idea is to progressively bring it down to an acceptable level. It's common practice in C to not exceed 3 levels, most notably in the Linux kernel.
In the case of Bevy, that goal is much more difficult to attain, since Rust code tends to nest more than C, with mod and impl blocks, and with closures (that can be factored out into functions anyway). Using let-else statements tends to flatten the code by a considerable amount by keeping the happy path of the function to the first level, and bail out on error. I think we can achieve a nesting limit of 5 (even 4 as a stretch goal) in a relatively short amount of time without recurring to excessively controversial techniques.

In my personal Bevy project I've been able to reach level 3, even with UI code. Also, I've started doing some experiments refactoring the most nested functions in the Bevy codebase to see how low we can push the limit.

[rlidwka]

This should be handled by humans during code review on case-by-case basis.

Refactoring existing code to reduce nesting level is a good idea. But enforcing it on CI is very much not: if someone needs to fix a simple bug by adding a condition into already-at-nesting-limit function, would she have to refactor the entire function just to make CI error go away?

[Nilirad]

I think we should still move forward. The fact that we've reached level 9 proves that we shouldn't rely on manual code review.

Yes, this may make a bugfix PR more complex, but the benefits greatly outweigh the drawbacks. If a bug can be lazily fixed by adding just one more indentation level, we worsen an already critical situation, accumulating technical debt over time, making further changes progressively more and more difficult, paradoxically.

As I mentioned before, Linus Torvalds highly encourages a strict 3 nesting levels policy on the Linux kernel codebase, but that doesn't prevent it from having arguably the most active open source community in the world. If a project with 30M lines of code thrives with 3 levels of indentation, I doubt we'll encounter problems by enforcing 9.

[rlidwka]

As I mentioned before, Linus Torvalds highly encourages a strict 3 nesting levels policy on the Linux kernel codebase

That's great! I believe we should also highly encourage 3 nesting levels policy.

I suggest to add a mention in the contributing guide that "don't use more than 3 nesting levels pretty please", and close this issue, because I'm sure we're going to have as much luck enforcing this as linux kernel does.

---

PS:
here's 6:
https://github.com/torvalds/linux/blob/bee0e7762ad2c6025b9f5245c040fcc36ef2bde8/fs/posix_acl.c#L409-L417
here's 7:
https://github.com/torvalds/linux/blob/bee0e7762ad2c6025b9f5245c040fcc36ef2bde8/lib/raid6/test/test.c#L135-L137
here's 7 again (or 8, depending on how you count):
https://github.com/torvalds/linux/blob/bee0e7762ad2c6025b9f5245c040fcc36ef2bde8/drivers/net/ethernet/dec/tulip/interrupt.c#L180-L187

[tim-blackbird]

The Linux project failing to enforce their style guide in CI doesn't seem like a reason for us to make the same mistake.

[doonv]

Heres an example of nesting for up to 10 levels (4 spaces):

// 0
fn example() {
    // 1
    { // 2
        { // 3
            { // 4
                { // 5
                    { // 6
                        { // 7
                            { // 8
                                { // 9
                                    // 10
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

In my opinion you shouldn't ever go beyond 7 idents.

[alice-i-cecile]

Closing; I don't think there's appetite for this currently.