Use Option<Ident> for lowered param names.

Parameter patterns are lowered to an `Ident` by
`lower_fn_params_to_names`, which is used when lowering bare function
types, trait methods, and foreign functions. Currently, there are two
exceptional cases where the lowered param can become an empty `Ident`.

- If the incoming pattern is an empty `Ident`. This occurs if the
  parameter is anonymous, e.g. in a bare function type.

- If the incoming pattern is neither an ident nor an underscore. Any
  such parameter will have triggered a compile error (hence the
  `span_delayed_bug`), but lowering still occurs.

This commit replaces these empty `Ident` results with `None`, which
eliminates a number of `kw::Empty` uses, and makes it impossible to fail
to check for these exceptional cases.

Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It
actually should have been removed in rust-lang#138482, the precursor to this PR.
That PR changed the lowering of wild patterns to `_` symbols instead of
empty symbols, which made the mentioned underscore check load-bearing.

Author: nnethercote

clippy_lints/src/functions/renamed_function_params.rs

@@ -5,7 +5,7 @@ use rustc_hir::hir_id::OwnerId;
 use rustc_hir::{Impl, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Node, TraitRef};
 use rustc_lint::LateContext;
 use rustc_span::Span;
-use rustc_span::symbol::{Ident, Symbol, kw};
+use rustc_span::symbol::{Ident, kw};
 
 use super::RENAMED_FUNCTION_PARAMS;
 
@@ -51,22 +51,33 @@ struct RenamedFnArgs(Vec<(Span, String)>);
 impl RenamedFnArgs {
     /// Comparing between an iterator of default names and one with current names,
     /// then collect the ones that got renamed.
-    fn new<I, T>(default_names: &mut I, current_names: &mut T) -> Self
+    fn new<I1, I2>(default_idents: &mut I1, current_idents: &mut I2) -> Self
     where
-        I: Iterator<Item = Ident>,
-        T: Iterator<Item = Ident>,
+        I1: Iterator<Item = Option<Ident>>,
+        I2: Iterator<Item = Option<Ident>>,
     {
         let mut renamed: Vec<(Span, String)> = vec![];
 
-        debug_assert!(default_names.size_hint() == current_names.size_hint());
-        while let (Some(def_name), Some(cur_name)) = (default_names.next(), current_names.next()) {
-            let current_name = cur_name.name;
-            let default_name = def_name.name;
-            if is_unused_or_empty_symbol(current_name) || is_unused_or_empty_symbol(default_name) {
-                continue;
-            }
-            if current_name != default_name {
-                renamed.push((cur_name.span, default_name.to_string()));
+        debug_assert!(default_idents.size_hint() == current_idents.size_hint());
+        while let (Some(default_ident), Some(current_ident)) =
+            (default_idents.next(), current_idents.next())
+        {
+            let has_name_to_check = |ident: Option<Ident>| {
+                if let Some(ident) = ident
+                    && ident.name != kw::Underscore
+                    && !ident.name.as_str().starts_with('_')
+                {
+                    Some(ident)
+                } else {
+                    None
+                }
+            };
+
+            if let Some(default_ident) = has_name_to_check(default_ident)
+                && let Some(current_ident) = has_name_to_check(current_ident)
+                && default_ident.name != current_ident.name
+            {
+                renamed.push((current_ident.span, default_ident.to_string()));
             }
         }
 
@@ -83,14 +94,6 @@ impl RenamedFnArgs {
     }
 }
 
-fn is_unused_or_empty_symbol(symbol: Symbol) -> bool {
-    // FIXME: `body_param_names` currently returning empty symbols for `wild` as well,
-    // so we need to check if the symbol is empty first.
-    // Therefore the check of whether it's equal to [`kw::Underscore`] has no use for now,
-    // but it would be nice to keep it here just to be future-proof.
-    symbol.is_empty() || symbol == kw::Underscore || symbol.as_str().starts_with('_')
-}
-
 /// Get the [`trait_item_def_id`](ImplItemRef::trait_item_def_id) of a relevant impl item.
 fn trait_item_def_id_of_impl(items: &[ImplItemRef], target: OwnerId) -> Option<DefId> {
     items.iter().find_map(|item| {

clippy_lints/src/lifetimes.rs

@@ -189,7 +189,7 @@ fn check_fn_inner<'tcx>(
     cx: &LateContext<'tcx>,
     sig: &'tcx FnSig<'_>,
     body: Option<BodyId>,
-    trait_sig: Option<&[Ident]>,
+    trait_sig: Option<&[Option<Ident>]>,
     generics: &'tcx Generics<'_>,
     span: Span,
     report_extra_lifetimes: bool,
@@ -264,7 +264,7 @@ fn could_use_elision<'tcx>(
     cx: &LateContext<'tcx>,
     func: &'tcx FnDecl<'_>,
     body: Option<BodyId>,
-    trait_sig: Option<&[Ident]>,
+    trait_sig: Option<&[Option<Ident>]>,
     named_generics: &'tcx [GenericParam<'_>],
     msrv: Msrv,
 ) -> Option<(Vec<LocalDefId>, Vec<Lifetime>)> {
@@ -310,7 +310,7 @@ fn could_use_elision<'tcx>(
         let body = cx.tcx.hir_body(body_id);
 
         let first_ident = body.params.first().and_then(|param| param.pat.simple_ident());
-        if non_elidable_self_type(cx, func, first_ident, msrv) {
+        if non_elidable_self_type(cx, func, Some(first_ident), msrv) {
             return None;
         }
 
@@ -384,8 +384,8 @@ fn allowed_lts_from(named_generics: &[GenericParam<'_>]) -> FxIndexSet<LocalDefI
 }
 
 // elision doesn't work for explicit self types before Rust 1.81, see rust-lang/rust#69064
-fn non_elidable_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option<Ident>, msrv: Msrv) -> bool {
-    if let Some(ident) = ident
+fn non_elidable_self_type<'tcx>(cx: &LateContext<'tcx>, func: &FnDecl<'tcx>, ident: Option<Option<Ident>>, msrv: Msrv) -> bool {
+    if let Some(Some(ident)) = ident
         && ident.name == kw::SelfLower
         && !func.implicit_self.has_implicit_self()
         && let Some(self_ty) = func.inputs.first()