rustdoc: Properly clean fn params in all contexts

Author: fmease

Diff for: src/librustdoc/clean/mod.rs

@@ -1103,7 +1103,10 @@ fn clean_function<'tcx>(
         let generics = clean_generics(generics, cx);
         let params = match params {
             ParamsSrc::Body(body_id) => clean_params_via_body(cx, sig.decl.inputs, body_id),
-            ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents),
+            // Let's not perpetuate anon params from Rust 2015; use `_` for them.
+            ParamsSrc::Idents(idents) => clean_params(cx, sig.decl.inputs, idents, |ident| {
+                Some(ident.map_or(kw::Underscore, |ident| ident.name))
+            }),
         };
         let decl = clean_fn_decl_with_params(cx, sig.decl, Some(&sig.header), params);
         (generics, decl)
@@ -1115,30 +1118,13 @@ fn clean_params<'tcx>(
     cx: &mut DocContext<'tcx>,
     types: &[hir::Ty<'tcx>],
     idents: &[Option<Ident>],
+    postprocess: impl Fn(Option<Ident>) -> Option<Symbol>,
 ) -> Vec<Parameter> {
-    fn nonempty_name(ident: &Option<Ident>) -> Option<Symbol> {
-        if let Some(ident) = ident
-            && ident.name != kw::Underscore
-        {
-            Some(ident.name)
-        } else {
-            None
-        }
-    }
-
-    // If at least one argument has a name, use `_` as the name of unnamed
-    // arguments. Otherwise omit argument names.
-    let default_name = if idents.iter().any(|ident| nonempty_name(ident).is_some()) {
-        Some(kw::Underscore)
-    } else {
-        None
-    };
-
     types
         .iter()
         .enumerate()
         .map(|(i, ty)| Parameter {
-            name: idents.get(i).and_then(nonempty_name).or(default_name),
+            name: postprocess(idents[i]),
             type_: clean_ty(ty, cx),
             is_const: false,
         })
@@ -1184,8 +1170,6 @@ fn clean_poly_fn_sig<'tcx>(
     did: Option<DefId>,
     sig: ty::PolyFnSig<'tcx>,
 ) -> FnDecl {
-    let mut names = did.map_or(&[] as &[_], |did| cx.tcx.fn_arg_idents(did)).iter();
-
     let mut output = clean_middle_ty(sig.output(), cx, None, None);
 
     // If the return type isn't an `impl Trait`, we can safely assume that this
@@ -1198,16 +1182,20 @@ fn clean_poly_fn_sig<'tcx>(
         output = output.sugared_async_return_type();
     }
 
+    let mut idents = did.map(|did| cx.tcx.fn_arg_idents(did)).unwrap_or_default().iter().copied();
+
+    // If this comes from a fn item, let's not perpetuate anon params from Rust 2015; use `_` for them.
+    // If this comes from a fn ptr ty, we just keep params unnamed since it's more conventional stylistically.
+    // Since the param name is not part of the semantic type, these params never bear a name unlike
+    // in the HIR case, thus we can't peform any fancy fallback logic unlike `clean_bare_fn_ty`.
+    let fallback = did.map(|_| kw::Underscore);
+
     let params = sig
         .inputs()
         .iter()
-        .map(|t| Parameter {
-            type_: clean_middle_ty(t.map_bound(|t| *t), cx, None, None),
-            name: Some(if let Some(Some(ident)) = names.next() {
-                ident.name
-            } else {
-                kw::Underscore
-            }),
+        .map(|ty| Parameter {
+            name: idents.next().flatten().map(|ident| ident.name).or(fallback),
+            type_: clean_middle_ty(ty.map_bound(|ty| *ty), cx, None, None),
             is_const: false,
         })
         .collect();
@@ -2597,7 +2585,17 @@ fn clean_bare_fn_ty<'tcx>(
             .filter(|p| !is_elided_lifetime(p))
             .map(|x| clean_generic_param(cx, None, x))
             .collect();
-        let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents);
+        // Since it's more conventional stylistically, elide the name of all params called `_`
+        // unless there's at least one interestingly named param in which case don't elide any
+        // name since mixing named and unnamed params is less legible.
+        let filter = |ident: Option<Ident>| {
+            ident.map(|ident| ident.name).filter(|&ident| ident != kw::Underscore)
+        };
+        let fallback =
+            bare_fn.param_idents.iter().copied().find_map(filter).map(|_| kw::Underscore);
+        let params = clean_params(cx, bare_fn.decl.inputs, bare_fn.param_idents, |ident| {
+            filter(ident).or(fallback)
+        });
         let decl = clean_fn_decl_with_params(cx, bare_fn.decl, None, params);
         (generic_params, decl)
     });

Diff for: src/librustdoc/clean/utils.rs

@@ -303,13 +303,12 @@ pub(crate) fn name_from_pat(p: &hir::Pat<'_>) -> Symbol {
     debug!("trying to get a name from pattern: {p:?}");
 
     Symbol::intern(&match &p.kind {
-        // FIXME(never_patterns): does this make sense?
-        PatKind::Missing => unreachable!(),
-        PatKind::Wild
-        | PatKind::Err(_)
+        PatKind::Err(_)
+        | PatKind::Missing // Let's not perpetuate anon params from Rust 2015; use `_` for them.
         | PatKind::Never
+        | PatKind::Range(..)
         | PatKind::Struct(..)
-        | PatKind::Range(..) => {
+        | PatKind::Wild => {
             return kw::Underscore;
         }
         PatKind::Binding(_, _, ident, _) => return ident.name,

Diff for: tests/rustdoc/anon-fn-params.rs

@@ -0,0 +1,25 @@
+// Test that we render the deprecated anonymous trait function parameters from Rust 2015 as
+// underscores in order not to perpetuate it and for legibility.
+
+//@ edition: 2015
+#![expect(anonymous_parameters)]
+
+// Check the "local case" (HIR cleaning) //
+
+//@ has anon_fn_params/trait.Trait.html
+pub trait Trait {
+    //@ has - '//*[@id="tymethod.required"]' 'fn required(_: Option<i32>, _: impl Fn(&str) -> bool)'
+    fn required(Option<i32>, impl Fn(&str) -> bool);
+    //@ has - '//*[@id="method.provided"]' 'fn provided(_: [i32; 2])'
+    fn provided([i32; 2]) {}
+}
+
+// Check the "extern case" (middle cleaning) //
+
+//@ aux-build: ext-anon-fn-params.rs
+extern crate ext_anon_fn_params;
+
+//@ has anon_fn_params/trait.ExtTrait.html
+//@ has - '//*[@id="tymethod.required"]' 'fn required(_: Option<i32>, _: impl Fn(&str) -> bool)'
+//@ has - '//*[@id="method.provided"]' 'fn provided(_: [i32; 2])'
+pub use ext_anon_fn_params::Trait as ExtTrait;

Diff for: tests/rustdoc/assoc-fns.rs

@@ -0,0 +1,13 @@
+// Basic testing for associated functions (in traits, trait impls & inherent impls).
+
+//@ has assoc_fns/trait.Trait.html
+pub trait Trait {
+    //@ has - '//*[@id="tymethod.required"]' 'fn required(first: i32, second: &str)'
+    fn required(first: i32, second: &str);
+
+    //@ has - '//*[@id="method.provided"]' 'fn provided(only: ())'
+    fn provided(only: ()) {}
+
+    //@ has - '//*[@id="tymethod.params_are_unnamed"]' 'fn params_are_unnamed(_: i32, _: u32)'
+    fn params_are_unnamed(_: i32, _: u32);
+}

Diff for: tests/rustdoc/auxiliary/ext-anon-fn-params.rs

@@ -0,0 +1,7 @@
+//@ edition: 2015
+#![expect(anonymous_parameters)]
+
+pub trait Trait {
+    fn required(Option<i32>, impl Fn(&str) -> bool);
+    fn provided([i32; 2]) {}
+}

Diff for: tests/rustdoc/ffi.rs

@@ -9,4 +9,8 @@ pub use lib::foreigner;
 extern "C" {
     //@ has ffi/fn.another.html //pre 'pub unsafe extern "C" fn another(cold_as_ice: u32)'
     pub fn another(cold_as_ice: u32);
+
+    //@ has ffi/fn.params_are_unnamed.html //pre \
+    //      'pub unsafe extern "C" fn params_are_unnamed(_: i32, _: u32)'
+    pub fn params_are_unnamed(_: i32, _: u32);
 }

Diff for: tests/rustdoc/inline_cross/auxiliary/fn-type.rs renamed to tests/rustdoc/inline_cross/auxiliary/fn-ptr-ty.rs

@@ -53,17 +53,17 @@ pub use default_generic_args::R2;
 
 //@ has user/type.H0.html
 // Check that we handle higher-ranked regions correctly:
-//@ has - '//*[@class="rust item-decl"]//code' "fn(_: for<'a> fn(_: Re<'a>))"
+//@ has - '//*[@class="rust item-decl"]//code' "fn(for<'a> fn(Re<'a>))"
 pub use default_generic_args::H0;
 
 //@ has user/type.H1.html
 // Check that we don't conflate distinct universially quantified regions (#1):
-//@ has - '//*[@class="rust item-decl"]//code' "for<'b> fn(_: for<'a> fn(_: Re<'a, &'b ()>))"
+//@ has - '//*[@class="rust item-decl"]//code' "for<'b> fn(for<'a> fn(Re<'a, &'b ()>))"
 pub use default_generic_args::H1;
 
 //@ has user/type.H2.html
 // Check that we don't conflate distinct universially quantified regions (#2):
-//@ has - '//*[@class="rust item-decl"]//code' "for<'a> fn(_: for<'b> fn(_: Re<'a, &'b ()>))"
+//@ has - '//*[@class="rust item-decl"]//code' "for<'a> fn(for<'b> fn(Re<'a, &'b ()>))"
 pub use default_generic_args::H2;
 
 //@ has user/type.P0.html
@@ -86,7 +86,7 @@ pub use default_generic_args::A0;
 // Demonstrates that we currently don't elide generic arguments that are alpha-equivalent to their
 // respective generic parameter (after instantiation) for perf reasons (it would require us to
 // create an inference context).
-//@ has - '//*[@class="rust item-decl"]//code' "Alpha<for<'arbitrary> fn(_: &'arbitrary ())>"
+//@ has - '//*[@class="rust item-decl"]//code' "Alpha<for<'arbitrary> fn(&'arbitrary ())>"
 pub use default_generic_args::A1;
 
 //@ has user/type.M0.html

Diff for: tests/rustdoc/inline_cross/default-generic-args.rs

@@ -2,11 +2,11 @@
 // They should be rendered exactly as the user wrote it, i.e., in source order and with unused
 // parameters present, not stripped.
 
-//@ aux-crate:fn_type=fn-type.rs
+//@ aux-crate:fn_ptr_ty=fn-ptr-ty.rs
 //@ edition: 2021
 #![crate_name = "user"]
 
 //@ has user/type.F.html
 //@ has - '//*[@class="rust item-decl"]//code' \
-//     "for<'z, 'a, '_unused> fn(_: &'z for<'b> fn(_: &'b str), _: &'a ()) -> &'a ();"
-pub use fn_type::F;
+//     "for<'z, 'a, '_unused> fn(&'z for<'b> fn(&'b str), &'a ()) -> &'a ();"
+pub use fn_ptr_ty::F;

Diff for: tests/rustdoc/inline_cross/fn-type.rs renamed to tests/rustdoc/inline_cross/fn-ptr-ty.rs

@@ -29,7 +29,7 @@ pub use impl_trait_aux::func4;
 //@ has impl_trait/fn.func5.html
 //@ has - '//pre[@class="rust item-decl"]' "func5("
 //@ has - '//pre[@class="rust item-decl"]' "_f: impl for<'any> Fn(&'any str, &'any str) -> bool + for<'r> Other<T<'r> = ()>,"
-//@ has - '//pre[@class="rust item-decl"]' "_a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(_: &'beta ())>"
+//@ has - '//pre[@class="rust item-decl"]' "_a: impl for<'beta, 'alpha, '_gamma> Auxiliary<'alpha, Item<'beta> = fn(&'beta ())>"
 //@ !has - '//pre[@class="rust item-decl"]' 'where'
 pub use impl_trait_aux::func5;