dtolnay / request-for-implementation

Crates that don't exist, but should
610 stars 6 forks source link

Tool to enforce that Cargo.toml dependencies are written in sorted order #29

Closed dtolnay closed 5 years ago

dtolnay commented 5 years ago
$ cargo dependencies-sorted? path/to/Cargo.toml; echo $?
0

Large projects tend to like for things to stay sorted to cut down the occurrence of merge conflicts between two developers appending to the end of the same unsorted list in parallel changes. Similar idea behind the remain crate.

The command should parse a Cargo.toml and validate that particular pieces are sorted, exiting 0 if all sorted and nonzero otherwise.

We would at least want to check:

Take a look at the documentation for Cargo.toml to see whether any other parts make sense to enforce as sorted.

jpoles1 commented 5 years ago

I'm somewhat new to Rust, but I'd like to try to take a stab at this one as a way to learn some more!

dtolnay commented 5 years ago

Yay! Let me know how it goes or if you get stuck on any part.

jpoles1 commented 5 years ago

Thanks @dtolnay! Will keep you updated as the project progresses.

jpoles1 commented 5 years ago

Still working on some of the basic functionality, and I've gotta write tests for everything, but I've got a prototype up and running at this repo: https://github.com/jpoles1/cargo-dep-sort

As a note, I had originally wanted to use the TOML crate, but as far as I can tell, parsing with this library does not respect the order of the TOML tables, and as such is not suitable for this application. It looks like it's possible to enable preservation of ordering, but this is an experimental feature, and thus requires nightly rustc builds to work, which I wanted to avoid for the time being.

dtolnay commented 5 years ago

You will need to use the toml crate. Regex are not a reliable way to interpret a TOML file. The preserve_order feature is neither experimental nor requires nightly. You can turn it on as:

[dependencies]
toml = { version = "0.5", features = ["preserve_order"] }
jpoles1 commented 5 years ago

Ok, will give that try, thanks for the guidance!

jpoles1 commented 5 years ago

Nothing perfect, but I've got the basic functionality up and running using the toml crate. My big concern right now is writing tests.

One other thing I'm having some trouble with is a feature to write sorted Cargo.toml files out of the software. The big issue is that the serialization features of the toml crate are limited, such that you can't use inline tables (like the toml dep line you suggested to me above). Still trying to work out a way around that.

pickfire commented 5 years ago

Should this be in clippy instead?

polybuildr commented 5 years ago

In case it helps, in https://github.com/rust-lang/rust/issues/57443#issuecomment-476892723 I came across the toml_edit crate (a format preserving TOML parser that cargo-edit uses). It has some issues but works well overall.

gsquire commented 5 years ago

I decided to give this a go and published an initial version here. I will add some unit tests soon and ensure that it works as expected. Please file issues if you find any!

Edit: Another idea could be an actual cargo sub-command that uses cargo_metadata.

dtolnay commented 5 years ago

Thanks @gsquire! I wasn't able to get it working (https://github.com/gsquire/toml-sorted/issues/2) but once that works I'll close out this issue.

DevinR528 commented 5 years ago

I added some to @jpoles1, implementation wondering if I'm moving in the right direction https://github.com/DevinR528/cargo-dep-sort. Rewriting the file is the goal right? Having it work like cargo itself, by assuming everything from cwd, is probably the most ergonomic?

dtolnay commented 5 years ago

In my case I wouldn't want it rewriting the file, just a reliable 0 or 1 exit code and consideration for common patterns like out-of-line dependency sections (syn in cargo-expand/Cargo.toml).

DevinR528 commented 5 years ago

So fail if:

[dependencies.b]
version = "0.15"

[dependencies.a]
version = "0.15"
dtolnay commented 5 years ago

Yes I would want that to exit nonzero. The out-of-line dependencies should need to be sorted among themselves.

Zykino commented 5 years ago

Maybe it is not your use case but having an option to override the file could be nice. Or at least write the correct order somewhere: console/file.

DevinR528 commented 5 years ago

Here's my crack at sort checking cargo.toml you can install it cargo install cargo-sort-ck. I'm glad I found this repo any excuse to write some rust is a good one thanks @dtolnay. @Zykino I also started adding that feature to https://github.com/DevinR528/cargo-dep-sort cloned from @jpoles1.

DevinR528 commented 5 years ago

cargo-sort-ck now has a toml tokenizer, as such can handle comments, nested headers, out of group nested headers and each "token" impls display so printing to a file would be as easy as .to_string().

DevinR528 commented 5 years ago

cargo-sort-ck is finished, other than bug fixes. Any feed back would be much appreciated, working alone can create a real echo chamber.

Thanks

dtolnay commented 5 years ago

Thanks @DevinR528! This is terrific. I filed some issues for you but I am really impressed by your crate.