rustfmt --help panics when stdout is closed

[lopopolo]

$ RUST_BACKTRACE=1 rustfmt --help | false
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1193:9
stack backtrace:
   0: _rust_begin_unwind
   1: std::panicking::begin_panic_fmt
   2: std::io::stdio::_print
   3: rustfmt::execute
   4: rustfmt::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
$ rustfmt --version
rustfmt 1.4.37-stable (59eed8a2 2021-11-01)

[calebcartwright]

Is there a practical/realistic case where you're encountering this in an impactful way? I ask because it seems rather contrived, and is something you'll likely encounter with any program that's using println! and friends for io

[lopopolo]

I think the issue is that CLI tools should be using write! instead of println! for output. This was a nice reference I found for the reasoning to prefer write!:

• writeln!(io::stdout()) vs println BurntSushi/advent-of-code#17 (comment)

I don't think production grade tools (which is what rustfmt is) should expose panics to users, even if they do unreasonable things.

[calebcartwright]

I can't really envision piping scenarios with the help text so I wanted to know whether this was something you were running into in practice (because that would play a factor in prioritization), or whether it was more general/philosophical.

println panics aren't new and can be found in a variety of tooling. It's quite alright (admirable even!) if you're on a mission to highlight these cases across the ecosystem. It's just not going to be a huge priority for us unless there's a real use case where it's problematic.

Panics are of course not desirable. There's no need to inform the rustfmt team of the production status of rustfmt, nor that panics should be avoided

[lopopolo]

Piping help text to grep is something fairly common.

$ rustfmt --help | grep config
        --config-path [Path for the configuration file]
                        rustfmt.toml config file. If not found reverts to the
        --print-config [default|minimal|current] PATH
                        Dumps a default or minimal config to PATH. A minimal
                        config is the subset of the current config file used
                        to stdout current config as if formatting the file at
        --config [key1=val1,key2=val2...]
                        `config`

While this doesn't panic on my machine, it is conceivable that a shell pipeline could be set up with grep and rustfmt that does cause a panic.

There's no need to inform the rustfmt team of the production status of rustfmt, nor that panics should be avoided

Apologies for the tone of my previous comment. I added the parenthetical to avoid coming across as implying that rustfmt is not a production grade tool. Thank you and the team for rustfmt.

[calebcartwright]

No worries, thanks for clarifying!

[ytmimi]

Going to close this as it's likely a duplicate of #2926