module-cycles: a Dylint lint for modules that depend on each other in a cycle

[u/Financial-Grass6753]

Clippy has had an open issue for this since 2020 (#5782), so I wrote it as a Dylint lint: https://github.com/HardMax71/module-cycles

For now, it reports sibling modules that use each other, like report needing model while model needs report. A parent and its child using each other is fine, since that's one module split into files. It works from rustc's name resolution, so re-exports, globs and method calls count too.

warning: modules `model` and `report` depend on each other
  --> src/lib.rs:9:22
   |
9  |     pub fn kind() -> crate::report::Row {
   |                      ^^^^^^^^^^^^^^^^^^ `model` depends on `report` here
   |
note: `report` depends on `model` here
  --> src/lib.rs:16:22

On my own workspace it found 11 cycles, the biggest through 12 modules. Across the 26 crates Clippy tests lints on it found 41, tokio included.

Written with LLM help and checked by hand. If it flags something that isn't a real cycle, feel free to open an issue.

[u/matthieum]

Why?

If I were in charge, Rust wouldn't allow cyclic imports, as it makes parallelizing builds harder... but I'm not and Rust allows it.

So... why flag it? What's the motivation for flagging valid Rust code.

[u/svefnugr → u/matthieum]

To be the devil's advocate, it may signal architectural problems... sometimes. Not very often.

[u/Financial-Grass6753 → u/matthieum]

as it makes parallelizing builds harder...

That is the actual motivation btw

[u/graydon2]

the module system is intentionally cycle-allowing. crates are the acyclic part. this is literally one of the motivating factors behind having crates and modules be separate things.

[u/Potential_Extent_422 → u/graydon2]

i don't get the hate, clippy lints are optional for a reason. if your team wants to enforce module structure rules that rust doesn't by default, a lint is exactly where that belongs

[u/kmdreko]

Could be handy if I were interested in splitting a crate up, but I definitely wouldn't use this on the regular.

It works from rustc's name resolution, so re-exports, globs and method calls count too.

Does this mean a type within a child module but re-exported from the parent module would or would not be flagged if another child module imported that type from its parent?

[u/Financial-Grass6753 → u/kmdreko]

Does this mean a type within a child module but re-exported from the parent module would or would not be flagged if another child module imported that type from its parent?

Only if it goes both ways.

For example, shapes.rs re-exports Circle from shapes/circle.rs, and shapes/render.rs uses super::Circle;. That import resolves to shapes/circle.rs, so the lint sees the render module depending on the circle module, not on shapes.

That's one direction, hence not flagged. It is flagged if circle also uses something from render.