Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make comma separated lists of anything easier to make for errors #136368

Merged
merged 1 commit into from
Feb 2, 2025
Merged

Make comma separated lists of anything easier to make for errors #136368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -1436,17 +1436,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(),
)
}
};
Expand Down
18 changes: 8 additions & 10 deletions compiler/rustc_builtin_macros/src/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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})"),
Expand Down
14 changes: 1 addition & 13 deletions compiler/rustc_errors/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)),
}
}
Comment on lines -2002 to -2012
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this because it becomes redundant in the face of the new function, and it is less flexible (only works with pre-made lists that are displayable, but doesn't let you customize the format of each item, it is equivalent to listify(v, |i| i.to_string())).


#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum TerminalUrl {
No,
Expand Down
46 changes: 10 additions & 36 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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()
Expand All @@ -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!(
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_hir_typeck/src/demand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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!(
Expand Down
35 changes: 12 additions & 23 deletions compiler/rustc_hir_typeck/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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()),
));
}

Expand Down
12 changes: 7 additions & 5 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
),
);
}
Expand Down Expand Up @@ -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,
),
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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(),
));
}
}
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 6 additions & 10 deletions compiler/rustc_middle/src/ty/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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(" + ");
Expand Down
Loading
Loading