From 79ee8f329d1d3105555fd40be5b0eac56c9a8e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 25 Nov 2018 12:41:38 -0800 Subject: [PATCH 1/4] Suggest appropriate place for lifetime when declared after type arguments --- src/libsyntax/parse/parser.rs | 43 +++++++++++++++---- src/test/ui/parser/issue-14303-enum.stderr | 4 ++ src/test/ui/parser/issue-14303-fn-def.stderr | 4 ++ src/test/ui/parser/issue-14303-impl.stderr | 4 ++ src/test/ui/parser/issue-14303-struct.stderr | 4 ++ src/test/ui/parser/issue-14303-trait.stderr | 4 ++ .../ui/suggestions/suggest-move-lifetimes.rs | 15 +++++++ .../suggestions/suggest-move-lifetimes.stderr | 32 ++++++++++++++ 8 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-move-lifetimes.rs create mode 100644 src/test/ui/suggestions/suggest-move-lifetimes.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e2f09affd4fea..07e13ffc3143b 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5178,8 +5178,10 @@ impl<'a> Parser<'a> { /// Parses (possibly empty) list of lifetime and type parameters, possibly including /// trailing comma and erroneous trailing attributes. crate fn parse_generic_params(&mut self) -> PResult<'a, Vec> { + let mut lifetimes = Vec::new(); let mut params = Vec::new(); - let mut seen_ty_param = false; + let mut seen_ty_param: Option = None; + let mut last_comma_span = None; loop { let attrs = self.parse_outer_attributes()?; if self.check_lifetime() { @@ -5190,25 +5192,48 @@ impl<'a> Parser<'a> { } else { Vec::new() }; - params.push(ast::GenericParam { + lifetimes.push(ast::GenericParam { ident: lifetime.ident, id: lifetime.id, attrs: attrs.into(), bounds, kind: ast::GenericParamKind::Lifetime, }); - if seen_ty_param { - self.span_err(self.prev_span, - "lifetime parameters must be declared prior to type parameters"); + if let Some(sp) = seen_ty_param { + let param_span = self.prev_span; + let ate_comma = self.eat(&token::Comma); + let remove_sp = if ate_comma { + param_span.until(self.span) + } else { + last_comma_span.unwrap_or(param_span).to(param_span) + }; + let mut err = self.struct_span_err( + self.prev_span, + "lifetime parameters must be declared prior to type parameters", + ); + if let Ok(snippet) = self.sess.source_map().span_to_snippet(param_span) { + err.multipart_suggestion( + "move the lifetime parameter prior to the first type parameter", + vec![ + (remove_sp, String::new()), + (sp.shrink_to_lo(), format!("{}, ", snippet)), + ], + ); + } + err.emit(); + if ate_comma { + last_comma_span = Some(self.prev_span); + continue + } } } else if self.check_ident() { // Parse type parameter. params.push(self.parse_ty_param(attrs)?); - seen_ty_param = true; + seen_ty_param = Some(self.prev_span); } else { // Check for trailing attributes and stop parsing. if !attrs.is_empty() { - let param_kind = if seen_ty_param { "type" } else { "lifetime" }; + let param_kind = if seen_ty_param.is_some() { "type" } else { "lifetime" }; self.span_err(attrs[0].span, &format!("trailing attribute after {} parameters", param_kind)); } @@ -5218,8 +5243,10 @@ impl<'a> Parser<'a> { if !self.eat(&token::Comma) { break } + last_comma_span = Some(self.prev_span); } - Ok(params) + lifetimes.extend(params); // ensure the correct order of lifetimes and type params + Ok(lifetimes) } /// Parse a set of optional generic type parameter declarations. Where diff --git a/src/test/ui/parser/issue-14303-enum.stderr b/src/test/ui/parser/issue-14303-enum.stderr index 7d546cb2180e3..622066a94f80a 100644 --- a/src/test/ui/parser/issue-14303-enum.stderr +++ b/src/test/ui/parser/issue-14303-enum.stderr @@ -3,6 +3,10 @@ error: lifetime parameters must be declared prior to type parameters | LL | enum X<'a, T, 'b> { | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | enum X<'a, 'b, T> { + | ^^^ -- error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-fn-def.stderr b/src/test/ui/parser/issue-14303-fn-def.stderr index c7b57f36376b5..630c9cb40de37 100644 --- a/src/test/ui/parser/issue-14303-fn-def.stderr +++ b/src/test/ui/parser/issue-14303-fn-def.stderr @@ -3,6 +3,10 @@ error: lifetime parameters must be declared prior to type parameters | LL | fn foo<'a, T, 'b>(x: &'a T) {} | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | fn foo<'a, 'b, T>(x: &'a T) {} + | ^^^ -- error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-impl.stderr b/src/test/ui/parser/issue-14303-impl.stderr index 0b7016eb7f71d..2e3181de90275 100644 --- a/src/test/ui/parser/issue-14303-impl.stderr +++ b/src/test/ui/parser/issue-14303-impl.stderr @@ -3,6 +3,10 @@ error: lifetime parameters must be declared prior to type parameters | LL | impl<'a, T, 'b> X {} | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | impl<'a, 'b, T> X {} + | ^^^ -- error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-struct.stderr b/src/test/ui/parser/issue-14303-struct.stderr index 4a4b678919495..c6b33120c18f0 100644 --- a/src/test/ui/parser/issue-14303-struct.stderr +++ b/src/test/ui/parser/issue-14303-struct.stderr @@ -3,6 +3,10 @@ error: lifetime parameters must be declared prior to type parameters | LL | struct X<'a, T, 'b> { | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | struct X<'a, 'b, T> { + | ^^^ -- error: aborting due to previous error diff --git a/src/test/ui/parser/issue-14303-trait.stderr b/src/test/ui/parser/issue-14303-trait.stderr index ab5cc5655bbe1..6d00f284bbbe7 100644 --- a/src/test/ui/parser/issue-14303-trait.stderr +++ b/src/test/ui/parser/issue-14303-trait.stderr @@ -3,6 +3,10 @@ error: lifetime parameters must be declared prior to type parameters | LL | trait Foo<'a, T, 'b> {} | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | trait Foo<'a, 'b, T> {} + | ^^^ -- error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.rs b/src/test/ui/suggestions/suggest-move-lifetimes.rs new file mode 100644 index 0000000000000..be6d29d933763 --- /dev/null +++ b/src/test/ui/suggestions/suggest-move-lifetimes.rs @@ -0,0 +1,15 @@ +struct A { + t: &'a T, +} + +struct B { + t: &'a T, + u: U, +} + +struct C { + t: &'a T, + u: U, +} + +fn main() {} diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.stderr b/src/test/ui/suggestions/suggest-move-lifetimes.stderr new file mode 100644 index 0000000000000..57e6780b35984 --- /dev/null +++ b/src/test/ui/suggestions/suggest-move-lifetimes.stderr @@ -0,0 +1,32 @@ +error: lifetime parameters must be declared prior to type parameters + --> $DIR/suggest-move-lifetimes.rs:1:13 + | +LL | struct A { + | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | struct A<'a, T> { + | ^^^ -- + +error: lifetime parameters must be declared prior to type parameters + --> $DIR/suggest-move-lifetimes.rs:5:15 + | +LL | struct B { + | ^ +help: move the lifetime parameter prior to the first type parameter + | +LL | struct B<'a, T, U> { + | ^^^ -- + +error: lifetime parameters must be declared prior to type parameters + --> $DIR/suggest-move-lifetimes.rs:10:16 + | +LL | struct C { + | ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | struct C { + | ^^^ -- + +error: aborting due to 3 previous errors + From 234d043d18175aff37200a91df2a1b7c3064fc80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 25 Nov 2018 12:46:42 -0800 Subject: [PATCH 2/4] Move lifetimes before the *first* type argument --- src/libsyntax/parse/parser.rs | 4 +++- src/test/ui/suggestions/suggest-move-lifetimes.stderr | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 07e13ffc3143b..1fdf86d48670b 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5229,7 +5229,9 @@ impl<'a> Parser<'a> { } else if self.check_ident() { // Parse type parameter. params.push(self.parse_ty_param(attrs)?); - seen_ty_param = Some(self.prev_span); + if seen_ty_param.is_none() { + seen_ty_param = Some(self.prev_span); + } } else { // Check for trailing attributes and stop parsing. if !attrs.is_empty() { diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.stderr b/src/test/ui/suggestions/suggest-move-lifetimes.stderr index 57e6780b35984..fa1cfe66ab530 100644 --- a/src/test/ui/suggestions/suggest-move-lifetimes.stderr +++ b/src/test/ui/suggestions/suggest-move-lifetimes.stderr @@ -25,8 +25,8 @@ LL | struct C { | ^^ help: move the lifetime parameter prior to the first type parameter | -LL | struct C { - | ^^^ -- +LL | struct C<'a, T, U> { + | ^^^ -- error: aborting due to 3 previous errors From 45dfe43887fee6c2ce4cbc3e46b31626330be094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Nov 2018 08:32:47 -0800 Subject: [PATCH 3/4] Emit one diagnostic for multiple misplaced lifetimes --- src/libsyntax/parse/parser.rs | 31 ++++++++++++------- .../ui/suggestions/suggest-move-lifetimes.rs | 6 ++++ .../suggestions/suggest-move-lifetimes.stderr | 16 ++++++++-- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1fdf86d48670b..ab5afaf3d99fe 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5182,6 +5182,8 @@ impl<'a> Parser<'a> { let mut params = Vec::new(); let mut seen_ty_param: Option = None; let mut last_comma_span = None; + let mut bad_lifetime_pos = vec![]; + let mut suggestions = vec![]; loop { let attrs = self.parse_outer_attributes()?; if self.check_lifetime() { @@ -5207,20 +5209,12 @@ impl<'a> Parser<'a> { } else { last_comma_span.unwrap_or(param_span).to(param_span) }; - let mut err = self.struct_span_err( - self.prev_span, - "lifetime parameters must be declared prior to type parameters", - ); + bad_lifetime_pos.push(param_span); + if let Ok(snippet) = self.sess.source_map().span_to_snippet(param_span) { - err.multipart_suggestion( - "move the lifetime parameter prior to the first type parameter", - vec![ - (remove_sp, String::new()), - (sp.shrink_to_lo(), format!("{}, ", snippet)), - ], - ); + suggestions.push((remove_sp, String::new())); + suggestions.push((sp.shrink_to_lo(), format!("{}, ", snippet))); } - err.emit(); if ate_comma { last_comma_span = Some(self.prev_span); continue @@ -5247,6 +5241,19 @@ impl<'a> Parser<'a> { } last_comma_span = Some(self.prev_span); } + if !bad_lifetime_pos.is_empty() { + let mut err = self.struct_span_err( + bad_lifetime_pos, + "lifetime parameters must be declared prior to type parameters", + ); + if !suggestions.is_empty() { + err.multipart_suggestion( + "move the lifetime parameter prior to the first type parameter", + suggestions, + ); + } + err.emit(); + } lifetimes.extend(params); // ensure the correct order of lifetimes and type params Ok(lifetimes) } diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.rs b/src/test/ui/suggestions/suggest-move-lifetimes.rs index be6d29d933763..5051a406078aa 100644 --- a/src/test/ui/suggestions/suggest-move-lifetimes.rs +++ b/src/test/ui/suggestions/suggest-move-lifetimes.rs @@ -12,4 +12,10 @@ struct C { u: U, } +struct D { + t: &'a T, + u: &'b U, + v: &'c V, +} + fn main() {} diff --git a/src/test/ui/suggestions/suggest-move-lifetimes.stderr b/src/test/ui/suggestions/suggest-move-lifetimes.stderr index fa1cfe66ab530..f3d6469b51255 100644 --- a/src/test/ui/suggestions/suggest-move-lifetimes.stderr +++ b/src/test/ui/suggestions/suggest-move-lifetimes.stderr @@ -9,10 +9,10 @@ LL | struct A<'a, T> { | ^^^ -- error: lifetime parameters must be declared prior to type parameters - --> $DIR/suggest-move-lifetimes.rs:5:15 + --> $DIR/suggest-move-lifetimes.rs:5:13 | LL | struct B { - | ^ + | ^^ help: move the lifetime parameter prior to the first type parameter | LL | struct B<'a, T, U> { @@ -28,5 +28,15 @@ help: move the lifetime parameter prior to the first type parameter LL | struct C<'a, T, U> { | ^^^ -- -error: aborting due to 3 previous errors +error: lifetime parameters must be declared prior to type parameters + --> $DIR/suggest-move-lifetimes.rs:15:16 + | +LL | struct D { + | ^^ ^^ ^^ +help: move the lifetime parameter prior to the first type parameter + | +LL | struct D<'a, 'b, 'c, T, U, V> { + | ^^^ ^^^ ^^^ --- + +error: aborting due to 4 previous errors From 6f028fe8e0db8c1251f24d694fc001aa933cf0ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 26 Nov 2018 13:58:46 -0800 Subject: [PATCH 4/4] Specify suggestion applicability --- src/libsyntax/parse/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ab5afaf3d99fe..c224f18259294 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5247,9 +5247,10 @@ impl<'a> Parser<'a> { "lifetime parameters must be declared prior to type parameters", ); if !suggestions.is_empty() { - err.multipart_suggestion( + err.multipart_suggestion_with_applicability( "move the lifetime parameter prior to the first type parameter", suggestions, + Applicability::MachineApplicable, ); } err.emit();