Add lint for panic!("{}")

[m-ou-se]

This adds a lint that warns about panic!("{}").

panic!(msg) invocations with a single argument use their argument as panic payload literally, without using it as a format string. The same holds for assert!(expr, msg).

This lints checks if msg is a string literal (after expansion), and warns in case it contained braces. It suggests to insert "{}", to use the message literally, or to add arguments to use it as a format string.

This lint is also a good starting point for adding warnings about panic!(not_a_string) later, once panic_any() becomes a stable alternative.

[rust-highfive]

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

[matthiaskrgr]

Clippy already has a lint for panic!("{}") https://rust-lang.github.io/rust-clippy/master/index.html#panic_params (not sure about assert! though..)

[m-ou-se]

Heh, interesting. The CI build is failing, because the lint found a problem in existing Rustc code:

Looks like it's working as it should. :)

[m-ou-se]

Clippy already has a lint for panic!("{}") https://rust-lang.github.io/rust-clippy/master/index.html#panic_params (not sure about assert! though..)

Clippy does not warn for panic!("{") though, only if it actually looks like a placeholder. This lint is part of moving towards consistent behaviour for panic!() in Rust 2021, so just a Clippy lint isn't enough.

This lint should be extended later to also provide advice on panic!(123) and other uses that should be discouraged (or removed in Rust 2021).

[m-ou-se]

All the clippy issues should be fixed now. Ready for review again. :)

[estebank]

@bors r+

[bors]

📌 Commit a125ef2 has been approved by estebank

[bors]

⌛ Testing commit a125ef2 with merge 74285eb...

[bors]

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 74285eb to master...

[flip1995]

Please ping the Clippy team for major changes to Clippy in the Rust repository! We have a lengthy process (by design) in place for deprecating lints.

[estebank]

@flip1995 apologies, didn't realize that the changes to clippy were being introduced in this PR and not going through clippy's process. I'll keep it in mind for future situations.

[pthariensflame]

Does this need relnotes? Thinking of compatibility notes in particular.

[petrochenkov]

@m-ou-se
We have lint naming conventions and panic_fmt doesn't follow them.
See RFC 344 (allow(panic_fmt) => "allow panic formatting" => what?).

[m-ou-se]

@petrochenkov Good to know, thanks. Now that std::panic::panic_any is stable, this lint needs to be extended to also warn for panic!(123). At that point it'll warn for every panic that does not use format_args formatting, so we could rename it to something like non_fmt_panic.