Split assert by jcarey9149 · Pull Request #16677 · rust-lang/rust-clippy

jcarey9149 · 2026-03-06T19:31:01Z

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: ['assert_multiple']: Add an assert for asserts with mulitple boolean tests

jcarey9149 · 2026-03-06T19:31:56Z

Sadly, I accidentally deleted the branch connected to my previous PR, causing it to close. @ada4a

github-actions · 2026-03-06T19:41:08Z

Lintcheck changes for c38698e

Lint	Added	Removed	Changed
`clippy::assert_multiple`	46	0	0

This comment will be updated if you push new changes

ada4a · 2026-03-07T07:29:44Z

Sadly, I accidentally deleted the branch connected to my previous PR, causing it to close.

Happens to the best of us, don't worry...

Looking at Lintcheck, there seem to be some classes of cases where we don't really want to lint:

the assertion has a message – it usually presents information about everything that is asserted, so we shouldn't be splitting that up; and even if it didn't, we wouldn't know which of the newly assert we should move the message into
assertions of the form n < x && x < m – splitting these would look unnatural imo, but I guess that's the whole premise of the lint... not sure, let me know what you think

ada4a

A bit more review.. I hope this will resolve the mystery of the missing suggestions

View changes since this review

clippy_lints/src/assert_multiple.rs

ada4a · 2026-03-07T09:05:41Z

clippy_lints/src/assert_multiple.rs

+    // which is represented by `'v` (the HIR lifetime `'tcx` refers to the
+    // data inside the `LateContext`, not the borrow itself).
+    cx: &'v LateContext<'tcx>,
+    suggest_asserts: &'v mut Vec<String>,


at this point you might as well just own the vec -- would make lifetime management easier

clippy_lints/src/assert_multiple.rs

jcarey9149 · 2026-03-07T20:32:42Z

Sadly, I accidentally deleted the branch connected to my previous PR, causing it to close.

Happens to the best of us, don't worry...

Looking at Lintcheck, there seem to be some classes of cases where we don't really want to lint:

the assertion has a message – it usually presents information about everything that is asserted, so we shouldn't be splitting that up; and even if it didn't, we wouldn't know which of the newly assert we should move the message into

assertions of the form n < x && x < m – splitting these would look unnatural imo, but I guess that's the whole premise of the lint... not sure, let me know what you think

I'm not sure I understand the comment about lintcheck. can you clarify please?

This is based on issue 1810.

ada4a · 2026-03-07T20:36:22Z

Ah, sorry. Lintcheck is a thing where the PR is run on a bunch of popular crates, in order to see if the lint is doing the right thing, and catch false positives. You can see its results in #16677 (comment) by clicking on the clippy::assert_multiple thing

clippy_lints/src/assert_multiple.rs

ada4a · 2026-03-08T08:53:43Z

clippy_lints/src/assert_multiple.rs

+// the visitor needs a mutable reference to a vector that lives
+// only for the duration of a single `check_expr` invocation.  we
+// therefore introduce a separate lifetime `'v` for that borrow.


the comment is outdated now, feel free to remove/rephrase

clippy_lints/src/assert_multiple.rs

ada4a · 2026-03-08T09:01:29Z

clippy_lints/src/assert_multiple.rs

+impl<'tcx> LateLintPass<'tcx> for AssertMultiple {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) {
+        if let Some(macro_call) = root_macro_call_first_node(cx, e)
+            && matches!(cx.tcx.get_diagnostic_name(macro_call.def_id), Some(sym::assert_macro))


This could be left as future work, but I don't think there's anything stopping this lint from also working with debug_assert!?

clippy_lints/src/assert_multiple.rs

ada4a · 2026-03-08T18:16:10Z

clippy_lints/src/assert_multiple.rs

+                self.suggests.push(tmptxt);
+            },
+
+            _ => {},


I think this will end up eating the subexpressions which don't match any of the kinds above? For example, given a: i32 and is_foo: bool, the following code:

assert!(a > 0 && is_foo);

would become just

assert!(a > 0);

Right?

ada4a · 2026-03-08T18:18:01Z

in the changelog message, the lint name should be in backticks (`), not in quotes (')

The wrong `TypeckResults` was used in the fallback equality function passed by the `match_same_arms` and `filter_map` lints. Previously, those fallback functions had no way of using the proper `TypeckResults`. Those (one per expression being compared) are now passed to the registered fallback function.

It is not necessary to materialize snippets into `String` objects just to check if they contain comments for example.

jcarey9149 · 2026-03-09T16:54:44Z

My latest update passes most tests, but the clippy test complains about collapsing my span_lint_and_then call into one (?) line. cargo dev fmt has a different idea.

How does this get resolved?

On the other front, it now supports debug_assert!() as well as assert!(). I also improved the handling of Or.

ada4a · 2026-03-09T17:25:20Z

the clippy test complains about collapsing my span_lint_and_then call into one (?) line

Ah no, it also suggests switching to span_lint_and_sugg (instead of span_lint_and_then). The reason it fires is because the then closure only contains one operation. This is an internal lint I personally dislike, but that's a topic for another time. For now, you can copy its suggestion:

span_lint_and_sugg(cx, ASSERT_MULTIPLE, e.span, "multiple asserts combined into one", "consider writing", am_visitor.suggests.join("\n"), Applicability::MaybeIncorrect)

ada4a · 2026-03-09T17:27:18Z

clippy_lints/src/assert_multiple.rs

+    #[clippy::version = "1.95.0"]
+    pub ASSERT_MULTIPLE,
+    nursery,
+    "Splitting an assert using '&&' into separate asserts makes it clearer which is failing."


Suggested change

"Splitting an assert using '&&' into separate asserts makes it clearer which is failing."

"Splitting an assert using `&&` into separate asserts makes it clearer which is failing."

ada4a · 2026-03-09T17:29:52Z

clippy_lints/src/assert_multiple.rs

+            && matches!(
+                cx.tcx.get_diagnostic_name(macro_call.def_id),
+                Some(sym::assert_macro | sym::debug_assert_macro)
+            )


to avoid matching on the diagnostic name again below, you can do something like this:

Suggested change

&& matches!(

cx.tcx.get_diagnostic_name(macro_call.def_id),

Some(sym::assert_macro | sym::debug_assert_macro)

)

&& let assert_string = match cx.tcx.get_diagnostic_name(macro_call.def_id) {

Some(sym::assert_macro) => "assert",

Some(sym::debug_assert_macro) => "debug_assert",

_ => return,

)

Note that this also uses &'static strs instead of owned Strings, since you don't really need ownership here.

ada4a · 2026-03-09T17:35:21Z

clippy_lints/src/assert_multiple.rs

+                    "multiple asserts combined into one",
+                    |diag| {
+                        diag.span_suggestion(
+                            e.span,


ohhhhh, I think I finally know why you haven't been getting any suggestions! I think it's because e here is a macro call, so its .span might be pointing to the definition of assert!, which is why it doesn't get shown. Try replacing this with macro_call.span

ada4a · 2026-03-09T17:37:59Z

Also, I'm not sure how you ended up putting a bunch of other people's commits onto your branch?.. Try git pull --rebase && git rebase master or something

jcarey9149 added 7 commits March 6, 2026 14:24

checkpoint

b8b77e1

add call

44aecbc

checkpoint

2578569

checkpoint

3a377c3

checkpoint

ad206f7

suggested cleanup

1b52792

handle suggestions

1412099

rustbot added the needs-fcp PRs that add, remove, or rename lints and need an FCP label Mar 6, 2026

ada4a suggested changes Mar 7, 2026

View reviewed changes

jcarey9149 added 2 commits March 7, 2026 12:57

handle suggestions

9628673

reformat

92210c3

ada4a suggested changes Mar 7, 2026

View reviewed changes

clippy_lints/src/assert_multiple.rs Outdated Show resolved Hide resolved

jcarey9149 added 4 commits March 7, 2026 17:58

remove binary or

d4b6fde

remove debug output

abc270e

broken lifetime problem

5eba71d

broken lifetime problem

a11f426

jcarey9149 commented Mar 8, 2026

View reviewed changes

clippy_lints/src/assert_multiple.rs Show resolved Hide resolved

jcarey9149 added 3 commits March 7, 2026 19:57

some lint cleanup

c78a952

more linting

47aac09

more lints

02d6097

ada4a suggested changes Mar 8, 2026

View reviewed changes

samueltardieu added 2 commits March 9, 2026 12:25

Fix ICE when using the min_generic_const_args incomplete feature

b25dd42

samueltardieu and others added 4 commits March 9, 2026 12:25

Do not materialize snippets when it is not needed to

2e783e6

It is not necessary to materialize snippets into `String` objects just to check if they contain comments for example.

Remove one DOM level for lint additional info

2c0d8e3

Remove unused label-default CSS class

0a96a61

more tests, support debug_assert!()

c38698e

ada4a suggested changes Mar 9, 2026

View reviewed changes

	"Splitting an assert using '&&' into separate asserts makes it clearer which is failing."
	"Splitting an assert using `&&` into separate asserts makes it clearer which is failing."

-            && matches!(
-                cx.tcx.get_diagnostic_name(macro_call.def_id),
-                Some(sym::assert_macro | sym::debug_assert_macro)
-            )
+            && let assert_string = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
+                Some(sym::assert_macro) => "assert",
+                Some(sym::debug_assert_macro) => "debug_assert",
+                _ => return,
+            )

Conversation

jcarey9149 commented Mar 6, 2026

Uh oh!

jcarey9149 commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ada4a commented Mar 7, 2026

Uh oh!

ada4a left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ada4a Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcarey9149 commented Mar 7, 2026

Uh oh!

ada4a commented Mar 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ada4a Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ada4a Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ada4a Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

ada4a commented Mar 8, 2026

Uh oh!

jcarey9149 commented Mar 9, 2026

Uh oh!

ada4a commented Mar 9, 2026

Uh oh!

ada4a Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

ada4a Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ada4a Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

ada4a commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Mar 6, 2026 •

edited

Loading

ada4a left a comment •

edited by rustbot

Loading

ada4a Mar 9, 2026 •

edited

Loading

ada4a commented Mar 9, 2026 •

edited

Loading