Store the names of `Fn` trait parameters in the AST, fix rustfmt bug

Project: rust-lang/rust | Language: Rust | Concepts: compiler-internals, api-design, backwards-compatibility, refactoring
Difficulty: senior
Link: https://github.com/rust-lang/rust/pull/160937

The problem

Previously, the Rust compiler's Abstract Syntax Tree (AST) did not retain the names of parameters for Fn trait types (e.g., impl Fn(msg: String)). This caused issues for tools like rustfmt, which need access to parameter names to format code correctly, and limited future semantic validation in the compiler.

Why it's non-trivial

A naive fix would simply add parameter names to the AST. However, this impacts many parts of the compiler and tooling, all of which assume that only types (not full parameters) are stored for Fn trait arguments. Care must be taken to update all relevant code paths, maintain backward compatibility, and avoid breaking the grammar or introducing regressions in formatting and analysis tools.

The approach

1. Change the AST to store full parameters (not just types) for parenthesized arguments

-    pub inputs: ThinVec<Box<Ty>>,
+    pub inputs: ThinVec<Param>,

What it does:
Changes the ParenthesizedArgs struct to store a vector of Param (which includes names, patterns, and attributes), instead of just types. This is the core data model change.

2. Update code that processes these arguments to handle `Param` instead of just `Ty`

Lowering to HIR:

-        let inputs = self.arena.alloc_from_iter(inputs.iter().map(|ty| {
-            self.lower_ty(ty, ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitParam))
+        let inputs = self.arena.alloc_from_iter(inputs.iter().map(|param| {
+            self.lower_ty(&param.ty, ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitParam))
         }));

What it does:
When lowering AST to HIR, extract the type from each Param for further processing, since only the type is needed at this stage.

AST Validation:

-                walk_list!(self, visit_ty, &data.inputs);
+                walk_list!(self, visit_param, &data.inputs);

What it does:
Ensures that validation walks over full parameters, not just types.

Pretty-printing:

-                self.commasep(Inconsistent, &data.inputs, |s, ty| s.print_type(ty));
+                self.commasep(Inconsistent, &data.inputs, |s, param| s.print_param(param, false));

What it does:
Updates pretty-printer to print parameters (with names) instead of just types.

3. Update the parser to produce `Param` for these arguments

-                        param.ty
+                        Ok(param)

What it does:
The parser now returns a full Param (with name, pattern, etc.) for each argument, not just the type.

4. Update all downstream consumers and utilities

Clippy utils:

-            over(&l.inputs, &r.inputs, |l, r| eq_ty(l, r)) && eq_fn_ret_ty(&l.output, &r.output)
+            over(&l.inputs, &r.inputs, eq_param) && eq_fn_ret_ty(&l.output, &r.output)

What it does:
Comparison utilities now compare full parameters, not just types.

Rustfmt:

-fn format_function_type<'a, I>(
-    inputs: I,
+fn format_function_type(
+    inputs: &[ast::Param],
...
-            inputs,
+            inputs.iter(),

What it does:
Formatting utilities now handle slices of Param, so they can access names for formatting.

Test case added:

+my_macro!(f(x: &str));
+my_macro!(f(x: &str) -> ());
+my_macro!(g(n: i32, m: usize) -> usize);

What it does:
Adds regression tests to ensure named parameters in macros and function traits are parsed and formatted correctly.

From the review thread

@ytmimi Thank you, again. I took a look at the rustfmt code changes and they all look very straightforward to me. Having the rustfmt target test case there will help us avoid any regressions and make sure that the formatting stays idempotent.

Assuming that CI passes and the timing run doesn't show any performance impacts from this change you can r=me.

@bors try @rust-timer queue
This should not have any effect, but changing the AST representation (although only a small part of it) deserves a sanity check

Takeaways

AST changes ripple through the entire toolchain: When changing a core data structure like the AST, all consumers (parsing, lowering, formatting, linting, etc.) must be updated to maintain correctness and avoid regressions.
Favor storing richer information early: Keeping parameter names (not just types) in the AST enables better tooling and future semantic checks, at the cost of slightly more complexity in the compiler pipeline.
Comprehensive regression tests are essential: Adding targeted tests ensures that changes to core representations do not break formatting, parsing, or macro expansion in subtle ways.

Store the names of Fn trait parameters in the AST, fix rustfmt bug