Skip to content

Commit 2db9b86

Browse files
estebankestebank
committed
Tweak suggest_constraining_type_param
Some of the bound restriction structured suggestions were incorrect while others had subpar output.
1 parent 3c1d9ad commit 2db9b86

File tree

66 files changed

+408
-570
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

66 files changed

+408
-570
lines changed

‎src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
222222
&mut err,
223223
&param.name.as_str(),
224224
"Copy",
225-
tcx.sess.source_map(),
226-
span,
227225
None,
228226
);
229227
}

‎src/librustc_trait_selection/traits/error_reporting/mod.rs‎

Lines changed: 46 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ use rustc_hir as hir;
2525
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
2626
use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate};
2727
use rustc_session::DiagnosticMessageId;
28-
use rustc_span::source_map::SourceMap;
29-
use rustc_span::{ExpnKind, Span, DUMMY_SP};
28+
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
3029
use std::fmt;
3130

3231
use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
@@ -1679,14 +1678,8 @@ pub fn suggest_constraining_type_param(
16791678
err: &mut DiagnosticBuilder<'_>,
16801679
param_name: &str,
16811680
constraint: &str,
1682-
source_map: &SourceMap,
1683-
span: Span,
16841681
def_id: Option<DefId>,
16851682
) -> bool {
1686-
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound with";
1687-
const MSG_RESTRICT_TYPE: &str = "consider restricting this type parameter with";
1688-
const MSG_RESTRICT_TYPE_FURTHER: &str = "consider further restricting this type parameter with";
1689-
16901683
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
16911684

16921685
let param = if let Some(param) = param {
@@ -1695,6 +1688,11 @@ pub fn suggest_constraining_type_param(
16951688
return false;
16961689
};
16971690

1691+
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
1692+
let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
1693+
let msg_restrict_type_further =
1694+
format!("consider further restricting type parameter `{}`", param_name);
1695+
16981696
if def_id == tcx.lang_items().sized_trait() {
16991697
// Type parameters are already `Sized` by default.
17001698
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
@@ -1717,19 +1715,21 @@ pub fn suggest_constraining_type_param(
17171715
// |
17181716
// replace with: `impl Foo + Bar`
17191717

1720-
err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));
1721-
1722-
err.tool_only_span_suggestion(
1723-
param.span,
1718+
err.span_suggestion_verbose(
1719+
param.span.shrink_to_hi(),
17241720
MSG_RESTRICT_BOUND_FURTHER,
1725-
format!("{} + {}", param_name, constraint),
1721+
format!(" + {}", constraint),
17261722
Applicability::MachineApplicable,
17271723
);
17281724

17291725
return true;
17301726
}
17311727

1732-
if generics.where_clause.predicates.is_empty() {
1728+
if generics.where_clause.predicates.is_empty()
1729+
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the
1730+
// `where` clause in stead of `trait Base<T: Copy = String>: Super<T>`.
1731+
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
1732+
{
17331733
if let Some(bounds_span) = param.bounds_span() {
17341734
// If user has provided some bounds, suggest restricting them:
17351735
//
@@ -1745,37 +1745,21 @@ pub fn suggest_constraining_type_param(
17451745
// |
17461746
// replace with: `T: Bar +`
17471747

1748-
err.span_help(
1749-
bounds_span,
1750-
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
1748+
err.span_suggestion_verbose(
1749+
bounds_span.shrink_to_hi(),
1750+
MSG_RESTRICT_BOUND_FURTHER,
1751+
format!(" + {}", constraint),
1752+
Applicability::MachineApplicable,
17511753
);
1752-
1753-
let span_hi = param.span.with_hi(span.hi());
1754-
let span_with_colon = source_map.span_through_char(span_hi, ':');
1755-
1756-
if span_hi != param.span && span_with_colon != span_hi {
1757-
err.tool_only_span_suggestion(
1758-
span_with_colon,
1759-
MSG_RESTRICT_BOUND_FURTHER,
1760-
format!("{}: {} + ", param_name, constraint),
1761-
Applicability::MachineApplicable,
1762-
);
1763-
}
17641754
} else {
17651755
// If user hasn't provided any bounds, suggest adding a new one:
17661756
//
17671757
// fn foo<T>(t: T) { ... }
17681758
// - help: consider restricting this type parameter with `T: Foo`
1769-
1770-
err.span_help(
1771-
param.span,
1772-
&format!("{} `{}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
1773-
);
1774-
1775-
err.tool_only_span_suggestion(
1776-
param.span,
1777-
MSG_RESTRICT_TYPE,
1778-
format!("{}: {}", param_name, constraint),
1759+
err.span_suggestion_verbose(
1760+
param.span.shrink_to_hi(),
1761+
&msg_restrict_type,
1762+
format!(": {}", constraint),
17791763
Applicability::MachineApplicable,
17801764
);
17811765
}
@@ -1839,55 +1823,44 @@ pub fn suggest_constraining_type_param(
18391823
}
18401824
}
18411825

1842-
let where_clause_span =
1843-
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
1826+
let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place();
1827+
// Account for `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
1828+
let mut trailing_comma = false;
1829+
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) {
1830+
if snippet.ends_with(",") {
1831+
trailing_comma = true;
1832+
}
1833+
}
1834+
let where_clause_span = if trailing_comma {
1835+
let hi = where_clause_span.hi();
1836+
Span::new(hi - BytePos(1), hi, where_clause_span.ctxt())
1837+
} else {
1838+
where_clause_span.shrink_to_hi()
1839+
};
18441840

18451841
match &param_spans[..] {
18461842
&[] => {
1847-
err.span_help(
1848-
param.span,
1849-
&format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
1850-
);
1851-
1852-
err.tool_only_span_suggestion(
1843+
err.span_suggestion_verbose(
18531844
where_clause_span,
1854-
MSG_RESTRICT_TYPE,
1845+
&msg_restrict_type_further,
18551846
format!(", {}: {}", param_name, constraint),
18561847
Applicability::MachineApplicable,
18571848
);
18581849
}
18591850

18601851
&[&param_span] => {
1861-
err.span_help(
1862-
param_span,
1863-
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
1852+
err.span_suggestion_verbose(
1853+
param_span.shrink_to_hi(),
1854+
MSG_RESTRICT_BOUND_FURTHER,
1855+
format!(" + {}", constraint),
1856+
Applicability::MachineApplicable,
18641857
);
1865-
1866-
let span_hi = param_span.with_hi(span.hi());
1867-
let span_with_colon = source_map.span_through_char(span_hi, ':');
1868-
1869-
if span_hi != param_span && span_with_colon != span_hi {
1870-
err.tool_only_span_suggestion(
1871-
span_with_colon,
1872-
MSG_RESTRICT_BOUND_FURTHER,
1873-
format!("{}: {} +", param_name, constraint),
1874-
Applicability::MachineApplicable,
1875-
);
1876-
}
18771858
}
18781859

18791860
_ => {
1880-
err.span_help(
1881-
param.span,
1882-
&format!(
1883-
"{} `where {}: {}`",
1884-
MSG_RESTRICT_TYPE_FURTHER, param_name, constraint,
1885-
),
1886-
);
1887-
1888-
err.tool_only_span_suggestion(
1861+
err.span_suggestion_verbose(
18891862
where_clause_span,
1890-
MSG_RESTRICT_BOUND_FURTHER,
1863+
&msg_restrict_type_further,
18911864
format!(", {}: {}", param_name, constraint),
18921865
Applicability::MachineApplicable,
18931866
);

‎src/librustc_trait_selection/traits/error_reporting/suggestions.rs‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -218,51 +218,29 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
218218
return;
219219
}
220220

221-
hir::Node::Item(hir::Item {
222-
kind: hir::ItemKind::Struct(_, generics),
223-
span,
224-
..
225-
})
226-
| hir::Node::Item(hir::Item {
227-
kind: hir::ItemKind::Enum(_, generics), span, ..
228-
})
229-
| hir::Node::Item(hir::Item {
230-
kind: hir::ItemKind::Union(_, generics),
231-
span,
232-
..
233-
})
221+
hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(_, generics), .. })
222+
| hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(_, generics), .. })
223+
| hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(_, generics), .. })
234224
| hir::Node::Item(hir::Item {
235225
kind: hir::ItemKind::Trait(_, _, generics, ..),
236-
span,
237-
..
238-
})
239-
| hir::Node::Item(hir::Item {
240-
kind: hir::ItemKind::Impl { generics, .. },
241-
span,
242226
..
243227
})
244228
| hir::Node::Item(hir::Item {
245-
kind: hir::ItemKind::Fn(_, generics, _),
246-
span,
247-
..
229+
kind: hir::ItemKind::Impl { generics, .. }, ..
248230
})
231+
| hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. })
249232
| hir::Node::Item(hir::Item {
250-
kind: hir::ItemKind::TyAlias(_, generics),
251-
span,
252-
..
233+
kind: hir::ItemKind::TyAlias(_, generics), ..
253234
})
254235
| hir::Node::Item(hir::Item {
255-
kind: hir::ItemKind::TraitAlias(generics, _),
256-
span,
257-
..
236+
kind: hir::ItemKind::TraitAlias(generics, _), ..
258237
})
259238
| hir::Node::Item(hir::Item {
260239
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }),
261-
span,
262240
..
263241
})
264-
| hir::Node::TraitItem(hir::TraitItem { generics, span, .. })
265-
| hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
242+
| hir::Node::TraitItem(hir::TraitItem { generics, .. })
243+
| hir::Node::ImplItem(hir::ImplItem { generics, .. })
266244
if param_ty =>
267245
{
268246
// Missing generic type parameter bound.
@@ -274,8 +252,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
274252
&mut err,
275253
&param_name,
276254
&constraint,
277-
self.tcx.sess.source_map(),
278-
*span,
279255
Some(trait_ref.def_id()),
280256
) {
281257
return;

‎src/test/ui/associated-const/associated-const-type-parameter-arrays-2.stderr‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ LL | const Y: usize;
77
LL | let _array = [4; <A as Foo>::Y];
88
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
99
|
10-
help: consider further restricting this bound with `+ Foo`
11-
--> $DIR/associated-const-type-parameter-arrays-2.rs:15:16
10+
help: consider further restricting this bound
1211
|
13-
LL | pub fn test<A: Foo, B: Foo>() {
14-
| ^^^
12+
LL | pub fn test<A: Foo + Foo, B: Foo>() {
13+
| ^^^^^
1514

1615
error: aborting due to previous error
1716

‎src/test/ui/associated-const/associated-const-type-parameter-arrays.stderr‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ LL | const Y: usize;
77
LL | let _array: [u32; <A as Foo>::Y];
88
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
99
|
10-
help: consider further restricting this bound with `+ Foo`
11-
--> $DIR/associated-const-type-parameter-arrays.rs:15:16
10+
help: consider further restricting this bound
1211
|
13-
LL | pub fn test<A: Foo, B: Foo>() {
14-
| ^^^
12+
LL | pub fn test<A: Foo + Foo, B: Foo>() {
13+
| ^^^^^
1514

1615
error: aborting due to previous error
1716

‎src/test/ui/associated-types/associated-types-invalid-trait-ref-issue-18865.stderr‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Foo<usize>` is not satisfied
44
LL | let u: <T as Foo<usize>>::Bar = t.get_bar();
55
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo<usize>` is not implemented for `T`
66
|
7-
help: consider further restricting this bound with `+ Foo<usize>`
8-
--> $DIR/associated-types-invalid-trait-ref-issue-18865.rs:9:8
7+
help: consider further restricting this bound
98
|
10-
LL | fn f<T:Foo<isize>>(t: &T) {
11-
| ^^^^^^^^^^
9+
LL | fn f<T:Foo<isize> + Foo<usize>>(t: &T) {
10+
| ^^^^^^^^^^^^
1211

1312
error: aborting due to previous error
1413

‎src/test/ui/associated-types/associated-types-no-suitable-bound.stderr‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Get` is not satisfied
44
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
66
|
7-
help: consider restricting this type parameter with `T: Get`
8-
--> $DIR/associated-types-no-suitable-bound.rs:11:13
7+
help: consider restricting type parameter `T`
98
|
10-
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
11-
| ^
9+
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}
10+
| ^^^^^
1211

1312
error: aborting due to previous error
1413

‎src/test/ui/associated-types/defaults-suitability.stderr‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ LL | trait Foo<T> {
2323
LL | type Bar: Clone = Vec<T>;
2424
| ^^^^^ the trait `std::clone::Clone` is not implemented for `T`
2525
|
26-
help: consider restricting this type parameter with `T: std::clone::Clone`
27-
--> $DIR/defaults-suitability.rs:32:11
28-
|
29-
LL | trait Foo<T> {
30-
| ^
3126
= note: required because of the requirements on the impl of `std::clone::Clone` for `std::vec::Vec<T>`
27+
help: consider restricting type parameter `T`
28+
|
29+
LL | trait Foo<T: std::clone::Clone> {
30+
| ^^^^^^^^^^^^^^^^^^^
3231

3332
error[E0277]: the trait bound `(): Foo<Self>` is not satisfied
3433
--> $DIR/defaults-suitability.rs:39:17
@@ -119,11 +118,10 @@ LL | | type Baz = T;
119118
LL | | }
120119
| |_- required by `Foo3`
121120
|
122-
help: consider restricting this type parameter with `where T: std::clone::Clone`
123-
--> $DIR/defaults-suitability.rs:88:12
121+
help: consider further restricting type parameter `T`
124122
|
125-
LL | trait Foo3<T> where
126-
| ^
123+
LL | Self::Baz: Clone, T: std::clone::Clone
124+
| ^^^^^^^^^^^^^^^^^^^^^^
127125

128126
error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
129127
--> $DIR/defaults-suitability.rs:27:5

0 commit comments

Comments
 (0)