Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding a json output that surfaces the attributes of the modules #91

Closed

Conversation

graysonarts
Copy link

I love this tool, but I needed an easy way to parse the output, so created a json printer. I tried to follow the pattern set up by the tree printer to implement the json output.

cargo fmt, cargo clippy, and cargo test all come back clean.

Here is what the output looks like. I didn't pretty print it because one can always use jq to format. Also, I didn't wrap the attributes in #[cfg(...)] like you do in tree. If that's a blocker, I can make that change easy enough. The path is also a full path as that's how it's stored internally. For my consumption, I'm planning to strip that based on the directory in which it was invoked after the fact, but if you'd prefer it to do relative paths, just let me know.

[
{"module":"cargo_modules","path":"/Users/grayson/code/contrib/cargo-modules/src/lib.rs","kind":"crate","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::theme","path":"/Users/grayson/code/contrib/cargo-modules/src/theme.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::theme::tree","path":"/Users/grayson/code/contrib/cargo-modules/src/theme/tree.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::theme::graph","path":"/Users/grayson/code/contrib/cargo-modules/src/theme/graph.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::target","path":"/Users/grayson/code/contrib/cargo-modules/src/target.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::printer","path":"/Users/grayson/code/contrib/cargo-modules/src/printer.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::printer::tree","path":"/Users/grayson/code/contrib/cargo-modules/src/printer/tree.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::printer::json","path":"/Users/grayson/code/contrib/cargo-modules/src/printer/json.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::printer::graph","path":"/Users/grayson/code/contrib/cargo-modules/src/printer/graph.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::orphans","path":"/Users/grayson/code/contrib/cargo-modules/src/orphans.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph","path":"/Users/grayson/code/contrib/cargo-modules/src/graph.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::walker","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/walker.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::util","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/util.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::orphans","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/orphans.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::node","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/node.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::node::visibility","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/node/visibility.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::node::attr","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/node/attr.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::edge","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/edge.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::graph::builder","path":"/Users/grayson/code/contrib/cargo-modules/src/graph/builder.rs","kind":"mod","visibility":"pub(crate)","attributes":[]}
,{"module":"cargo_modules::options","path":"/Users/grayson/code/contrib/cargo-modules/src/options.rs","kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::general","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::project","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::generate","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::generate::json","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::generate::tree","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::generate::graph","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::options::graph","path":null,"kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::generate","path":"/Users/grayson/code/contrib/cargo-modules/src/generate.rs","kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::generate::tree","path":"/Users/grayson/code/contrib/cargo-modules/src/generate/tree.rs","kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::generate::json","path":"/Users/grayson/code/contrib/cargo-modules/src/generate/json.rs","kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::generate::graph","path":"/Users/grayson/code/contrib/cargo-modules/src/generate/graph.rs","kind":"mod","visibility":"pub","attributes":[]}
,{"module":"cargo_modules::commands","path":"/Users/grayson/code/contrib/cargo-modules/src/commands.rs","kind":"mod","visibility":"pub","attributes":[]}
]```

@regexident
Copy link
Owner

Hey @graysonarts, this looks great!

Since the absolute paths cause problems with unit tests (since the working directory on CI differs from your setup) it would probably be desirable (at least for testing).

Relative paths might however not be suitable for --with-externs, which would produce paths outside of $PWD. Does generate json include extern declarations? 🤔

Maybe the generate json command should have a --absolute-paths or --relative-paths option? Not exactly sure which would make for a better default, absolute or relative paths.

@graysonarts
Copy link
Author

Relative paths might however not be suitable for --with-externs, which would produce paths outside of $PWD. Does generate json include extern declarations? 🤔

Oh, I haven't tried --with-externs. I'll do some testing for it and see what is does. I didn't change any of the generation code, so it should work like generate tree does, but I don't know for sure. Do you have a sample repo that uses externs that I can test against?

Maybe the generate json command should have a --absolute-paths or --relative-paths option? Not exactly sure which would make for a better default, absolute or relative paths.

My guess for most useful default would be relative paths with --absolute-paths option to override it, so I'll go that route if you are okay with that.

@regexident
Copy link
Owner

regexident commented Sep 20, 2021

Oh, I haven't tried --with-externs. I'll do some testing for it and see what is does. I didn't change any of the generation code, so it should work like generate tree does, but I don't know for sure. Do you have a sample repo that uses externs that I can test against?

The --with-externs (when used with --with-uses) will also generate nodes/edges for external symbols used by the crate when running generate graph.

My guess for most useful default would be relative paths with --absolute-paths option to override it, so I'll go that route if you are okay with that.

Sounds good to me!

As for testing I'd say having a single test for the "absolute path to relative path" logic should do. No need to duplicate the tests, especially as the absolute paths would cause issues anyway.

@regexident
Copy link
Owner

@graysonarts you may want to rebase this branch onto main as it fixes a bunch of rust-analyzer breakage.

@graysonarts
Copy link
Author

graysonarts commented Oct 11, 2021 via email

@regexident
Copy link
Owner

I'm gonna close this due to inactivity for now. Feel free to request a re-open once it takes up steam again though. 🙂

@regexident regexident closed this Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants