Stream: ideas

Topic: clippy cognitive complexity


view this post on Zulip Anton (Oct 26 2024 at 14:13):

Today I found out that clippy has a cognitive_complexity lint, it is in nursery state so it is not currently enabled for us.

I'd like to try enabling it. It's easy for functions to grow ever more complex over time and this lint could encourage us to keep things simple and maintainable.

These functions were identified as too complex:
Screenshot complexity table.png

I would currently add exceptions for all the functions in the table and put them in an issue so we can explore simplifying each one in a standalone PR.

If the lint does not turn out to be useful in practice we can just get rid of it.

view this post on Zulip Richard Feldman (Oct 26 2024 at 14:54):

I'm personally not a fan of that heuristic

view this post on Zulip Anton (Oct 26 2024 at 14:56):

Tell me more

view this post on Zulip Richard Feldman (Oct 26 2024 at 14:57):

I've worked on code bases that had the equivalent of that lint enabled, and it just resulted in "extract this chunk of the code into a helper function even though that makes the code harder to follow"

view this post on Zulip Richard Feldman (Oct 26 2024 at 14:58):

like it's the same number of conditionals, just now they're spread over 2 functions so you can't directly see how they flow into each other

view this post on Zulip Richard Feldman (Oct 26 2024 at 14:58):

sometimes a helper function is more clear, sometimes it's less clear, but the lint makes it so that even when it's less clear you still have to do it

view this post on Zulip Anton (Oct 26 2024 at 15:02):

I'd be ok with disableing it for a specific function if we believe the way it is written is the best way. Currently there is no friction for functions growing in complexity other than a thorough review, and I'd like there to be some friction.

view this post on Zulip Richard Feldman (Oct 26 2024 at 15:03):

how about this: let's try enabling it, but before merging it, see what functions it complains about, and what they'd look like before/after satisfying it, and discuss based on that?

view this post on Zulip Anton (Oct 26 2024 at 15:05):

Sounds good!

view this post on Zulip Anton (Oct 26 2024 at 15:08):

Relatedly; in PR#6859 we refactored build_loaded_file, clippy reports a too high complexity of 30 (on the original version on main) and in my opinion we vastly improved that function in that PR.

view this post on Zulip Richard Feldman (Oct 26 2024 at 15:10):

fair, although that happened anyway without the lint :wink:

view this post on Zulip Anton (Oct 26 2024 at 15:53):

true, we stumbled upon the complexity of build_loaded_file, the lint could show us similar functions that would benefit from some attention

view this post on Zulip Brendan Hansknecht (Oct 26 2024 at 16:43):

I am generally in the same boat as Richard in opinion here. I find those types of lint's too much of a nuisance. They also can turn away new contributors. I would rather a new contributor merge a complex function than feel like their code is inadequate and give up cause clippy said it was too complex.

view this post on Zulip Brendan Hansknecht (Oct 26 2024 at 16:45):

At the same time, I'm sure some of the functions it will list will be functions that are reasonable to refactor. Just likely not a priority and likely need a more complex refactor than the quick fix solutions those lints tend to lead to (putting arbitrary chunks of code into sub functions)

view this post on Zulip Anton (Oct 26 2024 at 17:37):

Good points, new contributors should not try to fix these high complexity functions. We could limit this to an issue with a list of the high complexity functions that we should take a look at and a separate CI job that could inform us of increases in complexity. That CI job would never fail so people could still merge their PRs if complexity was increased.

view this post on Zulip Anton (Oct 26 2024 at 17:40):

Just likely not a priority

If a complex function is often looked at when debugging different bugs I think it should be a priority to simplify it.

view this post on Zulip Brendan Hansknecht (Oct 26 2024 at 17:43):

We could limit this to an issue with a list of the high complexity functions that we should take a look at and a separate CI job that could inform us of increases in complexity

Yeah, something like that sounds much more likely to be useful.


Last updated: Jun 16 2026 at 16:19 UTC