Skip to content

fix(useless_format): fire on wrapped in a block-producing macro#17060

Merged
samueltardieu merged 2 commits into
rust-lang:masterfrom
shulaoda:05-23-fix_useless_format_fire_on_wrapped_in_a_block-producing_macro
May 23, 2026
Merged

fix(useless_format): fire on wrapped in a block-producing macro#17060
samueltardieu merged 2 commits into
rust-lang:masterfrom
shulaoda:05-23-fix_useless_format_fire_on_wrapped_in_a_block-producing_macro

Conversation

@shulaoda

@shulaoda shulaoda commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a false negative in useless_format: the lint silently failed to fire on format!("{}", s) when the call is the tail expression of a block produced by another macro. This affects rustc's entire define_helper!-generated family (with_forced_trimmed_paths!, with_no_trimmed_paths!, with_no_queries!, etc).

Spotted during review of #17058:

Even as a macro argument, using format!("{}", some_string) should trigger it.

Reproducer

#![feature(decl_macro)]
#![warn(clippy::useless_format)]

macro_rules! plain_mr { ($e:expr) => { $e }; }
macro_rules! block_mr { ($e:expr) => { { let _g: i32 = 0; $e } }; }
pub macro plain_dm($e:expr) { $e }
pub macro block_dm($e:expr) { { let _g: i32 = 0; $e } }

fn s() -> String { String::from("x") }

fn main() {
    let _ = format!("{}", s());                  // (1) bare         => lints
    let _ = plain_mr!(format!("{}", s()));       // (2) mr, no block => lints
    let _ = block_mr!(format!("{}", s()));       // (3) mr + block   => MISS (bug)
    let _ = plain_dm!(format!("{}", s()));       // (4) dm, no block => lints
    let _ = block_dm!(format!("{}", s()));       // (5) dm + block   => MISS (bug)
}

Before this PR, cases (3) and (5) are silently missed. After this PR all five fire as expected. Note that the discriminator is block-wrapping, not macro_rules vs decl_macro.

Root cause

clippy_lints/src/format.rs previously used root_macro_call_first_node, which requires first_node_in_macro == Some(ExpnId::root()). For block_dm!(format!(...)), the format! HIR parent is the Block emitted by block_dm, whose span lives in block_dm's expansion (sibling to format!'s). first_node_in_macro returns Some(block_dm.expn) rather than Some(root), and the lint bails.

Fix

Replace the gate with:

if let Some(macro_call) = matching_root_macro_call(cx, expr.span, sym::format_macro)
    && first_node_in_macro(cx, expr).is_some_and(|p_expn| p_expn != macro_call.expn)
  • matching_root_macro_call preserves hygiene. The outermost macro on expr.span's backtrace must be format!, so a format! written inside another macro's body (where the rewrite would target the macro definition) is still ignored.
  • first_node_in_macro(..).is_some_and(|p| p != macro_call.expn) preserves single-firing. expr must be the outermost node of format!'s expansion. Without p != macro_call.expn, deeper nodes whose parent is also in format!'s expansion (including internal format_args! invocations) would slip through and the lint would fire multiple times per call.

Test changes

  • tests/ui/format.{rs,fixed,stderr}: added #![feature(decl_macro)] and a block_wrap module covering all four pass-through and block-wrap combinations across macro_rules! and decl_macro, as regression coverage.
  • tests/ui/unused_format_specs.{rs,1.fixed,2.fixed}: added clippy::useless_format to the allow-list. The relaxed gate also starts firing on println!("{:.3}", format!("abcde"))-style cases, which appear in this test's .fixed outputs (after unused_format_specs suggests format_args! to format!). This is a latent true positive previously masked by the strict gate. The test is scoped to unused_format_specs and should not be entangled with another lint's coverage. The same pattern is already used by tests/ui/format.rs itself, which allows other unrelated lints.

Verification

  • cargo test --test compile-test: 1764 UI tests + 188 fixed checks + 46 ui-cargo tests pass. 0 duplicate diagnostics.
  • cargo test --test dogfood: catches the same false negative in clippy's own unnecessary_literal_unwrap.rs (the line that useless_format misses format! inside block-wrapping macros #17059 is fixing manually), demonstrating the fix on real-world code.

Related

changelog: [useless_format] no longer misses format! calls that are the tail expression of a block produced by another macro (e.g. rustc's with_forced_trimmed_paths!).

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2026
@rustbot

rustbot commented May 23, 2026

Copy link
Copy Markdown
Collaborator

r? @Jarcho

rustbot has assigned @Jarcho.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, llogiq, samueltardieu

@shulaoda shulaoda marked this pull request as draft May 23, 2026 06:32
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2026
@shulaoda

Copy link
Copy Markdown
Contributor Author

r? @samueltardieu

@samueltardieu samueltardieu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include a commit before the current one which removes the use of format!() in unnecessary_literal_unwrap.rs? This way, the build stays clean at any point.

View changes since this review

Comment thread clippy_lints/src/format.rs Outdated
@rustbot

rustbot commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label May 23, 2026
Comment thread tests/ui/unused_format_specs.rs Outdated
@shulaoda shulaoda force-pushed the 05-23-fix_useless_format_fire_on_wrapped_in_a_block-producing_macro branch from f08f793 to c31d970 Compare May 23, 2026 08:14
@shulaoda shulaoda marked this pull request as ready for review May 23, 2026 08:23
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2026
@shulaoda

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@samueltardieu samueltardieu added this pull request to the merge queue May 23, 2026
Merged via the queue into rust-lang:master with commit b59f804 May 23, 2026
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2026
@shulaoda

Copy link
Copy Markdown
Contributor Author

@samueltardieu Thanks for the patient review 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants