lint assert!(true / false)

[matthiaskrgr]

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

[Arkweid]

Take this one :)

[ordovicia]

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.

[Arkweid]

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

[flip1995]

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: Add assert(true) and assert(false) lints #3582 (comment))
• if it is a assert!(false, "_") call, suggest panic!("_") instead of just panic!() (advanced)

[Lythenas]

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

[flip1995]

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).

[Lythenas]

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")

[Lythenas]

I will need some help here.

I found

rust-clippy/clippy_lints/src/write.rs

Lines 324 to 441 in 54bf4ff

	fn check_tts<'a>(cx: &EarlyContext<'a>, tts: &TokenStream, is_write: bool) -> (Option<FmtStr>, Option<Expr>) {
	use fmt_macros::*;
	let tts = tts.clone();
	let mut parser = parser::Parser::new(&cx.sess.parse_sess, tts, None, false, false, None);
	let mut expr: Option<Expr> = None;
	if is_write {
	expr = match parser.parse_expr().map_err(|mut err| err.cancel()) {
	Ok(p) => Some(p.into_inner()),
	Err(_) => return (None, None),
	};
	// might be `writeln!(foo)`
	if parser.expect(&token::Comma).map_err(|mut err| err.cancel()).is_err() {
	return (None, expr);
	}
	}
	let (fmtstr, fmtstyle) = match parser.parse_str().map_err(|mut err| err.cancel()) {
	Ok((fmtstr, fmtstyle)) => (fmtstr.to_string(), fmtstyle),
	Err(_) => return (None, expr),
	};
	let fmtspan = parser.prev_span;
	let tmp = fmtstr.clone();
	let mut args = vec![];
	let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false);
	while let Some(piece) = fmt_parser.next() {
	if !fmt_parser.errors.is_empty() {
	return (None, expr);
	}
	if let Piece::NextArgument(arg) = piece {
	if arg.format.ty == "?" {
	// FIXME: modify rustc's fmt string parser to give us the current span
	span_lint(cx, USE_DEBUG, parser.prev_span, "use of `Debug`-based formatting");
	}
	args.push(arg);
	}
	}
	let lint = if is_write { WRITE_LITERAL } else { PRINT_LITERAL };
	let mut idx = 0;
	loop {
	const SIMPLE: FormatSpec<'_> = FormatSpec {
	fill: None,
	align: AlignUnknown,
	flags: 0,
	precision: CountImplied,
	precision_span: None,
	width: CountImplied,
	width_span: None,
	ty: "",
	};
	if !parser.eat(&token::Comma) {
	return (
	Some(FmtStr {
	contents: fmtstr,
	style: fmtstyle,
	span: fmtspan,
	}),
	expr,
	);
	}
	let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|mut err| err.cancel()) {
	expr
	} else {
	return (
	Some(FmtStr {
	contents: fmtstr,
	style: fmtstyle,
	span: fmtspan,
	}),
	None,
	);
	};
	match &token_expr.kind {
	ExprKind::Lit(_) => {
	let mut all_simple = true;
	let mut seen = false;
	for arg in &args {
	match arg.position {
	ArgumentImplicitlyIs(n) | ArgumentIs(n) => {
	if n == idx {
	all_simple &= arg.format == SIMPLE;
	seen = true;
	}
	},
	ArgumentNamed(_) => {},
	}
	}
	if all_simple && seen {
	span_lint(cx, lint, token_expr.span, "literal with an empty format string");
	}
	idx += 1;
	},
	ExprKind::Assign(lhs, rhs) => {
	if let ExprKind::Lit(_) = rhs.kind {
	if let ExprKind::Path(_, p) = &lhs.kind {
	let mut all_simple = true;
	let mut seen = false;
	for arg in &args {
	match arg.position {
	ArgumentImplicitlyIs(_) | ArgumentIs(_) => {},
	ArgumentNamed(name) => {
	if *p == name {
	seen = true;
	all_simple &= arg.format == SIMPLE;
	}
	},
	}
	}
	if all_simple && seen {
	span_lint(cx, lint, rhs.span, "literal with an empty format string");
	}
	}
	}
	},
	_ => idx += 1,
	}
	}
	}

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?

[flip1995]

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.

[CAD97]

The implementation now evaluates the constant and emits different help messages based on whether the assertion is always true or false. I think that's sufficient to close this issue.

With #11966 fixed (not linting when the assert! is always constant evaluated), assert!(TRUE) should be replaced with const _: () = assert!(TRUE); (or similar) to ensure constant evaluation, and assert!(FALSE) should (almost¹) always be a panic!ing macro instead.

Footnotes

1. The exception being "fickle" consts that vary based on cfg and/or lib versions, which could validly only want a runtime panic. ↩