Rust-clippy: lint assert!(true / false)

Created on 23 Dec 2018  路  9Comments  路  Source: rust-lang/rust-clippy

assert!(true); will be optimized out by the compiler
assert!(false); should probably be replaced by a panic!().

A-suggestion L-enhancement good-first-issue hacktoberfest

All 9 comments

Take this one :)

How about replacing assert!(false) with unreachable!()?
I guess programmers often use assert!(false) and equivalent in other languages with the intention that the code is unreachable.
Of course panic!() is more versatile, so it may be better to use panic!() for the replacement.

PR added :)
@matthiaskrgr can you check? Especially the error messages, I'm not sure what the description is as it should be

After #3582 was merged this lint can be enhanced by suggestions for assert(true/false):

  • suggest ""/"panic!()" (should be easy, but some weird things were going on: https://github.com/rust-lang/rust-clippy/pull/3582#discussion_r246656947)
  • if it is a assert!(false, "_") call, suggest panic!("_") instead of just panic!() (advanced)

I would like to try and implement the first suggestion (assert(true) -> "" and assert(false) -> panic!()). And after maybe the second suggestion too.

Just for clarification:

assert(true) -> "" and assert(false) -> panic!()

suggesting "" instead of assert doesn't make sense. Then I saw, that I recommended to do this:

suggest ""/"panic!()"

What I meant with that is, to suggest removing the assert completely if it is assert!(true) and to suggest panic!() if it is assert!(false).

Right. I didn't think that far. Also I notices the first part is already implemented so I will start on the second part assert!(false, "msg") -> panic/unreachable!("msg")

I will need some help here.

I found
https://github.com/rust-lang/rust-clippy/blob/54bf4ffd626970e831bb80c037f804a3b3450835/clippy_lints/src/write.rs#L324-L441

But it uses EarlyContext and the assertions_on_constants is a LateLintPass. Is there still some way to reuse this? Or am I look at this the wrong way?

assertions_on_constants is a LateLintPass

Yes, that's why I wrote (advanced) behind this. What you need to do is to look at the expanded code. You can do this with the rustc flag -Zunpretty=hir.

For example:

fn main() {
    assert!(false, "some {} things", "String");
}

expands to

#[prelude_import]
use ::std::prelude::v1::*;
#[macro_use]
extern crate std;
fn main() {
              match { let _t = !false; _t } {
                  true => {
                      {
                          ::std::rt::begin_panic_fmt(&<::core::fmt::Arguments>::new_v1(&["some ",
                                                                                         " things"],
                                                                                       &match (&"String",)
                                                                                            {
                                                                                            (arg0,)
                                                                                            =>
                                                                                            [<::core::fmt::ArgumentV1>::new(arg0,
                                                                                                                            ::core::fmt::Display::fmt)],
                                                                                        }),
                                                     &("main.rs", 2u32, 5u32))
                      }
                  }
                  _ => { }
              };
          }

You then have to match the expanded construct. Something similar was done before in clippy_lints/src/format.rs, so you may be able to reuse this code. It would also be great to have a utils function that preduces a suggestion for this core::fmt::Arguments::new_v1 things, since this is not trivial. But a first step would be to only produce suggestions for assert!s without format args. (assert!(false, "no fmt args")), since something similar is already done in clippy_lints/src/format.rs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Manishearth picture Manishearth  路  26Comments

sanmai-NL picture sanmai-NL  路  23Comments

b1zzu picture b1zzu  路  28Comments

dtolnay picture dtolnay  路  20Comments

malbarbo picture malbarbo  路  26Comments