From 04243710a0077ad140b8259c21bc1aac02057a48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 May 2020 16:31:48 -0700 Subject: [PATCH 1/2] Account for trailing comma when suggesting `where` clauses Fix #72693. --- src/librustc_middle/ty/diagnostics.rs | 21 ++++++++----------- .../traits/error_reporting/suggestions.rs | 11 +++++++++- src/test/ui/bound-suggestions.rs | 2 +- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/librustc_middle/ty/diagnostics.rs b/src/librustc_middle/ty/diagnostics.rs index 613d66d59c55b..96c36d989e928 100644 --- a/src/librustc_middle/ty/diagnostics.rs +++ b/src/librustc_middle/ty/diagnostics.rs @@ -7,7 +7,6 @@ use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::{QPath, TyKind, WhereBoundPredicate, WherePredicate}; -use rustc_span::{BytePos, Span}; impl<'tcx> TyS<'tcx> { /// Similar to `TyS::is_primitive`, but also considers inferred numeric values to be primitive. @@ -221,18 +220,16 @@ pub fn suggest_constraining_type_param( } } - let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place(); // Account for `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. - let mut trailing_comma = false; - if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) { - trailing_comma = snippet.ends_with(','); - } - let where_clause_span = if trailing_comma { - let hi = where_clause_span.hi(); - Span::new(hi - BytePos(1), hi, where_clause_span.ctxt()) - } else { - where_clause_span.shrink_to_hi() - }; + let end = generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(); + let where_clause_span = generics + .where_clause + .predicates + .last() + .map(|p| p.span()) + .unwrap_or(end) + .shrink_to_hi() + .to(end); match ¶m_spans[..] { &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 5c85855535e38..eb281b270ff2a 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -169,8 +169,17 @@ pub trait InferCtxtExt<'tcx> { } fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) { + let end = generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(); ( - generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(), + // Account for `where T: Foo,` so we don't suggest two trailing commas. + generics + .where_clause + .predicates + .last() + .map(|p| p.span()) + .unwrap_or(end) + .shrink_to_hi() + .to(end), format!( "{} {}", if !generics.where_clause.predicates.is_empty() { "," } else { " where" }, diff --git a/src/test/ui/bound-suggestions.rs b/src/test/ui/bound-suggestions.rs index 605a6df838658..562dec9f080de 100644 --- a/src/test/ui/bound-suggestions.rs +++ b/src/test/ui/bound-suggestions.rs @@ -19,7 +19,7 @@ fn test_one_bound(t: T) { } #[allow(dead_code)] -fn test_no_bounds_where(x: X, y: Y) where X: std::fmt::Debug { +fn test_no_bounds_where(x: X, y: Y) where X: std::fmt::Debug, { println!("{:?} {:?}", x, y); //~^ ERROR doesn't implement } From 0d18136b776cd7cda2c4932f447e6484377f42b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 May 2020 09:44:41 -0700 Subject: [PATCH 2/2] Move common code to `WhereClause` --- src/librustc_hir/hir.rs | 7 +++++++ src/librustc_middle/ty/diagnostics.rs | 13 +------------ .../traits/error_reporting/suggestions.rs | 11 +---------- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index ef398ab25d3fb..a46871ac80798 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -525,6 +525,13 @@ impl WhereClause<'_> { pub fn span_for_predicates_or_empty_place(&self) -> Span { self.span } + + /// `Span` where further predicates would be suggested, accounting for trailing commas, like + /// in `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. + pub fn tail_span_for_suggestion(&self) -> Span { + let end = self.span_for_predicates_or_empty_place().shrink_to_hi(); + self.predicates.last().map(|p| p.span()).unwrap_or(end).shrink_to_hi().to(end) + } } /// A single predicate in a where-clause. diff --git a/src/librustc_middle/ty/diagnostics.rs b/src/librustc_middle/ty/diagnostics.rs index 96c36d989e928..ba3cc0a7af2c6 100644 --- a/src/librustc_middle/ty/diagnostics.rs +++ b/src/librustc_middle/ty/diagnostics.rs @@ -220,22 +220,11 @@ pub fn suggest_constraining_type_param( } } - // Account for `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. - let end = generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(); - let where_clause_span = generics - .where_clause - .predicates - .last() - .map(|p| p.span()) - .unwrap_or(end) - .shrink_to_hi() - .to(end); - match ¶m_spans[..] { &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), _ => { err.span_suggestion_verbose( - where_clause_span, + generics.where_clause.tail_span_for_suggestion(), &msg_restrict_type_further, format!(", {}: {}", param_name, constraint), Applicability::MachineApplicable, diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index eb281b270ff2a..a8d4de873f958 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -169,17 +169,8 @@ pub trait InferCtxtExt<'tcx> { } fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) { - let end = generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(); ( - // Account for `where T: Foo,` so we don't suggest two trailing commas. - generics - .where_clause - .predicates - .last() - .map(|p| p.span()) - .unwrap_or(end) - .shrink_to_hi() - .to(end), + generics.where_clause.tail_span_for_suggestion(), format!( "{} {}", if !generics.where_clause.predicates.is_empty() { "," } else { " where" },