Rollup merge of rust-lang#136368 - estebank:listify, r=fee1-dead

Make comma separated lists of anything easier to make for errors

Provide a new function `listify`, meant to be used in cases similar to `pluralize!`. When you have a slice of arbitrary elements that need to be presented to the user, `listify` allows you to turn that into a list of comma separated strings.

This reduces a lot of redundant logic that happens often in diagnostics.

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -4,7 +4,7 @@ use std::collections::BTreeMap;
 
 use rustc_abi::{FieldIdx, VariantIdx};
 use rustc_data_structures::fx::FxIndexMap;
-use rustc_errors::{Applicability, Diag, EmissionGuarantee, MultiSpan};
+use rustc_errors::{Applicability, Diag, EmissionGuarantee, MultiSpan, listify};
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::{self as hir, CoroutineKind, LangItem};
 use rustc_index::IndexSlice;
@@ -29,7 +29,7 @@ use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
 use rustc_trait_selection::error_reporting::traits::call_kind::{CallDesugaringKind, call_kind};
 use rustc_trait_selection::infer::InferCtxtExt;
 use rustc_trait_selection::traits::{
-    FulfillmentErrorCode, type_known_to_meet_bound_modulo_regions,
+    FulfillmentError, FulfillmentErrorCode, type_known_to_meet_bound_modulo_regions,
 };
 use tracing::debug;
 
@@ -1433,17 +1433,15 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
                                             error.obligation.predicate,
                                         )
                                     }
-                                    [errors @ .., last] => {
+                                    _ => {
                                         format!(
                                             "you could `clone` the value and consume it, if the \
-                                             following trait bounds could be satisfied: \
-                                             {} and `{}`",
-                                            errors
-                                                .iter()
-                                                .map(|e| format!("`{}`", e.obligation.predicate))
-                                                .collect::<Vec<_>>()
-                                                .join(", "),
-                                            last.obligation.predicate,
+                                             following trait bounds could be satisfied: {}",
+                                            listify(&errors, |e: &FulfillmentError<'tcx>| format!(
+                                                "`{}`",
+                                                e.obligation.predicate
+                                            ))
+                                            .unwrap(),
                                         )
                                     }
                                 };

compiler/rustc_builtin_macros/src/format.rs

@@ -8,7 +8,9 @@ use rustc_ast::{
     token,
 };
 use rustc_data_structures::fx::FxHashSet;
-use rustc_errors::{Applicability, Diag, MultiSpan, PResult, SingleLabelManySpans};
+use rustc_errors::{
+    Applicability, Diag, MultiSpan, PResult, SingleLabelManySpans, listify, pluralize,
+};
 use rustc_expand::base::*;
 use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
 use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiag, LintId};
@@ -975,15 +977,11 @@ fn report_invalid_references(
         } else {
             MultiSpan::from_spans(invalid_refs.iter().filter_map(|&(_, span, _, _)| span).collect())
         };
-        let arg_list = if let &[index] = &indexes[..] {
-            format!("argument {index}")
-        } else {
-            let tail = indexes.pop().unwrap();
-            format!(
-                "arguments {head} and {tail}",
-                head = indexes.into_iter().map(|i| i.to_string()).collect::<Vec<_>>().join(", ")
-            )
-        };
+        let arg_list = format!(
+            "argument{} {}",
+            pluralize!(indexes.len()),
+            listify(&indexes, |i: &usize| i.to_string()).unwrap_or_default()
+        );
         e = ecx.dcx().struct_span_err(
             span,
             format!("invalid reference to positional {arg_list} ({num_args_desc})"),

compiler/rustc_errors/src/lib.rs

@@ -65,7 +65,7 @@ pub use rustc_error_messages::{
     SubdiagMessage, fallback_fluent_bundle, fluent_bundle,
 };
 use rustc_lint_defs::LintExpectationId;
-pub use rustc_lint_defs::{Applicability, pluralize};
+pub use rustc_lint_defs::{Applicability, listify, pluralize};
 use rustc_macros::{Decodable, Encodable};
 pub use rustc_span::ErrorGuaranteed;
 pub use rustc_span::fatal_error::{FatalError, FatalErrorMarker};
@@ -1999,18 +1999,6 @@ pub fn a_or_an(s: &str) -> &'static str {
     }
 }
 
-/// Grammatical tool for displaying messages to end users in a nice form.
-///
-/// Take a list ["a", "b", "c"] and output a display friendly version "a, b and c"
-pub fn display_list_with_comma_and<T: std::fmt::Display>(v: &[T]) -> String {
-    match v {
-        [] => "".to_string(),
-        [a] => a.to_string(),
-        [a, b] => format!("{a} and {b}"),
-        [a, v @ ..] => format!("{a}, {}", display_list_with_comma_and(v)),
-    }
-}
-
 #[derive(Clone, Copy, PartialEq, Hash, Debug)]
 pub enum TerminalUrl {
     No,

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -3,7 +3,7 @@ use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::unord::UnordMap;
 use rustc_errors::codes::*;
 use rustc_errors::{
-    Applicability, Diag, ErrorGuaranteed, MultiSpan, pluralize, struct_span_code_err,
+    Applicability, Diag, ErrorGuaranteed, MultiSpan, listify, pluralize, struct_span_code_err,
 };
 use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
@@ -808,14 +808,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             .map(|(trait_, mut assocs)| {
                 assocs.sort();
                 let trait_ = trait_.print_trait_sugared();
-                format!("{} in `{trait_}`", match &assocs[..] {
-                    [] => String::new(),
-                    [only] => format!("`{only}`"),
-                    [assocs @ .., last] => format!(
-                        "{} and `{last}`",
-                        assocs.iter().map(|a| format!("`{a}`")).collect::<Vec<_>>().join(", ")
-                    ),
-                })
+                format!(
+                    "{} in `{trait_}`",
+                    listify(&assocs[..], |a| format!("`{a}`")).unwrap_or_default()
+                )
             })
             .collect::<Vec<String>>();
         names.sort();
@@ -1075,18 +1071,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 }
             })
             .collect();
-        let this_type = match &types_and_spans[..] {
-            [.., _, (last, _)] => format!(
-                "{} and {last}",
-                types_and_spans[..types_and_spans.len() - 1]
-                    .iter()
-                    .map(|(x, _)| x.as_str())
-                    .intersperse(", ")
-                    .collect::<String>()
-            ),
-            [(only, _)] => only.to_string(),
-            [] => bug!("expected one segment to deny"),
-        };
+        let this_type = listify(&types_and_spans, |(t, _)| t.to_string())
+            .expect("expected one segment to deny");
 
         let arg_spans: Vec<Span> = segments
             .clone()
@@ -1102,21 +1088,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             ProhibitGenericsArg::Infer => kinds.push("generic"),
         });
 
-        let (kind, s) = match kinds[..] {
-            [.., _, last] => (
-                format!(
-                    "{} and {last}",
-                    kinds[..kinds.len() - 1]
-                        .iter()
-                        .map(|&x| x)
-                        .intersperse(", ")
-                        .collect::<String>()
-                ),
-                "s",
-            ),
-            [only] => (only.to_string(), ""),
-            [] => bug!("expected at least one generic to prohibit"),
-        };
+        let s = pluralize!(kinds.len());
+        let kind =
+            listify(&kinds, |k| k.to_string()).expect("expected at least one generic to prohibit");
         let last_span = *arg_spans.last().unwrap();
         let span: MultiSpan = arg_spans.into();
         let mut err = struct_span_code_err!(

compiler/rustc_hir_typeck/src/demand.rs

@@ -1,4 +1,4 @@
-use rustc_errors::{Applicability, Diag, MultiSpan};
+use rustc_errors::{Applicability, Diag, MultiSpan, listify};
 use rustc_hir as hir;
 use rustc_hir::def::Res;
 use rustc_hir::intravisit::Visitor;
@@ -1016,18 +1016,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 },
                 self.tcx.def_path_str(candidate.item.container_id(self.tcx))
             ),
-            [.., last] if other_methods_in_scope.len() < 5 => {
+            _ if other_methods_in_scope.len() < 5 => {
                 format!(
-                    "the methods of the same name on {} and `{}`",
-                    other_methods_in_scope[..other_methods_in_scope.len() - 1]
-                        .iter()
-                        .map(|c| format!(
-                            "`{}`",
-                            self.tcx.def_path_str(c.item.container_id(self.tcx))
-                        ))
-                        .collect::<Vec<String>>()
-                        .join(", "),
-                    self.tcx.def_path_str(last.item.container_id(self.tcx))
+                    "the methods of the same name on {}",
+                    listify(
+                        &other_methods_in_scope[..other_methods_in_scope.len() - 1],
+                        |c| format!("`{}`", self.tcx.def_path_str(c.item.container_id(self.tcx)))
+                    )
+                    .unwrap_or_default(),
                 )
             }
             _ => format!(

compiler/rustc_hir_typeck/src/expr.rs

@@ -11,7 +11,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::unord::UnordMap;
 use rustc_errors::codes::*;
 use rustc_errors::{
-    Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, Subdiagnostic, pluralize,
+    Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, Subdiagnostic, listify, pluralize,
     struct_span_code_err,
 };
 use rustc_hir::def::{CtorKind, DefKind, Res};
@@ -2190,13 +2190,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
             if !missing_mandatory_fields.is_empty() {
                 let s = pluralize!(missing_mandatory_fields.len());
-                let fields: Vec<_> =
-                    missing_mandatory_fields.iter().map(|f| format!("`{f}`")).collect();
-                let fields = match &fields[..] {
-                    [] => unreachable!(),
-                    [only] => only.to_string(),
-                    [start @ .., last] => format!("{} and {last}", start.join(", ")),
-                };
+                let fields = listify(&missing_mandatory_fields, |f| format!("`{f}`")).unwrap();
                 self.dcx()
                     .struct_span_err(
                         span.shrink_to_hi(),
@@ -2553,25 +2547,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             .partition(|field| field.2);
         err.span_labels(used_private_fields.iter().map(|(_, span, _)| *span), "private field");
         if !remaining_private_fields.is_empty() {
-            let remaining_private_fields_len = remaining_private_fields.len();
-            let names = match &remaining_private_fields
-                .iter()
-                .map(|(name, _, _)| name)
-                .collect::<Vec<_>>()[..]
-            {
-                _ if remaining_private_fields_len > 6 => String::new(),
-                [name] => format!("`{name}` "),
-                [names @ .., last] => {
-                    let names = names.iter().map(|name| format!("`{name}`")).collect::<Vec<_>>();
-                    format!("{} and `{last}` ", names.join(", "))
-                }
-                [] => bug!("expected at least one private field to report"),
+            let names = if remaining_private_fields.len() > 6 {
+                String::new()
+            } else {
+                format!(
+                    "{} ",
+                    listify(&remaining_private_fields, |(name, _, _)| format!("`{name}`"))
+                        .expect("expected at least one private field to report")
+                )
             };
             err.note(format!(
                 "{}private field{s} {names}that {were} not provided",
                 if used_fields.is_empty() { "" } else { "...and other " },
-                s = pluralize!(remaining_private_fields_len),
-                were = pluralize!("was", remaining_private_fields_len),
+                s = pluralize!(remaining_private_fields.len()),
+                were = pluralize!("was", remaining_private_fields.len()),
             ));
         }
 

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -4,8 +4,7 @@ use itertools::Itertools;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::codes::*;
 use rustc_errors::{
-    Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, a_or_an,
-    display_list_with_comma_and, pluralize,
+    Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, a_or_an, listify, pluralize,
 };
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::DefId;
@@ -2462,7 +2461,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 param.span,
                                 format!(
                                     "{} need{} to match the {} type of this parameter",
-                                    display_list_with_comma_and(&other_param_matched_names),
+                                    listify(&other_param_matched_names, |n| n.to_string())
+                                        .unwrap_or_default(),
                                     pluralize!(if other_param_matched_names.len() == 1 {
                                         0
                                     } else {
@@ -2477,7 +2477,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 format!(
                                     "this parameter needs to match the {} type of {}",
                                     matched_ty,
-                                    display_list_with_comma_and(&other_param_matched_names),
+                                    listify(&other_param_matched_names, |n| n.to_string())
+                                        .unwrap_or_default(),
                                 ),
                             );
                         }
@@ -2523,7 +2524,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             generic_param.span,
                             format!(
                                 "{} {} reference this parameter `{}`",
-                                display_list_with_comma_and(&param_idents_matching),
+                                listify(&param_idents_matching, |n| n.to_string())
+                                    .unwrap_or_default(),
                                 if param_idents_matching.len() == 2 { "both" } else { "all" },
                                 generic_param.name.ident().name,
                             ),

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -4,7 +4,7 @@ use core::iter;
 use hir::def_id::LocalDefId;
 use rustc_ast::util::parser::ExprPrecedence;
 use rustc_data_structures::packed::Pu128;
-use rustc_errors::{Applicability, Diag, MultiSpan};
+use rustc_errors::{Applicability, Diag, MultiSpan, listify};
 use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{
@@ -1836,16 +1836,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 error.obligation.predicate,
                             ));
                         }
-                        [errors @ .., last] => {
+                        _ => {
                             diag.help(format!(
                                 "`Clone` is not implemented because the following trait bounds \
-                                 could not be satisfied: {} and `{}`",
-                                errors
-                                    .iter()
-                                    .map(|e| format!("`{}`", e.obligation.predicate))
-                                    .collect::<Vec<_>>()
-                                    .join(", "),
-                                last.obligation.predicate,
+                                 could not be satisfied: {}",
+                                listify(&errors, |e| format!("`{}`", e.obligation.predicate))
+                                    .unwrap(),
                             ));
                         }
                     }

compiler/rustc_lint_defs/src/lib.rs

@@ -42,6 +42,23 @@ macro_rules! pluralize {
     };
 }
 
+/// Grammatical tool for displaying messages to end users in a nice form.
+///
+/// Take a list of items and a function to turn those items into a `String`, and output a display
+/// friendly comma separated list of those items.
+// FIXME(estebank): this needs to be changed to go through the translation machinery.
+pub fn listify<T>(list: &[T], fmt: impl Fn(&T) -> String) -> Option<String> {
+    Some(match list {
+        [only] => fmt(&only),
+        [others @ .., last] => format!(
+            "{} and {}",
+            others.iter().map(|i| fmt(i)).collect::<Vec<_>>().join(", "),
+            fmt(&last),
+        ),
+        [] => return None,
+    })
+}
+
 /// Indicates the confidence in the correctness of a suggestion.
 ///
 /// All suggestions are marked with an `Applicability`. Tools use the applicability of a suggestion

compiler/rustc_middle/src/ty/diagnostics.rs

@@ -5,7 +5,7 @@ use std::ops::ControlFlow;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{
-    Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display, pluralize,
+    Applicability, Diag, DiagArgValue, IntoDiagArg, into_diag_arg_using_display, listify, pluralize,
 };
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
@@ -362,11 +362,8 @@ pub fn suggest_constraining_type_params<'a>(
             let n = trait_names.len();
             let stable = if all_stable { "" } else { "unstable " };
             let trait_ = if all_known { format!("trait{}", pluralize!(n)) } else { String::new() };
-            format!("{stable}{trait_}{}", match &trait_names[..] {
-                [t] => format!(" {t}"),
-                [ts @ .., last] => format!(" {} and {last}", ts.join(", ")),
-                [] => return false,
-            },)
+            let Some(trait_names) = listify(&trait_names, |n| n.to_string()) else { return false };
+            format!("{stable}{trait_} {trait_names}")
         } else {
             // We're more explicit when there's a mix of stable and unstable traits.
             let mut trait_names = constraints
@@ -378,10 +375,9 @@ pub fn suggest_constraining_type_params<'a>(
                 .collect::<Vec<_>>();
             trait_names.sort();
             trait_names.dedup();
-            match &trait_names[..] {
-                [t] => t.to_string(),
-                [ts @ .., last] => format!("{} and {last}", ts.join(", ")),
-                [] => return false,
+            match listify(&trait_names, |t| t.to_string()) {
+                Some(names) => names,
+                None => return false,
             }
         };
         let constraint = constraint.join(" + ");