use rustfmt

[dvc94ch]

Why use rustfmt? The main reason is so you don't annoy developers when they accidentally run it before committing their changes.

[tcharding]

Yes please, it is a real drag to have to turn off format on save in your editor everytime one wants to work on rust-libp2p when ones daily work is on a rust code base that uses rustfmt. It also means I can't have a buffer open doing rust-libp2p work and a buffer open to other repos without have to reset all the formatting changes, while manageable its a drag to have to think about this stuff.

Can we configure rustfmt in a manner that is acceptable to the rust-libp2p project?

[burdges]

Rust was designed to be expressive, which makes Rust far harder to fmt than languages explicitly designed to be inexpressive, and so rustfmt ends up being fairly crappy.

We could set rustfmt.toml to ignore = ["/"] according to https://github.com/rust-lang/rustfmt/blob/master/Configurations.md except that never actually worked: rust-lang/rustfmt#4147 (comment)

As luck would have it, we could exploit rustfmt's bugginess for this by simply doing https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=856d116c793b74e6809441b4ccf7af68 in lib.rs or somewhere so that rustfmt panics rust-lang/rustfmt#3678

[rklaehn]

@burdges out of curiosity: could you point out examples where the rustfmt formatted code is so bad? I guess the huge lists of type constraints on some parts of the code will look bad. Maybe these sections can be manually excluded. In general I think fmt does an OK job.

[burdges]

I only know my own code, not the libp2p crate, but..

It screws up any whitespace that communicates emphasis, especially with iterator chains like https://github.com/w3f/schnorrkel/blob/master/src/vrf.rs#L944 so you need far more temporaries, which becomes excessive. There are many situations even post-NLL where Rust requires closures be handled in line, not as temporaries.

Anyways Parity has their own style guide and afaik they ban rustfmt from their code bases. I hope this becomes more commonplace..

[dvc94ch]

Actually there are parity codebases that use rustfmt. Gav doesn't like it so it's not used in substrate or polkadot, but many smaller crates/repos it's up to the devs basically. Also note that this is a libp2p repo not a paritytech repo.

[dvc94ch]

While my preference is for consistent formatting that doesn't require devs to learn a new style guide for each repo they contribute to, finding a way for rustfmt to simply ignore and not do anything is an acceptable solution for me. Note that code style is in many ways subjective and on a large project with many devs there is going to be inconsistent formatting even with a style guide.

[rklaehn]

I think in 95% of the time rustfmt does an OK job, and for the remaining 5% I am fine with manually disabling it.

But if that is not an option, just disabling it for the entire crate would already be an improvement for me.

[tcharding]

Any solution that doesn't require me to change my editor config to work on the repository is fine for me. Consistency is good, the granularity doesn't matter much I'd say i.e., more than one style in a repo but consistent within xyz modules - is libp2p really big enough or old enough to warrant that?

[JayKickliter]

Why use rustfmt? The main reason is so you don't annoy developers when they accidentally run it before committing their changes.

Yes and pretty-please. It's going to make a pretty big diff the first time, which no one likes. But the longer we wait, the worse that diff is going to be. But it's still worth the initial diff noise. It's helpful for maintainers and reviewers in that they don't have to bother with asking formatting changes on PRs. It helps first time contributors (I'm feeling this right now) by not having to guess the projects style.

It screws up any whitespace that communicates emphasis, especially with iterator chains like

I feel you, and empathize. In the cases where it legitimately makes for illegible code, locally disabling is an option. But I haven't seen rustfmt do that since maybe the first few releases.

[vorot93]

Any movement on this? It's really more a political decision than a technical one - reformatting and adding to CI would take like 5 mintutes.

[whereistejas]

Hi everyoone,
I think the general consensus agrees with using rustfmt. I would like to take up this issue and create a PR for it. Delaying this decision only makes the diffs bigger and bigger.

[dvc94ch]

the guy that needs to be convinced currently is @mxinden, and he'll probably do it himself iff convinced. But doesn't look like it.

[whereistejas]

I don't think he is against it. @mxinden what is your opinion, shall we rustfmt the entire repo?

[mxinden]

I am not opposed to using rustfmt in rust-libp2p. If you look closely you will see that e.g. #1838 is already formatted via rustfmt.

and he'll probably do it himself iff convinced.

Correct. Given this being a large change I would like to do it myself, in case we do decide to move to rustfmt.

It is on my TODO list to test it out e.g. for libp2p-core. Stay tuned.

[dvc94ch]

One thing to keep in mind is that rustfmt needs preserve the ast. A codebase that was never rustfmt'd might have some weird formatting due to how rustfmt breaks lines. But the solution isn't to blame rustfmt but over time split the lines in a more sensible way. It will rarely cause issues if lines are refactored in a way that line breaking isn't necessary.