How are you using rustfmt and clippy?

I don't share your other views (but can sympathize with them), but I do agree on this point.

I've thought in the past about writing an easy-to-add CI utility that auto-fixes styling issues and creates a commit on your branch, adding a comment to the PR telling you about it and asking you to pull the changes locally.

I never really got around to it, and the main blocker was whether that's actually worse, because then you might be continuing locally with your changes, only to later realise there's an upstream commit that you need to pull, with potentially many merge conflicts as a result...

Still, I do like the idea of something like that to reduce the friction of people contributing to my projects...

2 Likes

Rather than making it automatic, could you make it a bot (similar to how Rust devs can call @bors try to trigger a build)? Then either the PR author could call it when they're ready, or the project owner could call it just before merging.

Yeah, that would probably be better. I'm still not entire sold on that workflow though. You don't want the CI to show up red if there are only styling problems, as that gets back to the original problem of seeing a failure, having to click through to the CI log output, only to see that it's styling related, which a @rustfmtbot fix comment could have solved.

But then if you don't make the CI red, a PR can accidentally be merged into master with unresolved styling issues. To solve that, you'd have to integrate it more tightly into Bors, letting it do the style fixing in the final merge PRs, but that requires a different level of integration, and probably opens another set of issues that need to be resolved.

Not sure what the best solution is, but I've been thinking about it for a while, and I suspect I'll write some kind of tool for this in the near future, as I do see a problem space this could solve.

1 Like

yess one of the things i love about rust and its community is that there's one more-or-less agreed coding style, as it avoids the tiring repeated kinds of discussions in other languages, as well as having to learn how to format your code for every new project, changing editor settings, etc etc

this doesn't mean i never disagree with how rustfmt lays out the code (there's some good examples of frustrations in this topic), but also, i like not having to think about that, and if it's really bad there's #[rustfmt_skip]β€”and style discussion can be limited to cases where the deviation is intentional

i empathize here, it is annoying, but personally, i accept it, and even prefer it to the alternative of having formatting inconsistencies slip in and polluting git history with "fix up style" commits
(one way to ensure it never happens to you is to add "rustfmt" as a local git commit hook)

(and this annoyance is not specific to rust: sometimes there are arbitrary linters hooked up to the CI in C++ projects, and it's worse because to contribute one has to learn tools and formatting for the specific project, and how they evolve over time, instead of just "run rustfmt")

2 Likes

I use both but enforce neither. Much like @kornel expressed as a contributor perspective, as a maintainer perspective I've found that enforcement at the CI or procedural level significantly decreases engagement for comparatively little gain. PRs that are fine will remain completely untouched if lints are required to pass, which means either I do it myself upon merging (which is annoying and rather defeats the point) or I poke PR authors to do it, which is annoying to them and most of the time doesn't work. Also, having the main branch build turn red because of a style error is ridiculous, and only enforcing style for PRs is even more so. Additionally, enforcement silently turns away people who object or can't be bothered, further reducing engagement. For projects with lots of traffic, this may not be a perceived issue, but for smaller projects it absolutely is.

I enable clippy at the pedantic level and manually disable lints that don't fit on an item, module, or crate basis.

I run rustfmt as stock, but recently have considered changing the indentation to tabs; however I have not yet put that into practice on any project (because I haven't started any new projects since this reconsideration, and don't want to unilaterally change an existing project to tabs for now).

2 Likes

We were actually discussing this over on the bors-ng forum, and went with a webhook-based setup where bors sends your script a message, your script updates the staging.tmp branch with formatting and such, and then that's the commit that gets moved to staging and tested. It's a bit complicated, and we have been working on implementing other stuff, but it seems like what you'd want.

4 Likes

The webhook RFC looks awesome, and indeed would make the auto-rustfmt on merge a possibility (with relative ease, from the looks of things).

It's unfortunate I never took the time to really dive into Elixir, even though many of my fellow Rubyists encouraged me to do so. I've got too much on my plate right now to really get up-to-speed with the language and its ecosystem. Who knows, maybe some day...

1 Like

Rust works really fantastically with Elixir as NIF's, Ports, whatever!

Solution: yellow CI. Actually give a detailed report. Here's an example with Travis where the coverage build failed. Alternatively, use something like GitHub Actions such that the test and style are actually reported separately to the user on the main PR page, not just on the Checks tab.

I really do care about good CI behavior, and participate in crate-ci on and off as I get the time. The problem is that most CI give you a way to say green/red, but with lints you really want a green/yellow/red. Basically,

  • If tests fail, red
  • If lints fail, yellow
  • Otherwise, green

The bors "not rocket science" rule would accept yellow or green, but with developer pressure to move off of yellow to green again.

If/when GitHub Actions let me in, you can expect the crate-ci documentation to move towards GitHub Actions being fully supported such that the GitHub status reports can be as useful as possible.


I enforce rustfmt/clippy on my CI, but I also accept #[rustfmt::skip]/#[allow(clippy::_)] very liberally. For a new project, I suggest trying your hardest to actually do this: make cargo fmt -- --check and cargo clippy --all-targets --tests -- -D warnings pass on your project, and keep it that way. Lints are exponentially more useful the quieter they are, because you can see the new lints and decide whether they're important or not.

1 Like

This post was flagged by the community and is temporarily hidden.

I don't always like what rustfmt and/or clippy do, but I always obey them (with the default configurations). Neither hurts the code so much that it's worth it to take on the huge mental burden of deciding where to deviate from the standard. I just wish they were integrated with rustc somehow, because I don't have CI for my private projects and I sometimes forget to run them.

3 Likes

I do have to agree with this on the topic of clippy. It was disappointing when the decision was to get rid of the compiler plugin option for using it rather than stabilizing it.

1 Like

As for the actual topic: I really don't use either of them. The lints built into Rust themselves are good enough most of the time, and the formatting tends not to be that big of a deal.

1 Like

Unfortunately, I find #[rustfmt::skip] to be almost useless because of this sort of error:

error: attributes are not yet allowed on `if` expressions

One example of a situation where I break rustfmt rules is stuff like this as the final expression in a function:

if short_var {
    short_thing
} else {
    other_short_thing
}

...which, in such a distinctive context, I tend to format as...

if short_var { short_thing }
else { other_short_thing }
1 Like

I consider rustfmt harmful.
But clippy is useful as long as you disable style warnings/errors

ssokolow,

I really wish people would not squish things into a line like that for no useful reason. Putting the conditions and the alternatives on separate lines keeps things nice and clear.

DoumanAsh

And others...

Well there we have it. There seems to be little hope of getting programmers to agree on a single style. Despite most of them agreeing that everyone working to the same style would be a good thing. Despite the community spirit the Rust project emphasizes all the time. There are just too many free spirits that want to do weird things.

OvermindDL1,

This almost sounds like a good reason to change my mind on using tabs despite the fact they are an abomination.

Except... I just spent an hour discussing this with a woman that has been teaching the visually impaired and blind her whole life, was director of a school for the blind and has written books on the subject. She is also very highly regarded among here former students.

Conclusion: She said use spaces!

Ah well.

1 Like

To be honest, I consider having to make that judgment call a code smell. Normally, I'm willing to do a fair bit of re-architecting of my functions to avoid that.

However, when I can't, it's the best of a bunch of bad options.

I have a strong aversion to C/C++ styles which place brackets other than the terminal one alone on their own lines and, when the if condition and the single expressions in the case bodies are that short, it becomes such a vertical smear-out that it triggers those same impulses.

I considered alternatives such as a macro or an extension trait, but the idea of having such things anywhere other than std triggered my UNIX programmer "don't over-abstract and split your code into a million tiny bits" instincts even worse.

Well there we have it. There seems to be little hope of getting programmers to agree on a single style. Despite most of them agreeing that everyone working to the same style would be a good thing. Despite the community spirit the Rust project emphasizes all the time. There are just too many free spirits that want to do weird things.

I think we've done a good job of getting everyone to agree to gravitate toward a constellation of very similar styles, centered around stuff we feel we can agree upon, but we don't think the last mile is worth the pain when rustfmt has the potential to be a central component for a "rustfmt to personal style before editing, then rustfmt back to project style before committing" workflow.

2 Likes

By way of a example, what is your typing speed? It's all about characters per second which can be much lower with some disabilities.

To experience the feeling try typing one line in binary.

Then just indent a line using tabs vs spaces. Ahhhhh....

"Tabs should never be used for alignment, only spaces, even if that means tabbing over to the indentation area then spacing over to align."

This best practice is to always tab out first and any remaining spaces are purely alignment. IE it should be used consistently.

1 Like

You have no right to expect people to agree on style.
I do not want to do weird things, I want to do things that work.
If you want to force style on people, then force it in compiler, but then people wouldn't just use such compiler, right?
So take a hint and consider that not everyone want to think/like the same.

DHorse2,

As far as I can tell programmer productivity is not limited by their typing speed. As it happens my typing speed is very low, but that is of no consequence as the speed at which I can think about what I want to type is another order of magnitude slower.

I used to think it was just me but famously many studies have estimated that software engineers produce in the order of 10 lines of code per day. An estimate that I have observed in the projects I have been involved with over the years.

It's never about characters per second.

Besides, when I want to indent I hit the TAB key, my editor emits 4 spaces. Similarly when removing indents, BACKSPACE removes 4 spaces. So using spaces for indent does not mean more key strokes.

I'm not sure I under stand the suggested experiment there.

1 Like