Add assert message to suggestion in lint assertions_on_constants

[Lythenas]

• suggest replacing assert!(false, "msg") with panic!("msg")
• extend to allow variables any expression for "msg"
• suggest replacing assert!(false, "msg {}", "arg") with panic!("msg {}", "arg")

changelog: add assert message to suggestion in lint assertions_on_constants

Work towards fixing: #3575

[Lythenas]

Currently I just reused the help message. I'm not 100% sure if this should be a suggestion.

[flip1995]

You created the if_chain! with the author lint, right? These if_chains normally can e simplified. If that's not possible, it would be great if you could use more telling names, than block2, lit3, ...

[flip1995]

Also you should be able to reuse this code from here to the end of the file:

rust-clippy/clippy_lints/src/format.rs

Line 73 in 54bf4ff

fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arms: &'a [Arm]) -> Option<String> {

[Lythenas]

You created the if_chain! with the author lint, right? These if_chains normally can e simplified. If that's not possible, it would be great if you could use more telling names, than block2, lit3, ...

Yes I created it originally from the code expanded by -Zunpretty=hir. But I had to change a few thing to get it to work.

I will change the names and try to simplify a bit. I'm not 100% sure what can be simplified and what's important but I guess the risk of someone accidentally writing the same code the macro generates are very slim (or maybe impossible because of the DropTemps and $crate in the path to begin_panic).

Should I for example keep things like ExprKind::Match(..., MatchSource::IfDesugar { contains_else_clause: false }) or simplify them to just ExprKind::Match(..., _)?

[flip1995]

I've gone over the if_chain and left you some comments on things, that I think can be removed or should be renamed.

Since the match is pretty specific for this expansion if you only match on the "happy path", that leads to the begin_panic call, I don't think that you also have to match the unrelated expanded parts. Also DropTemps is only possible in expanded code and not in hand written code (IIUC), so we should be good here.

[flip1995]

Maybe this would be worth adding to the utils.

Yes, that would be great! While your at it, can you also remove this function:

rust-clippy/clippy_lints/src/utils/mod.rs

Lines 362 to 364 in 737f0a6

	pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> Res {
	cx.tables.qpath_res(qpath, id)
	}

and use cx.tables.qpath_res(..) directly?

On the other hand, I think this refactor should be done in another PR. So

1. Remove resolve_node(..)
2. Use cx.tables.qpath_res(..) directly
3. Put match_function_call in utils
4. grep for match_def_path and replace it with match_function_call where applicable.

I think this is too big of a change for this unrelated PR.

[flip1995]

I'd suggest to do the refactor from my comment above and

suggest replacing assert!(false, "msg {}", "arg") with panic!("msg {}", "arg")

in another PR.

If you want to, you can do

extend to allow variables for "msg"

in this PR or also in another PR, as off your choosing.

[Lythenas]

I think I will do the following in this PR:

• Keep the match_function_call in assertions_on_constants.rs
• replace resolve_node in assertions_on_constants.rs
• and finish up suggest replacing assert!(false, "msg") with panic!("msg")
• extend to allow variables for "msg"

And then split the rest into other PRs:

1. • Move match_function_call to utils
   • Use match_function_call in other lints where possible
   • remove resolve_node (maybe also in a separate PR)
2. • suggest replacing assert!(false, "msg {}", "arg") with panic!("msg {}", "arg")

[flip1995]

Thanks!

@bors r+

[bors]

📋 Looks like this PR is still in progress, ignoring approval.

Hint: Remove WIP from this PR's title when it is ready for review.

[flip1995]

ping @Lythenas IIUC this PR is finished. Since this is still marked as WIP, are there still open points you want to address?

[Lythenas]

Sorry forgot to remove the WIP. I'm all done here.

[flip1995]

Thanks!

@bors r+

[bors]

📌 Commit 6ee8d75 has been approved by flip1995

[bors]

⌛ Testing commit 6ee8d75 with merge d97fbdb...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing d97fbdb to master...