1
Fork 0

Implement @jackh726's suggestions

This commit is contained in:
Fabian Wolff 2021-05-09 22:31:49 +02:00
parent 439ef6d762
commit 3c0c3874fc
5 changed files with 67 additions and 79 deletions

View file

@ -1821,21 +1821,19 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
crate fn add_missing_lifetime_specifiers_label( crate fn add_missing_lifetime_specifiers_label(
&self, &self,
err: &mut DiagnosticBuilder<'_>, err: &mut DiagnosticBuilder<'_>,
spans: Vec<Span>, spans_with_counts: Vec<(Span, usize)>,
counts: Vec<usize>,
lifetime_names: &FxHashSet<Symbol>, lifetime_names: &FxHashSet<Symbol>,
lifetime_spans: Vec<Span>, lifetime_spans: Vec<Span>,
params: &[ElisionFailureInfo], params: &[ElisionFailureInfo],
) { ) {
let snippets: Vec<Option<String>> = spans let snippets: Vec<Option<String>> = spans_with_counts
.iter() .iter()
.copied() .map(|(span, _)| self.tcx.sess.source_map().span_to_snippet(*span).ok())
.map(|span| self.tcx.sess.source_map().span_to_snippet(span).ok())
.collect(); .collect();
for (span, count) in spans.iter().zip(counts.iter()) { for (span, count) in &spans_with_counts {
err.span_label( err.span_label(
span.clone(), *span,
format!( format!(
"expected {} lifetime parameter{}", "expected {} lifetime parameter{}",
if *count == 1 { "named".to_string() } else { count.to_string() }, if *count == 1 { "named".to_string() } else { count.to_string() },
@ -1847,7 +1845,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
let suggest_existing = let suggest_existing =
|err: &mut DiagnosticBuilder<'_>, |err: &mut DiagnosticBuilder<'_>,
name: &str, name: &str,
formatters: &Vec<Option<Box<dyn Fn(&str) -> String>>>| { formatters: Vec<Option<Box<dyn Fn(&str) -> String>>>| {
if let Some(MissingLifetimeSpot::HigherRanked { span: for_span, span_type }) = if let Some(MissingLifetimeSpot::HigherRanked { span: for_span, span_type }) =
self.missing_named_lifetime_spots.iter().rev().next() self.missing_named_lifetime_spots.iter().rev().next()
{ {
@ -1892,9 +1890,9 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
} }
} }
introduce_suggestion.push((*for_span, for_sugg)); introduce_suggestion.push((*for_span, for_sugg));
for (span, formatter) in spans.iter().copied().zip(formatters.iter()) { for ((span, _), formatter) in spans_with_counts.iter().zip(formatters.iter()) {
if let Some(formatter) = formatter { if let Some(formatter) = formatter {
introduce_suggestion.push((span, formatter(&lt_name))); introduce_suggestion.push((*span, formatter(&lt_name)));
} }
} }
err.multipart_suggestion_with_style( err.multipart_suggestion_with_style(
@ -1905,12 +1903,12 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
); );
} }
let mut spans_suggs: Vec<_> = Vec::new(); let spans_suggs: Vec<_> = formatters
for (span, fmt) in spans.iter().copied().zip(formatters.iter()) { .into_iter()
if let Some(formatter) = fmt { .filter_map(|fmt| fmt)
spans_suggs.push((span, formatter(name))); .zip(spans_with_counts.iter())
} .map(|(formatter, (span, _))| (*span, formatter(name)))
} .collect();
err.multipart_suggestion_with_style( err.multipart_suggestion_with_style(
&format!( &format!(
"consider using the `{}` lifetime", "consider using the `{}` lifetime",
@ -1921,7 +1919,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
SuggestionStyle::ShowAlways, SuggestionStyle::ShowAlways,
); );
}; };
let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: &Vec<Option<String>>| { let suggest_new = |err: &mut DiagnosticBuilder<'_>, suggs: Vec<Option<String>>| {
for missing in self.missing_named_lifetime_spots.iter().rev() { for missing in self.missing_named_lifetime_spots.iter().rev() {
let mut introduce_suggestion = vec![]; let mut introduce_suggestion = vec![];
let msg; let msg;
@ -1967,8 +1965,8 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
} }
MissingLifetimeSpot::Static => { MissingLifetimeSpot::Static => {
let mut spans_suggs = Vec::new(); let mut spans_suggs = Vec::new();
for ((span, snippet), count) in for ((span, count), snippet) in
spans.iter().copied().zip(snippets.iter()).zip(counts.iter().copied()) spans_with_counts.iter().copied().zip(snippets.iter())
{ {
let (span, sugg) = match snippet.as_deref() { let (span, sugg) = match snippet.as_deref() {
Some("&") => (span.shrink_to_hi(), "'static ".to_owned()), Some("&") => (span.shrink_to_hi(), "'static ".to_owned()),
@ -2018,7 +2016,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
} }
} }
} }
for (span, sugg) in spans.iter().copied().zip(suggs.iter()) { for ((span, _), sugg) in spans_with_counts.iter().copied().zip(suggs.iter()) {
if let Some(sugg) = sugg { if let Some(sugg) = sugg {
introduce_suggestion.push((span, sugg.to_string())); introduce_suggestion.push((span, sugg.to_string()));
} }
@ -2039,68 +2037,57 @@ impl<'tcx> LifetimeContext<'_, 'tcx> {
match &lifetime_names[..] { match &lifetime_names[..] {
[name] => { [name] => {
let mut suggs: Vec<Option<Box<dyn Fn(&str) -> String>>> = Vec::new(); let mut suggs: Vec<Option<Box<dyn Fn(&str) -> String>>> = Vec::new();
for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) { for (snippet, (_, count)) in snippets.iter().zip(spans_with_counts.iter().copied())
if snippet == Some("&".to_string()) { {
suggs.push(Some(Box::new(|name| format!("&{} ", name)))); suggs.push(match snippet.as_deref() {
} else if snippet == Some("'_".to_string()) { Some("&") => Some(Box::new(|name| format!("&{} ", name))),
suggs.push(Some(Box::new(|n| n.to_string()))); Some("'_") => Some(Box::new(|n| n.to_string())),
} else if snippet == Some("".to_string()) { Some("") => Some(Box::new(move |n| format!("{}, ", n).repeat(count))),
suggs.push(Some(Box::new(move |n| format!("{}, ", n).repeat(count)))); Some(snippet) if !snippet.ends_with('>') => Some(Box::new(move |name| {
} else if let Some(snippet) = snippet { format!(
if !snippet.ends_with('>') {
suggs.push(Some(Box::new(move |name| {
format!(
"{}<{}>",
snippet,
std::iter::repeat(name.to_string())
.take(count)
.collect::<Vec<_>>()
.join(", ")
)
})));
} else {
suggs.push(None);
}
} else {
suggs.push(None);
}
}
suggest_existing(err, &name.as_str()[..], &suggs);
}
[] => {
let mut suggs: Vec<Option<String>> = Vec::new();
for (snippet, count) in snippets.iter().cloned().zip(counts.iter().copied()) {
if snippet == Some("&".to_string()) {
suggs.push(Some("&'a ".to_string()));
} else if snippet == Some("'_".to_string()) {
suggs.push(Some("'a".to_string()));
} else if let Some(snippet) = snippet {
if snippet == "" {
suggs.push(Some(
std::iter::repeat("'a, ").take(count).collect::<Vec<_>>().join(""),
));
} else {
suggs.push(Some(format!(
"{}<{}>", "{}<{}>",
snippet, snippet,
std::iter::repeat("'a").take(count).collect::<Vec<_>>().join(", ") std::iter::repeat(name.to_string())
))); .take(count)
} .collect::<Vec<_>>()
} else { .join(", ")
suggs.push(None); )
} })),
_ => None,
});
} }
suggest_new(err, &suggs); suggest_existing(err, &name.as_str()[..], suggs);
}
[] => {
let mut suggs = Vec::new();
for (snippet, (_, count)) in
snippets.iter().cloned().zip(spans_with_counts.iter().copied())
{
suggs.push(match snippet.as_deref() {
Some("&") => Some("&'a ".to_string()),
Some("'_") => Some("'a".to_string()),
Some("") => {
Some(std::iter::repeat("'a, ").take(count).collect::<Vec<_>>().join(""))
}
Some(snippet) => Some(format!(
"{}<{}>",
snippet,
std::iter::repeat("'a").take(count).collect::<Vec<_>>().join(", "),
)),
None => None,
});
}
suggest_new(err, suggs);
} }
lts if lts.len() > 1 => { lts if lts.len() > 1 => {
err.span_note(lifetime_spans, "these named lifetimes are available to use"); err.span_note(lifetime_spans, "these named lifetimes are available to use");
let mut spans_suggs: Vec<_> = Vec::new(); let mut spans_suggs: Vec<_> = Vec::new();
for (span, snippet) in spans.iter().copied().zip(snippets.iter()) { for ((span, _), snippet) in spans_with_counts.iter().copied().zip(snippets.iter()) {
if Some("") == snippet.as_deref() { match snippet.as_deref() {
spans_suggs.push((span, "'lifetime, ".to_string())); Some("") => spans_suggs.push((span, "'lifetime, ".to_string())),
} else if Some("&") == snippet.as_deref() { Some("&") => spans_suggs.push((span, "&'lifetime ".to_string())),
spans_suggs.push((span, "&'lifetime ".to_string())); _ => {}
} }
} }

View file

@ -3038,8 +3038,10 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
spans.sort(); spans.sort();
let mut spans_dedup = spans.clone(); let mut spans_dedup = spans.clone();
spans_dedup.dedup(); spans_dedup.dedup();
let counts: Vec<_> = let spans_with_counts: Vec<_> = spans_dedup
spans_dedup.iter().map(|sp| spans.iter().filter(|nsp| *nsp == sp).count()).collect(); .into_iter()
.map(|sp| (sp, spans.iter().filter(|nsp| *nsp == &sp).count()))
.collect();
let mut err = self.report_missing_lifetime_specifiers(spans.clone(), lifetime_refs.len()); let mut err = self.report_missing_lifetime_specifiers(spans.clone(), lifetime_refs.len());
@ -3052,8 +3054,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
self.add_missing_lifetime_specifiers_label( self.add_missing_lifetime_specifiers_label(
&mut err, &mut err,
spans, spans_with_counts,
counts,
&lifetime_names, &lifetime_names,
lifetime_spans, lifetime_spans,
error.unwrap_or(&[]), error.unwrap_or(&[]),

View file

@ -8,7 +8,7 @@ use std::path::Path;
const ENTRY_LIMIT: usize = 1000; const ENTRY_LIMIT: usize = 1000;
// FIXME: The following limits should be reduced eventually. // FIXME: The following limits should be reduced eventually.
const ROOT_ENTRY_LIMIT: usize = 1388; const ROOT_ENTRY_LIMIT: usize = 1388;
const ISSUES_ENTRY_LIMIT: usize = 2557; const ISSUES_ENTRY_LIMIT: usize = 2551;
fn check_entries(path: &Path, bad: &mut bool) { fn check_entries(path: &Path, bad: &mut bool) {
let dirs = walkdir::WalkDir::new(&path.join("test/ui")) let dirs = walkdir::WalkDir::new(&path.join("test/ui"))