Rollup merge of #95970 - WaffleLapkin:nicer_trait_suggestions, r=compiler-errors
Fix suggestions in case of `T:` bounds
This PR 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]; }
| ++++
```
r? `@compiler-errors`
I've tried fixing #95898 here too, but got too confused with how `suggest_traits_to_import` works and what it does :sweat_smile:
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index 1511807..76971d7 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -17,7 +17,7 @@
use rustc_index::vec::IndexVec;
use rustc_macros::HashStable_Generic;
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::{def_id::LocalDefId, BytePos, Span, DUMMY_SP};
use rustc_target::asm::InlineAsmRegOrRegClass;
@@ -523,6 +523,40 @@ pub fn bounds_span_for_suggestions(&self) -> Option<Span> {
})
.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)]
diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs
index 49d0ce5..3b044b1 100644
--- a/compiler/rustc_middle/src/ty/diagnostics.rs
+++ b/compiler/rustc_middle/src/ty/diagnostics.rs
@@ -336,10 +336,14 @@ pub fn suggest_constraining_type_params<'a>(
}
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((
span,
- format!(" + {}", constraint),
+ if bound_list_non_empty {
+ format!(" + {}", constraint)
+ } else {
+ format!(" {}", constraint)
+ },
SuggestChangingConstraintsMessage::RestrictBoundFurther,
))
};
@@ -360,7 +364,10 @@ pub fn suggest_constraining_type_params<'a>(
// |
// 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;
}
@@ -383,15 +390,25 @@ pub fn suggest_constraining_type_params<'a>(
// --
// |
// 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 {
+ 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:
//
// fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo`
suggestions.push((
- param.span.shrink_to_hi(),
- format!(": {}", constraint),
+ span,
+ format!("{colon} {constraint}"),
SuggestChangingConstraintsMessage::RestrictType { ty: param_name },
));
}
@@ -459,17 +476,21 @@ pub fn suggest_constraining_type_params<'a>(
));
} else {
let mut param_spans = Vec::new();
+ let mut non_empty = false;
for predicate in generics.where_clause.predicates {
if let WherePredicate::BoundPredicate(WhereBoundPredicate {
span,
bounded_ty,
+ bounds,
..
}) = predicate
{
if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind {
if let Some(segment) = path.segments.first() {
if segment.ident.to_string() == param_name {
+ non_empty = !bounds.is_empty();
+
param_spans.push(span);
}
}
@@ -478,7 +499,7 @@ pub fn suggest_constraining_type_params<'a>(
}
match param_spans[..] {
- [¶m_span] => suggest_restrict(param_span.shrink_to_hi()),
+ [¶m_span] => suggest_restrict(param_span.shrink_to_hi(), non_empty),
_ => {
suggestions.push((
generics.where_clause.tail_span_for_suggestion(),
diff --git a/src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed b/src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed
index d31046c..fb6cf7e 100644
--- a/src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed
+++ b/src/test/ui/moves/use_of_moved_value_copy_suggestions.fixed
@@ -69,4 +69,18 @@
(t, t) //~ use of moved value: `t`
}
+#[rustfmt::skip]
+fn existing_colon<T: Copy>(t: T) {
+ //~^ HELP consider restricting type parameter `T`
+ [t, t]; //~ use of moved value: `t`
+}
+
+fn existing_colon_in_where<T>(t: T)
+where
+ T: Copy,
+ //~^ HELP consider further restricting this bound
+{
+ [t, t]; //~ use of moved value: `t`
+}
+
fn main() {}
diff --git a/src/test/ui/moves/use_of_moved_value_copy_suggestions.rs b/src/test/ui/moves/use_of_moved_value_copy_suggestions.rs
index 7cc5189..cadbf2a 100644
--- a/src/test/ui/moves/use_of_moved_value_copy_suggestions.rs
+++ b/src/test/ui/moves/use_of_moved_value_copy_suggestions.rs
@@ -69,4 +69,18 @@ fn duplicate_custom_4<T: A>(t: S<T>) -> (S<T>, S<T>)
(t, t) //~ use of moved value: `t`
}
+#[rustfmt::skip]
+fn existing_colon<T:>(t: T) {
+ //~^ HELP consider restricting type parameter `T`
+ [t, t]; //~ use of moved value: `t`
+}
+
+fn existing_colon_in_where<T>(t: T)
+where
+ T:,
+ //~^ HELP consider further restricting this bound
+{
+ [t, t]; //~ use of moved value: `t`
+}
+
fn main() {}
diff --git a/src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr b/src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr
index 8e72697..f525208 100644
--- a/src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr
+++ b/src/test/ui/moves/use_of_moved_value_copy_suggestions.stderr
@@ -142,6 +142,38 @@
LL | T: B + Trait + Copy,
| ++++++++++++++
-error: aborting due to 9 previous errors
+error[E0382]: use of moved value: `t`
+ --> $DIR/use_of_moved_value_copy_suggestions.rs:83:9
+ |
+LL | fn existing_colon_in_where<T>(t: T)
+ | - move occurs because `t` has type `T`, which does not implement the `Copy` trait
+...
+LL | [t, t];
+ | - ^ value used here after move
+ | |
+ | value moved here
+ |
+help: consider further restricting this bound
+ |
+LL | T: Copy,
+ | ++++
+
+error[E0382]: use of moved value: `t`
+ --> $DIR/use_of_moved_value_copy_suggestions.rs:75:9
+ |
+LL | fn existing_colon<T:>(t: T) {
+ | - move occurs because `t` has type `T`, which does not implement the `Copy` trait
+LL |
+LL | [t, t];
+ | - ^ value used here after move
+ | |
+ | value moved here
+ |
+help: consider restricting type parameter `T`
+ |
+LL | fn existing_colon<T: Copy>(t: T) {
+ | ++++
+
+error: aborting due to 11 previous errors
For more information about this error, try `rustc --explain E0382`.
