1
Fork 0

Fix wrong suggestions for T:

This commit fixes a corner case in `suggest_constraining_type_params`
that was causing incorrect suggestions.

For the following functions:
```rust
fn a<T:>(t: T) { [t, t]; }
fn b<T>(t: T) where T: { [t, t]; }
```
We previously suggested the following:
```text
...
help: consider restricting type parameter `T`
  |
1 | fn a<T: Copy:>(t: T) { [t, t]; }
  |       ++++++
...
help: consider further restricting this bound
  |
2 | fn b<T>(t: T) where T: + Copy { [t, t]; }
  |                        ++++++
```
Note that neither `T: Copy:` not `where T: + Copy` is a correct bound.

With this commit the suggestions are correct:
```text
...
help: consider restricting type parameter `T`
  |
1 | fn a<T: Copy>(t: T) { [t, t]; }
  |         ++++
...
help: consider further restricting this bound
  |
2 | fn b<T>(t: T) where T: Copy { [t, t]; }
  |                        ++++
```
This commit is contained in:
Maybe Waffle 2022-04-12 11:29:23 +04:00
parent fa72316031
commit de2edb226b
2 changed files with 63 additions and 8 deletions

View file

@ -17,7 +17,7 @@ use rustc_error_messages::MultiSpan;
use rustc_index::vec::IndexVec; use rustc_index::vec::IndexVec;
use rustc_macros::HashStable_Generic; use rustc_macros::HashStable_Generic;
use rustc_span::hygiene::MacroKind; use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::Spanned; use rustc_span::source_map::{SourceMap, Spanned};
use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{def_id::LocalDefId, BytePos, Span, DUMMY_SP}; use rustc_span::{def_id::LocalDefId, BytePos, Span, DUMMY_SP};
use rustc_target::asm::InlineAsmRegOrRegClass; use rustc_target::asm::InlineAsmRegOrRegClass;
@ -523,6 +523,40 @@ impl<'hir> GenericParam<'hir> {
}) })
.map(|sp| sp.shrink_to_hi()) .map(|sp| sp.shrink_to_hi())
} }
/// Returns the span of `:` after a generic parameter.
///
/// For example:
///
/// ```text
/// fn a<T:>()
/// ^
/// | here
/// here |
/// v
/// fn b<T :>()
///
/// fn c<T
///
/// :>()
/// ^
/// |
/// here
/// ```
pub fn colon_span_for_suggestions(&self, source_map: &SourceMap) -> Option<Span> {
let sp = source_map
.span_extend_while(self.span.shrink_to_hi(), |c| c.is_whitespace() || c == ':')
.ok()?;
let snippet = source_map.span_to_snippet(sp).ok()?;
let offset = snippet.find(':')?;
let colon_sp = sp
.with_lo(BytePos(sp.lo().0 + offset as u32))
.with_hi(BytePos(sp.lo().0 + (offset + ':'.len_utf8()) as u32));
Some(colon_sp)
}
} }
#[derive(Default)] #[derive(Default)]

View file

@ -336,10 +336,14 @@ pub fn suggest_constraining_type_params<'a>(
} }
let constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>().join(" + "); let constraint = constraints.iter().map(|&(c, _)| c).collect::<Vec<_>>().join(" + ");
let mut suggest_restrict = |span| { let mut suggest_restrict = |span, bound_list_non_empty| {
suggestions.push(( suggestions.push((
span, span,
format!(" + {}", constraint), if bound_list_non_empty {
format!(" + {}", constraint)
} else {
format!(" {}", constraint)
},
SuggestChangingConstraintsMessage::RestrictBoundFurther, SuggestChangingConstraintsMessage::RestrictBoundFurther,
)) ))
}; };
@ -360,7 +364,10 @@ pub fn suggest_constraining_type_params<'a>(
// | // |
// replace with: `impl Foo + Bar` // replace with: `impl Foo + Bar`
suggest_restrict(param.span.shrink_to_hi()); // `impl Trait` must have at least one trait in the list
let bound_list_non_empty = true;
suggest_restrict(param.span.shrink_to_hi(), bound_list_non_empty);
continue; continue;
} }
@ -383,15 +390,25 @@ pub fn suggest_constraining_type_params<'a>(
// -- // --
// | // |
// replace with: `T: Bar +` // replace with: `T: Bar +`
suggest_restrict(span);
// `bounds_span_for_suggestions` returns `None` if the list is empty
let bound_list_non_empty = true;
suggest_restrict(span, bound_list_non_empty);
} else { } else {
let (colon, span) = match param.colon_span_for_suggestions(tcx.sess.source_map()) {
// If there is already a colon after generic, do not suggest adding it again
Some(sp) => ("", sp.shrink_to_hi()),
None => (":", param.span.shrink_to_hi()),
};
// If user hasn't provided any bounds, suggest adding a new one: // If user hasn't provided any bounds, suggest adding a new one:
// //
// fn foo<T>(t: T) { ... } // fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo` // - help: consider restricting this type parameter with `T: Foo`
suggestions.push(( suggestions.push((
param.span.shrink_to_hi(), span,
format!(": {}", constraint), format!("{colon} {constraint}"),
SuggestChangingConstraintsMessage::RestrictType { ty: param_name }, SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
)); ));
} }
@ -459,17 +476,21 @@ pub fn suggest_constraining_type_params<'a>(
)); ));
} else { } else {
let mut param_spans = Vec::new(); let mut param_spans = Vec::new();
let mut non_empty = false;
for predicate in generics.where_clause.predicates { for predicate in generics.where_clause.predicates {
if let WherePredicate::BoundPredicate(WhereBoundPredicate { if let WherePredicate::BoundPredicate(WhereBoundPredicate {
span, span,
bounded_ty, bounded_ty,
bounds,
.. ..
}) = predicate }) = predicate
{ {
if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind { if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind {
if let Some(segment) = path.segments.first() { if let Some(segment) = path.segments.first() {
if segment.ident.to_string() == param_name { if segment.ident.to_string() == param_name {
non_empty = !bounds.is_empty();
param_spans.push(span); param_spans.push(span);
} }
} }
@ -478,7 +499,7 @@ pub fn suggest_constraining_type_params<'a>(
} }
match param_spans[..] { match param_spans[..] {
[&param_span] => suggest_restrict(param_span.shrink_to_hi()), [&param_span] => suggest_restrict(param_span.shrink_to_hi(), non_empty),
_ => { _ => {
suggestions.push(( suggestions.push((
generics.where_clause.tail_span_for_suggestion(), generics.where_clause.tail_span_for_suggestion(),