Skip to content

Commit e9b96f9

Browse files
estebankestebank
committed
Modify current_type_ascription to reduce span wrangling
1 parent 06cab8e commit e9b96f9

8 files changed

+111
-94
lines changed

‎src/librustc_resolve/late.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,6 @@ struct DiagnosticMetadata<'ast> {
368368
/// The current enclosing function (used for better errors).
369369
current_function: Option<(FnKind<'ast>, Span)>,
370370

371-
/// Keeps track of the parent `Expr` when visiting an expression in `visit_expr`.
372-
current_expr: Option<&'ast Expr>,
373-
374371
/// A list of labels as of yet unused. Labels will be removed from this map when
375372
/// they are used (in a `break` or `continue` statement)
376373
unused_labels: FxHashMap<NodeId, Span>,
@@ -422,10 +419,7 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> {
422419
});
423420
}
424421
fn visit_expr(&mut self, expr: &'ast Expr) {
425-
let prev = self.diagnostic_metadata.current_expr;
426-
self.diagnostic_metadata.current_expr = Some(expr);
427-
self.resolve_expr(expr, prev);
428-
self.diagnostic_metadata.current_expr = prev;
422+
self.resolve_expr(expr, None);
429423
}
430424
fn visit_local(&mut self, local: &'ast Local) {
431425
let local_spans = match local.pat.kind {
@@ -2169,8 +2163,10 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
21692163
self.resolve_expr(argument, None);
21702164
}
21712165
}
2172-
ExprKind::Type(ref type_expr, _) => {
2173-
self.diagnostic_metadata.current_type_ascription.push(type_expr.span);
2166+
ExprKind::Type(ref type_expr, ref ty) => {
2167+
self.diagnostic_metadata
2168+
.current_type_ascription
2169+
.push(type_expr.span.between(ty.span));
21742170
visit::walk_expr(self, expr);
21752171
self.diagnostic_metadata.current_type_ascription.pop();
21762172
}

‎src/librustc_resolve/late/diagnostics.rs

+86-82
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
use std::ops::Deref;
@@ -226,16 +226,16 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
226226
self.r.lookup_import_candidates(ident, ns, &self.parent_scope, is_enum_variant);
227227

228228
if !enum_candidates.is_empty() {
229-
if let (PathSource::Type, Some(Expr { kind: ExprKind::Type(expr, ty), .. })) =
230-
(source, self.diagnostic_metadata.current_expr)
229+
if let (PathSource::Type, Some(span)) =
230+
(source, self.diagnostic_metadata.current_type_ascription.last())
231231
{
232232
if self
233233
.r
234234
.session
235235
.parse_sess
236236
.type_ascription_path_suggestions
237237
.borrow()
238-
.contains(&expr.span.between(ty.span))
238+
.contains(span)
239239
{
240240
// Already reported this issue on the lhs of the type ascription.
241241
err.delay_as_bug();
@@ -666,6 +666,24 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
666666
Res::Def(DefKind::Enum, def_id),
667667
PathSource::TupleStruct(_) | PathSource::Expr(..),
668668
) => {
669+
if self
670+
.diagnostic_metadata
671+
.current_type_ascription
672+
.last()
673+
.map(|sp| {
674+
self.r
675+
.session
676+
.parse_sess
677+
.type_ascription_path_suggestions
678+
.borrow()
679+
.contains(&sp)
680+
})
681+
.unwrap_or(false)
682+
{
683+
err.delay_as_bug();
684+
// We already suggested changing `:` into `::` during parsing.
685+
return false;
686+
}
669687
if let Some(variants) = self.collect_enum_variants(def_id) {
670688
if !variants.is_empty() {
671689
let msg = if variants.len() == 1 {
@@ -903,93 +921,79 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
903921
fn type_ascription_suggestion(&self, err: &mut DiagnosticBuilder<'_>, base_span: Span) {
904922
let sm = self.r.session.source_map();
905923
let base_snippet = sm.span_to_snippet(base_span);
906-
if let Some(sp) = self.diagnostic_metadata.current_type_ascription.last() {
907-
let mut sp = *sp;
908-
loop {
909-
// Try to find the `:`; bail on first non-':' / non-whitespace.
910-
sp = sm.next_point(sp);
911-
if let Ok(snippet) = sm.span_to_snippet(sp.to(sm.next_point(sp))) {
912-
let line_sp = sm.lookup_char_pos(sp.hi()).line;
913-
let line_base_sp = sm.lookup_char_pos(base_span.lo()).line;
914-
if snippet == ":" {
915-
let mut show_label = true;
916-
if line_sp != line_base_sp {
917-
err.span_suggestion_short(
918-
sp,
919-
"did you mean to use `;` here instead?",
920-
";".to_string(),
924+
if let Some(&sp) = self.diagnostic_metadata.current_type_ascription.last() {
925+
if let Ok(snippet) = sm.span_to_snippet(sp) {
926+
let len = snippet.trim_end().len() as u32;
927+
if snippet.trim() == ":" {
928+
let colon_sp =
929+
sp.with_lo(sp.lo() + BytePos(len - 1)).with_hi(sp.lo() + BytePos(len));
930+
let mut show_label = true;
931+
if sm.is_multiline(sp) {
932+
err.span_suggestion_short(
933+
colon_sp,
934+
"maybe you meant to write `;` here",
935+
";".to_string(),
936+
Applicability::MaybeIncorrect,
937+
);
938+
} else {
939+
let after_colon_sp =
940+
self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
941+
if snippet.len() == 1 {
942+
// `foo:bar`
943+
err.span_suggestion(
944+
colon_sp,
945+
"maybe you meant to write a path separator here",
946+
"::".to_string(),
921947
Applicability::MaybeIncorrect,
922948
);
923-
} else {
924-
let colon_sp = self.get_colon_suggestion_span(sp);
925-
let after_colon_sp =
926-
self.get_colon_suggestion_span(colon_sp.shrink_to_hi());
927-
if !sm
928-
.span_to_snippet(after_colon_sp)
929-
.map(|s| s == " ")
930-
.unwrap_or(false)
949+
show_label = false;
950+
if self
951+
.r
952+
.session
953+
.parse_sess
954+
.type_ascription_path_suggestions
955+
.borrow()
956+
.contains(&colon_sp)
931957
{
932-
err.span_suggestion(
933-
colon_sp,
934-
"maybe you meant to write a path separator here",
935-
"::".to_string(),
936-
Applicability::MaybeIncorrect,
937-
);
938-
show_label = false;
939-
if self
940-
.r
941-
.session
942-
.parse_sess
943-
.type_ascription_path_suggestions
944-
.borrow()
945-
.contains(&colon_sp)
946-
{
947-
err.delay_as_bug();
948-
}
949-
self.r
950-
.session
951-
.parse_sess
952-
.type_ascription_path_suggestions
953-
.borrow_mut()
954-
.insert(colon_sp);
958+
err.delay_as_bug();
955959
}
956-
if let Ok(base_snippet) = base_snippet {
957-
let mut sp = after_colon_sp;
958-
for _ in 0..100 {
959-
// Try to find an assignment
960-
sp = sm.next_point(sp);
961-
let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
962-
match snippet {
963-
Ok(ref x) if x.as_str() == "=" => {
964-
err.span_suggestion(
965-
base_span,
966-
"maybe you meant to write an assignment here",
967-
format!("let {}", base_snippet),
968-
Applicability::MaybeIncorrect,
969-
);
970-
show_label = false;
971-
break;
972-
}
973-
Ok(ref x) if x.as_str() == "\n" => break,
974-
Err(_) => break,
975-
Ok(_) => {}
960+
self.r
961+
.session
962+
.parse_sess
963+
.type_ascription_path_suggestions
964+
.borrow_mut()
965+
.insert(colon_sp);
966+
}
967+
if let Ok(base_snippet) = base_snippet {
968+
let mut sp = after_colon_sp;
969+
for _ in 0..100 {
970+
// Try to find an assignment
971+
sp = sm.next_point(sp);
972+
let snippet = sm.span_to_snippet(sp.to(sm.next_point(sp)));
973+
match snippet {
974+
Ok(ref x) if x.as_str() == "=" => {
975+
err.span_suggestion(
976+
base_span,
977+
"maybe you meant to write an assignment here",
978+
format!("let {}", base_snippet),
979+
Applicability::MaybeIncorrect,
980+
);
981+
show_label = false;
982+
break;
976983
}
984+
Ok(ref x) if x.as_str() == "\n" => break,
985+
Err(_) => break,
986+
Ok(_) => {}
977987
}
978988
}
979989
}
980-
if show_label {
981-
err.span_label(
982-
base_span,
983-
"expecting a type here because of type ascription",
984-
);
985-
}
986-
break;
987-
} else if !snippet.trim().is_empty() {
988-
debug!("tried to find type ascription `:` token, couldn't find it");
989-
break;
990990
}
991-
} else {
992-
break;
991+
if show_label {
992+
err.span_label(
993+
base_span,
994+
"expecting a type here because of type ascription",
995+
);
996+
}
993997
}
994998
}
995999
}

‎src/test/ui/type/ascription/issue-47666.fixed

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// run-rustfix
2+
fn main() {
3+
let _ = Option::Some(vec![0, 1]); //~ ERROR expected type, found
4+
}
5+
6+
// This case isn't currently being handled gracefully due to the macro invocation.

‎src/test/ui/type/ascription/issue-47666.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// run-rustfix
12
fn main() {
23
let _ = Option:Some(vec![0, 1]); //~ ERROR expected type, found
34
}

‎src/test/ui/type/ascription/issue-47666.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: expected type, found reserved keyword `box`
2-
--> $DIR/issue-47666.rs:2:25
2+
--> $DIR/issue-47666.rs:3:25
33
|
44
LL | let _ = Option:Some(vec![0, 1]);
55
| - ^^^^^^^^^^

‎src/test/ui/type/type-ascription-with-fn-call.fixed

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// run-rustfix
2+
#![feature(type_ascription)]
3+
4+
fn main() {
5+
f() ;
6+
f(); //~ ERROR expected type, found function
7+
}
8+
9+
fn f() {}

‎src/test/ui/type/type-ascription-with-fn-call.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// run-rustfix
12
#![feature(type_ascription)]
23

34
fn main() {

‎src/test/ui/type/type-ascription-with-fn-call.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error[E0573]: expected type, found function `f`
2-
--> $DIR/type-ascription-with-fn-call.rs:5:5
2+
--> $DIR/type-ascription-with-fn-call.rs:6:5
33
|
44
LL | f() :
5-
| - help: did you mean to use `;` here instead?
5+
| - help: maybe you meant to write `;` here
66
LL | f();
77
| ^^^
88
| |

0 commit comments

Comments
 (0)