Skip to content

Commit 6ed06b2

Browse files
estebankestebank
committed
Reduce verbosity of some type ascription errors
* Deduplicate type ascription LHS errors * Remove duplicated `:` -> `::` suggestion from parse error * Tweak wording to be more accurate * Modify `current_type_ascription` to reduce span wrangling * remove now unnecessary match arm * Add run-rustfix to appropriate tests
1 parent 6b09c37 commit 6ed06b2

28 files changed

+212
-181
lines changed

‎src/librustc_parse/parser/diagnostics.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ impl<'a> Parser<'a> {
333333
Applicability::MachineApplicable
334334
},
335335
);
336+
self.sess.type_ascription_path_suggestions.borrow_mut().insert(sp);
336337
} else if op_pos.line != next_pos.line && maybe_expected_semicolon {
337338
err.span_suggestion(
338339
sp,

‎src/librustc_resolve/late.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2241,8 +2241,15 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
22412241
self.resolve_expr(argument, None);
22422242
}
22432243
}
2244-
ExprKind::Type(ref type_expr, _) => {
2245-
self.diagnostic_metadata.current_type_ascription.push(type_expr.span);
2244+
ExprKind::Type(ref type_expr, ref ty) => {
2245+
// `ParseSess::type_ascription_path_suggestions` keeps spans of colon tokens in
2246+
// type ascription. Here we are trying to retrieve the span of the colon token as
2247+
// well, but only if it's written without spaces `expr:Ty` and therefore confusable
2248+
// with `expr::Ty`, only in this case it will match the span from
2249+
// `type_ascription_path_suggestions`.
2250+
self.diagnostic_metadata
2251+
.current_type_ascription
2252+
.push(type_expr.span.between(ty.span));
22462253
visit::walk_expr(self, expr);
22472254
self.diagnostic_metadata.current_type_ascription.pop();
22482255
}

‎src/librustc_resolve/late/diagnostics.rs‎

Lines changed: 117 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_hir::PrimTy;
1717
use rustc_session::config::nightly_options;
1818
use rustc_span::hygiene::MacroKind;
1919
use rustc_span::symbol::{kw, sym, Ident};
20-
use rustc_span::Span;
20+
use rustc_span::{BytePos, Span};
2121

2222
use log::debug;
2323

@@ -223,13 +223,31 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
223223
if candidates.is_empty() && is_expected(Res::Def(DefKind::Enum, crate_def_id)) {
224224
let enum_candidates =
225225
self.r.lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant);
226-
let mut enum_candidates = enum_candidates
227-
.iter()
228-
.map(|suggestion| import_candidate_to_enum_paths(&suggestion))
229-
.collect::<Vec<_>>();
230-
enum_candidates.sort();
231226

232227
if !enum_candidates.is_empty() {
228+
if let (PathSource::Type, Some(span)) =
229+
(source, self.diagnostic_metadata.current_type_ascription.last())
230+
{
231+
if self
232+
.r
233+
.session
234+
.parse_sess
235+
.type_ascription_path_suggestions
236+
.borrow()
237+
.contains(span)
238+
{
239+
// Already reported this issue on the lhs of the type ascription.
240+
err.delay_as_bug();
241+
return (err, candidates);
242+
}
243+
}
244+
245+
let mut enum_candidates = enum_candidates
246+
.iter()
247+
.map(|suggestion| import_candidate_to_enum_paths(&suggestion))
248+
.collect::<Vec<_>>();
249+
enum_candidates.sort();
250+
233251
// Contextualize for E0412 "cannot find type", but don't belabor the point
234252
// (that it's a variant) for E0573 "expected type, found variant".
235253
let preamble = if res.is_none() {
@@ -484,10 +502,21 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
484502
match source {
485503
PathSource::Expr(Some(
486504
parent @ Expr { kind: ExprKind::Field(..) | ExprKind::MethodCall(..), .. },
487-
)) => {
488-
path_sep(err, &parent);
489-
}
490-
PathSource::Expr(None) if followed_by_brace => {
505+
)) if path_sep(err, &parent) => {}
506+
PathSource::Expr(
507+
None
508+
| Some(Expr {
509+
kind:
510+
ExprKind::Path(..)
511+
| ExprKind::Binary(..)
512+
| ExprKind::Unary(..)
513+
| ExprKind::If(..)
514+
| ExprKind::While(..)
515+
| ExprKind::ForLoop(..)
516+
| ExprKind::Match(..),
517+
..
518+
}),
519+
) if followed_by_brace => {
491520
if let Some(sp) = closing_brace {
492521
err.multipart_suggestion(
493522
"surround the struct literal with parentheses",
@@ -508,11 +537,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
508537
);
509538
}
510539
}
511-
PathSource::Expr(
512-
None | Some(Expr { kind: ExprKind::Call(..) | ExprKind::Path(..), .. }),
513-
)
514-
| PathSource::TupleStruct(_)
515-
| PathSource::Pat => {
540+
PathSource::Expr(_) | PathSource::TupleStruct(_) | PathSource::Pat => {
516541
let span = match &source {
517542
PathSource::Expr(Some(Expr {
518543
span, kind: ExprKind::Call(_, _), ..
@@ -593,6 +618,24 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
593618
Res::Def(DefKind::Enum, def_id),
594619
PathSource::TupleStruct(_) | PathSource::Expr(..),
595620
) => {
621+
if self
622+
.diagnostic_metadata
623+
.current_type_ascription
624+
.last()
625+
.map(|sp| {
626+
self.r
627+
.session
628+
.parse_sess
629+
.type_ascription_path_suggestions
630+
.borrow()
631+
.contains(&sp)
632+
})
633+
.unwrap_or(false)
634+
{
635+
err.delay_as_bug();
636+
// We already suggested changing `:` into `::` during parsing.
637+
return false;
638+
}
596639
if let Some(variants) = self.collect_enum_variants(def_id) {
597640
if !variants.is_empty() {
598641
let msg = if variants.len() == 1 {
@@ -609,7 +652,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
609652
);
610653
}
611654
} else {
612-
err.note("did you mean to use one of the enum's variants?");
655+
err.note("you might have meant to use one of the enum's variants");
613656
}
614657
}
615658
(Res::Def(DefKind::Struct, def_id), _) if ns == ValueNS => {
@@ -829,77 +872,73 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
829872
fn type_ascription_suggestion(&self, err: &mut DiagnosticBuilder<'_>, base_span: Span) {
830873
let sm = self.r.session.source_map();
831874
let base_snippet = sm.span_to_snippet(base_span);
832-
if let Some(sp) = self.diagnostic_metadata.current_type_ascription.last() {
833-
let mut sp = *sp;
834-
loop {
835-
// Try to find the `:`; bail on first non-':' / non-whitespace.
836-
sp = sm.next_point(sp);
837-
if let Ok(snippet) = sm.span_to_snippet(sp.to(sm.next_point(sp))) {
838-
let line_sp = sm.lookup_char_pos(sp.hi()).line;
839-
let line_base_sp = sm.lookup_char_pos(base_span.lo()).line;
840-
if snippet == ":" {
841-
let mut show_label = true;
842-
if line_sp != line_base_sp {
843-
err.span_suggestion_short(
844-
sp,
845-
"did you mean to use `;` here instead?",
846-
";".to_string(),
875+
if let Some(&sp) = self.diagnostic_metadata.current_type_ascription.last() {
876+
if let Ok(snippet) = sm.span_to_snippet(sp) {
877+
let len = snippet.trim_end().len() as u32;
878+
if snippet.trim() == ":" {
879+
let colon_sp =
880+
sp.with_lo(sp.lo() + BytePos(len - 1)).with_hi(sp.lo() + BytePos(len));
881+
let mut show_label = true;
882+
if sm.is_multiline(sp) {
883+
err.span_suggestion_short(
884+
colon_sp,
885+
"maybe you meant to write `;` here",
886+
";".to_string(),
887+
Applicability::MaybeIncorrect,
888+
);
889+
} else {
890+
let after_colon_sp =
891+
self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
892+
if snippet.len() == 1 {
893+
// `foo:bar`
894+
err.span_suggestion(
895+
colon_sp,
896+
"maybe you meant to write a path separator here",
897+
"::".to_string(),
847898
Applicability::MaybeIncorrect,
848899
);
849-
} else {
850-
let colon_sp = self.get_colon_suggestion_span(sp);
851-
let after_colon_sp =
852-
self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
853-
if !sm
854-
.span_to_snippet(after_colon_sp)
855-
.map(|s| s == " ")
856-
.unwrap_or(false)
900+
show_label = false;
901+
if !self
902+
.r
903+
.session
904+
.parse_sess
905+
.type_ascription_path_suggestions
906+
.borrow_mut()
907+
.insert(colon_sp)
857908
{
858-
err.span_suggestion(
859-
colon_sp,
860-
"maybe you meant to write a path separator here",
861-
"::".to_string(),
862-
Applicability::MaybeIncorrect,
863-
);
864-
show_label = false;
909+
err.delay_as_bug();
865910
}
866-
if let Ok(base_snippet) = base_snippet {
867-
let mut sp = after_colon_sp;
868-
for _ in 0..100 {
869-
// Try to find an assignment
870-
sp = sm.next_point(sp);
871-
let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
872-
match snippet {
873-
Ok(ref x) if x.as_str() == "=" => {
874-
err.span_suggestion(
875-
base_span,
876-
"maybe you meant to write an assignment here",
877-
format!("let {}", base_snippet),
878-
Applicability::MaybeIncorrect,
879-
);
880-
show_label = false;
881-
break;
882-
}
883-
Ok(ref x) if x.as_str() == "\n" => break,
884-
Err(_) => break,
885-
Ok(_) => {}
911+
}
912+
if let Ok(base_snippet) = base_snippet {
913+
let mut sp = after_colon_sp;
914+
for _ in 0..100 {
915+
// Try to find an assignment
916+
sp = sm.next_point(sp);
917+
let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
918+
match snippet {
919+
Ok(ref x) if x.as_str() == "=" => {
920+
err.span_suggestion(
921+
base_span,
922+
"maybe you meant to write an assignment here",
923+
format!("let {}", base_snippet),
924+
Applicability::MaybeIncorrect,
925+
);
926+
show_label = false;
927+
break;
886928
}
929+
Ok(ref x) if x.as_str() == "\n" => break,
930+
Err(_) => break,
931+
Ok(_) => {}
887932
}
888933
}
889934
}
890-
if show_label {
891-
err.span_label(
892-
base_span,
893-
"expecting a type here because of type ascription",
894-
);
895-
}
896-
break;
897-
} else if !snippet.trim().is_empty() {
898-
debug!("tried to find type ascription `:` token, couldn't find it");
899-
break;
900935
}
901-
} else {
902-
break;
936+
if show_label {
937+
err.span_label(
938+
base_span,
939+
"expecting a type here because of type ascription",
940+
);
941+
}
903942
}
904943
}
905944
}

‎src/librustc_session/parse.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,8 @@ pub struct ParseSess {
138138
pub reached_eof: Lock<bool>,
139139
/// Environment variables accessed during the build and their values when they exist.
140140
pub env_depinfo: Lock<FxHashSet<(Symbol, Option<Symbol>)>>,
141+
/// All the type ascriptions expressions that have had a suggestion for likely path typo.
142+
pub type_ascription_path_suggestions: Lock<FxHashSet<Span>>,
141143
}
142144

143145
impl ParseSess {
@@ -164,6 +166,7 @@ impl ParseSess {
164166
symbol_gallery: SymbolGallery::default(),
165167
reached_eof: Lock::new(false),
166168
env_depinfo: Default::default(),
169+
type_ascription_path_suggestions: Default::default(),
167170
}
168171
}
169172

‎src/test/ui/suggestions/issue-61226.fixed‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// run-rustfix
2+
struct X {}
3+
fn main() {
4+
let _ = vec![X {}]; //…
5+
//~^ ERROR expected value, found struct `X`
6+
}

‎src/test/ui/suggestions/issue-61226.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
// run-rustfix
12
struct X {}
23
fn main() {
3-
vec![X]; //…
4+
let _ = vec![X]; //…
45
//~^ ERROR expected value, found struct `X`
56
}

‎src/test/ui/suggestions/issue-61226.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
error[E0423]: expected value, found struct `X`
2-
--> $DIR/issue-61226.rs:3:10
2+
--> $DIR/issue-61226.rs:4:18
33
|
44
LL | struct X {}
55
| ----------- `X` defined here
66
LL | fn main() {
7-
LL | vec![X]; //…
8-
| ^ help: use struct literal syntax instead: `X {}`
7+
LL | let _ = vec![X]; //…
8+
| ^ help: use struct literal syntax instead: `X {}`
99

1010
error: aborting due to previous error
1111

‎src/test/ui/suggestions/type-ascription-instead-of-method.fixed‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// run-rustfix
2+
fn main() {
3+
let _ = Box::new("foo".to_string());
4+
//~^ ERROR expected type, found
5+
}

‎src/test/ui/suggestions/type-ascription-instead-of-method.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1+
// run-rustfix
12
fn main() {
2-
Box:new("foo".to_string())
3+
let _ = Box:new("foo".to_string());
34
//~^ ERROR expected type, found
45
}

‎src/test/ui/suggestions/type-ascription-instead-of-method.stderr‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
error: expected type, found `"foo"`
2-
--> $DIR/type-ascription-instead-of-method.rs:2:13
2+
--> $DIR/type-ascription-instead-of-method.rs:3:21
33
|
4-
LL | Box:new("foo".to_string())
5-
| - ^^^^^ expected type
6-
| |
7-
| help: maybe write a path separator here: `::`
4+
LL | let _ = Box:new("foo".to_string());
5+
| - ^^^^^ expected type
6+
| |
7+
| help: maybe write a path separator here: `::`
88
|
99
= note: `#![feature(type_ascription)]` lets you annotate an expression with a type: `<expr>: <type>`
1010

0 commit comments

Comments
 (0)