Account for new assert! desugaring in !condition suggestion

[estebank]

rustc in rust-lang/rust#122661 is going to change the desugaring of assert! to be

match condition {
    true => {}
    _ => panic!(),
}

which will make the edge-case of condition being impl Not<Output = bool> while not being bool itself no longer a straightforward suggestion, but !!condition will coerce the expression to be bool, so it can be machine applicable.

changelog: [bool_assert]: Account for new assert! desugaring in suggestion

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[samueltardieu]

Could you please add this implementation and those tests?

impl Add for ImplNotTraitWithBool {
    type Output = Self;

    fn add(self, other: Self) -> Self::Output {
        self
    }
}
[…]
    assert_eq!(!b, true);
    //~^ bool_assert_comparison
    assert_eq!(!b, false);
    //~^ bool_assert_comparison
    assert_eq!(b + b, true);
    //~^ bool_assert_comparison
    assert_eq!(b + b, false);
    //~^ bool_assert_comparison

Right now, the assert_eq!(b + b, true) will fail once fixed because it would be transformed into assert!(!!b + b). The proposed change fixes that.

assert_eq!(!b, false); will get an extra pair of parentheses once fixed assert!(!(!b)) but this will be rare and can be ignored as it is only an aesthetic issue.

[samueltardieu]

Suggested change

	if let ty::Bool = non_lit_ty.kind() {
	if bool_value ^ eq_macro {
	let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) else {
	return;
	};
	suggestions.push((non_lit_expr.span, (!sugg).to_string()));
	}
	} else {
	// If we have a `value` that is *not* `bool` but that `!value` *is*, we suggest
	// `!!value`.
	suggestions.push((
	non_lit_expr.span.shrink_to_lo(),
	if bool_value ^ eq_macro {
	"!".to_string()
	} else {
	"!!".to_string()
	},
	));
	}
	let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) else {
	return;
	};
	let sugg = if bool_value ^ eq_macro {
	!sugg.maybe_paren()
	} else if ty::Bool == *non_lit_ty.kind() {
	sugg
	} else {
	!!sugg.maybe_paren()
	};
	suggestions.push((non_lit_expr.span, sugg.to_string()));

I would even remove the early return here with:

                if let Some(sugg) = Sugg::hir_opt(cx, non_lit_expr) {
                    let sugg = if bool_value ^ eq_macro {
                        !sugg.maybe_paren()
                    } else if ty::Bool == *non_lit_ty.kind() {
                        sugg
                    } else {
                        !!sugg.maybe_paren()
                    };
                    suggestions.push((non_lit_expr.span, sugg.to_string()));

                    diag.multipart_suggestion(
                        format!("replace it with `{non_eq_mac}!(..)`"),
                        suggestions,
                        Applicability::MachineApplicable,
                    );
                }

[estebank]

Addressed and squashed

[samueltardieu]

LGTM

Concerning the process: if we merge it here, you'll have to wait at least until the next sync PR (which will be submitted on Aug. 21) is merged into the compiler repository, which might not be ideal since it blocks rust-lang/rust#122661.

Since the change is very localized, a possibility would be to resubmit it in r-l/r where I could approve it again through bors. This would supersede this PR and would remove a blocker sooner.

Tell me what you prefer.

[estebank]

@samueltardieu lets do the latter. I'll submit it later.

[estebank]

Closing in favor of rust-lang/rust#145273