regexident / cargo-modules

Visualize/analyze a Rust crate's internal structure
Mozilla Public License 2.0
944 stars 48 forks source link

Identify cycles between imports #18

Open withoutboats opened 6 years ago

withoutboats commented 6 years ago

As a rule, I try to organize my code so that the relationships between modules are acyclic - if I import something from module a into module b, I don't import anything from module b into module a.

Since this can construct the graph of import relationships between modules, it would be great for it to have a way to identify cycles in that graph, to help find the parts of your program that are getting "spaghetti."

Alternatively, though, I suspect there's already a common utility that can identify cycles in dot graph data (though I don't know what it is), and possibly just documenting and recommending you pipe the data into that could be a good solution.

regexident commented 6 years ago

Thanks, @withoutboats!

I too would love to have cargo modules provide such a feature.

How would you expect such a hypothetical command cargo modules cycles to present detected cycles?

Something like this?

Error: Circular dependency between `foo` and `foo::bar::baz::blee`.

┌> foo
│  └─> foo::bar
│      └─> foo::bar::baz
│          └─> foo::bar::baz::blee
└─────────────┘
withoutboats commented 6 years ago

I have no idea! :)

pdh11 commented 1 year ago

FWIW, if you have a dot graph with cycles in, and you just want to pick out the cycles from it, that's what sccmap is for. You give it a dot file as input, and its output is a reduced dot file that contains only those nodes and edges which participate in cycles.

matta commented 3 months ago

@smoelius perhaps you can shed light on this? I'm not clear whether --acyclic is working properly, or whether it has regressed somehow.

I arrived here because I would like to identify cycles between module imports in my project (at least the obvious ones, in light of the point made in #185).

Trying the --acyclic option as shows in the README does not produce useful results:

% cargo modules dependencies --acyclic --lib
Error: Circular dependency between `sift::handle_key_event::Action` and `sift::handle_key_event::Action::eq`.

┌> sift::handle_key_event::Action
│  └─> sift::handle_key_event::Action::eq
└──────┘

This telling me about a #[derive(PartialEq)], which is not what I care about.

Exploring this further I try eliminating certain things from the graph, with the idea of having only the module imports remaining, and I get a more interesting case:

% cargo modules dependencies --acyclic --lib --no-fns --no-traits --no-sysroot
Error: Circular dependency between `sift::ui_state::State` and `sift::ui_state::State::new`.

┌> sift::ui_state::State
│  └─> sift::ui_state::State::new
└──────┘

...in this case new appears in the impl State in the same .rs file. I am assuming this is not intended behavior?

Also, --acyclic always seems to report one cycle and stop.

smoelius commented 3 months ago

...in this case new appears in the impl State in the same .rs file. I am assuming this is not intended behavior?

My preference would be that such a case not be reported. The same would be true for the Action/Action::eq case. Similar experiences caused me to open #185.

Also, --acyclic always seems to report one cycle and stop.

If memory serves me right, that behavior was intended.

cargo modules has changed a bit since #184 was merged. (For example, the command used to be cargo modules graph ... --acyclic).

I am afraid I don't have any more I could add that would be of value.