From 33d793c326a83ed2e71a4d5e612526cf13c890b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 28 Mar 2020 13:48:04 -0700 Subject: [PATCH 1/6] Point at all constraints before args --- src/librustc_ast_passes/ast_validation.rs | 42 +++++--- src/test/ui/parser/issue-32214.stderr | 2 +- .../ui/suggestions/suggest-move-types.stderr | 96 +++++++++++-------- 3 files changed, 86 insertions(+), 54 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index de7ae10723f4d..680a58d8b7786 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -14,7 +14,7 @@ use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::walk_list; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{error_code, struct_span_err, Applicability}; +use rustc_errors::{error_code, pluralize, struct_span_err, Applicability}; use rustc_parse::validate_attr; use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; use rustc_session::lint::LintBuffer; @@ -647,24 +647,38 @@ impl<'a> AstValidator<'a> { return; } // Find all generic argument coming after the first constraint... - let mut misplaced_args = Vec::new(); - let mut first = None; - for arg in &data.args { - match (arg, first) { - (AngleBracketedArg::Arg(a), Some(_)) => misplaced_args.push(a.span()), - (AngleBracketedArg::Constraint(c), None) => first = Some(c.span), - (AngleBracketedArg::Arg(_), None) | (AngleBracketedArg::Constraint(_), Some(_)) => { - } - } - } + let constraint_spans = data + .args + .iter() + .filter_map(|arg| match arg { + AngleBracketedArg::Constraint(c) => Some(c.span), + _ => None, + }) + .collect::>(); + let arg_spans = data + .args + .iter() + .filter_map(|arg| match arg { + AngleBracketedArg::Arg(a) => Some(a.span()), + _ => None, + }) + .collect::>(); + let constraint_len = constraint_spans.len(); // ...and then error: self.err_handler() .struct_span_err( - misplaced_args.clone(), + arg_spans.clone(), "generic arguments must come before the first constraint", ) - .span_label(first.unwrap(), "the first constraint is provided here") - .span_labels(misplaced_args, "generic argument") + .span_labels( + constraint_spans, + &format!( + "the constraint{} {} provided here", + pluralize!(constraint_len), + if constraint_len == 1 { "is" } else { "are" } + ), + ) + .span_labels(arg_spans, "generic argument") .emit(); } } diff --git a/src/test/ui/parser/issue-32214.stderr b/src/test/ui/parser/issue-32214.stderr index 742f4fdc38bbd..6bb1425ee133f 100644 --- a/src/test/ui/parser/issue-32214.stderr +++ b/src/test/ui/parser/issue-32214.stderr @@ -4,7 +4,7 @@ error: generic arguments must come before the first constraint LL | pub fn test >() {} | ------- ^ generic argument | | - | the first constraint is provided here + | the constraint is provided here error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-move-types.stderr b/src/test/ui/suggestions/suggest-move-types.stderr index 4dd0613757a95..b4cf0deacee46 100644 --- a/src/test/ui/suggestions/suggest-move-types.stderr +++ b/src/test/ui/suggestions/suggest-move-types.stderr @@ -4,7 +4,7 @@ error: generic arguments must come before the first constraint LL | struct A> { | ---- ^ generic argument | | - | the first constraint is provided here + | the constraint is provided here error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:33:43 @@ -13,70 +13,88 @@ LL | struct Al<'a, T, M: OneWithLifetime> { | ---- ^ ^^ generic argument | | | | | generic argument - | the first constraint is provided here + | the constraint is provided here error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:40:46 | LL | struct B> { - | ---- ^ ^ ^ generic argument - | | | | - | | | generic argument - | | generic argument - | the first constraint is provided here + | ---- ---- ---- ^ ^ ^ generic argument + | | | | | | + | | | | | generic argument + | | | | generic argument + | | | the constraints are provided here + | | the constraints are provided here + | the constraints are provided here error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:48:71 | LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ---- ^ ^ ^ ^^ ^^ ^^ generic argument - | | | | | | | - | | | | | | generic argument - | | | | | generic argument - | | | | generic argument - | | | generic argument - | | generic argument - | the first constraint is provided here + | ---- ---- ---- ^ ^ ^ ^^ ^^ ^^ generic argument + | | | | | | | | | + | | | | | | | | generic argument + | | | | | | | generic argument + | | | | | | generic argument + | | | | | generic argument + | | | | generic argument + | | | the constraints are provided here + | | the constraints are provided here + | the constraints are provided here error: generic arguments must come before the first constraint - --> $DIR/suggest-move-types.rs:57:49 + --> $DIR/suggest-move-types.rs:57:28 | LL | struct C> { - | ---- ^ ^ generic argument - | | | - | | generic argument - | the first constraint is provided here + | ^ ---- ---- ---- ^ ^ generic argument + | | | | | | + | | | | | generic argument + | | | | the constraints are provided here + | | | the constraints are provided here + | | the constraints are provided here + | generic argument error: generic arguments must come before the first constraint - --> $DIR/suggest-move-types.rs:65:78 + --> $DIR/suggest-move-types.rs:65:53 | LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ---- ^ ^^ ^ ^^ generic argument - | | | | | - | | | | generic argument - | | | generic argument - | | generic argument - | the first constraint is provided here + | ^ ^^ ---- ---- ---- ^ ^^ ^ ^^ generic argument + | | | | | | | | | + | | | | | | | | generic argument + | | | | | | | generic argument + | | | | | | generic argument + | | | | | the constraints are provided here + | | | | the constraints are provided here + | | | the constraints are provided here + | | generic argument + | generic argument error: generic arguments must come before the first constraint - --> $DIR/suggest-move-types.rs:74:43 + --> $DIR/suggest-move-types.rs:74:28 | LL | struct D> { - | ---- ^ ^ generic argument - | | | - | | generic argument - | the first constraint is provided here + | ^ ---- ---- ^ ---- ^ generic argument + | | | | | | + | | | | | the constraints are provided here + | | | | generic argument + | | | the constraints are provided here + | | the constraints are provided here + | generic argument error: generic arguments must come before the first constraint - --> $DIR/suggest-move-types.rs:82:72 + --> $DIR/suggest-move-types.rs:82:53 | LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ---- ^ ^^ ^ ^^ generic argument - | | | | | - | | | | generic argument - | | | generic argument - | | generic argument - | the first constraint is provided here + | ^ ^^ ---- ---- ^ ^^ ---- ^ ^^ generic argument + | | | | | | | | | + | | | | | | | | generic argument + | | | | | | | the constraints are provided here + | | | | | | generic argument + | | | | | generic argument + | | | | the constraints are provided here + | | | the constraints are provided here + | | generic argument + | generic argument error[E0747]: type provided when a lifetime was expected --> $DIR/suggest-move-types.rs:33:43 From dcb4e817bc8469a0a47e0eaecdf1cf0ea980a496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 28 Mar 2020 17:45:36 -0700 Subject: [PATCH 2/6] Suggest correct order for args and constraints --- src/librustc_ast_passes/ast_validation.rs | 23 +++++++++++ src/test/ui/parser/issue-32214.stderr | 5 +++ .../ui/suggestions/suggest-move-types.stderr | 40 +++++++++++++++++++ 3 files changed, 68 insertions(+) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 680a58d8b7786..4c7edfc83fa33 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -663,6 +663,20 @@ impl<'a> AstValidator<'a> { _ => None, }) .collect::>(); + let snippet_span = match &constraint_spans[..] { + [single] => *single, + [first, .., last] => first.to(*last), + [] => unreachable!(), + }; + let removal_span = match &arg_spans[..] { + [first, ..] => snippet_span.until(*first), + [] => unreachable!(), + }; + let sugg_span = match &arg_spans[..] { + [.., last] => last.shrink_to_hi(), + [] => unreachable!(), + }; + let snippet = self.session.source_map().span_to_snippet(snippet_span).unwrap(); let constraint_len = constraint_spans.len(); // ...and then error: self.err_handler() @@ -679,6 +693,15 @@ impl<'a> AstValidator<'a> { ), ) .span_labels(arg_spans, "generic argument") + .multipart_suggestion( + "move the constraints after the generic arguments", + vec![ + (removal_span, String::new()), + (sugg_span.shrink_to_lo(), ", ".to_string()), + (sugg_span, snippet), + ], + Applicability::MachineApplicable, + ) .emit(); } } diff --git a/src/test/ui/parser/issue-32214.stderr b/src/test/ui/parser/issue-32214.stderr index 6bb1425ee133f..959a1ab121b29 100644 --- a/src/test/ui/parser/issue-32214.stderr +++ b/src/test/ui/parser/issue-32214.stderr @@ -5,6 +5,11 @@ LL | pub fn test >() {} | ------- ^ generic argument | | | the constraint is provided here + | +help: move the constraints after the generic arguments + | +LL | pub fn test >() {} + | --^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-move-types.stderr b/src/test/ui/suggestions/suggest-move-types.stderr index b4cf0deacee46..3d2b35fcf454d 100644 --- a/src/test/ui/suggestions/suggest-move-types.stderr +++ b/src/test/ui/suggestions/suggest-move-types.stderr @@ -5,6 +5,11 @@ LL | struct A> { | ---- ^ generic argument | | | the constraint is provided here + | +help: move the constraints after the generic arguments + | +LL | struct A> { + | --^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:33:43 @@ -14,6 +19,11 @@ LL | struct Al<'a, T, M: OneWithLifetime> { | | | | | generic argument | the constraint is provided here + | +help: move the constraints after the generic arguments + | +LL | struct Al<'a, T, M: OneWithLifetime> { + | -- ^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:40:46 @@ -26,6 +36,11 @@ LL | struct B> { | | | the constraints are provided here | | the constraints are provided here | the constraints are provided here + | +help: move the constraints after the generic arguments + | +LL | struct B> { + | -- ^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:48:71 @@ -41,6 +56,11 @@ LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { + | -- ^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:57:28 @@ -53,6 +73,11 @@ LL | struct C> { | | | the constraints are provided here | | the constraints are provided here | generic argument + | +help: move the constraints after the generic arguments + | +LL | struct C> { + | -- ^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:65:53 @@ -68,6 +93,11 @@ LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { + | -- ^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:74:28 @@ -80,6 +110,11 @@ LL | struct D> { | | | the constraints are provided here | | the constraints are provided here | generic argument + | +help: move the constraints after the generic arguments + | +LL | struct D> { + | -- ^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:82:53 @@ -95,6 +130,11 @@ LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { + | -- ^^^^^^^^^^^^^^^^^^^^^^^ error[E0747]: type provided when a lifetime was expected --> $DIR/suggest-move-types.rs:33:43 From c2e0e71a0943b16e6e6f8aad88aaea7b6f60dea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 29 Mar 2020 11:31:58 -0700 Subject: [PATCH 3/6] Suggest correct order for arguments when encountering early constraints When encountering constraints before type arguments or lifetimes, suggest the correct order. --- src/librustc_ast/ast.rs | 4 +- src/librustc_ast_passes/ast_validation.rs | 56 ++++++++++++------- src/librustc_ast_pretty/pprust.rs | 4 +- src/librustc_errors/diagnostic_builder.rs | 14 +++++ src/test/ui/parser/issue-32214.stderr | 6 +- .../ui/suggestions/suggest-move-types.stderr | 36 ++++++------ 6 files changed, 74 insertions(+), 46 deletions(-) diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index 6586280d21454..f91cbe51d85d1 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -300,8 +300,8 @@ pub enum GenericBound { impl GenericBound { pub fn span(&self) -> Span { match self { - &GenericBound::Trait(ref t, ..) => t.span, - &GenericBound::Outlives(ref l) => l.ident.span, + GenericBound::Trait(ref t, ..) => t.span, + GenericBound::Outlives(ref l) => l.ident.span, } } } diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 4c7edfc83fa33..4f076c963c1b6 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -640,6 +640,32 @@ impl<'a> AstValidator<'a> { } } + fn suggest_correct_generic_order(&self, data: &AngleBracketedArgs) -> String { + // Lifetimes always come first. + let lt_sugg = data.args.iter().filter_map(|arg| match arg { + AngleBracketedArg::Arg(lt @ GenericArg::Lifetime(_)) => { + Some(pprust::to_string(|s| s.print_generic_arg(lt))) + } + _ => None, + }); + let args_sugg = data.args.iter().filter_map(|a| match a { + AngleBracketedArg::Arg(GenericArg::Lifetime(_)) => None, + AngleBracketedArg::Arg(arg) => Some(pprust::to_string(|s| s.print_generic_arg(arg))), + AngleBracketedArg::Constraint(_) => None, + }); + // Cosntraints always come last. + let constraint_sugg = data.args.iter().filter_map(|a| match a { + AngleBracketedArg::Arg(_) => None, + AngleBracketedArg::Constraint(c) => { + Some(pprust::to_string(|s| s.print_assoc_constraint(c))) + } + }); + format!( + "<{}>", + lt_sugg.chain(args_sugg).chain(constraint_sugg).collect::>().join(", ") + ) + } + /// Enforce generic args coming before constraints in `<...>` of a path segment. fn check_generic_args_before_constraints(&self, data: &AngleBracketedArgs) { // Early exit in case it's partitioned as it should be. @@ -663,20 +689,7 @@ impl<'a> AstValidator<'a> { _ => None, }) .collect::>(); - let snippet_span = match &constraint_spans[..] { - [single] => *single, - [first, .., last] => first.to(*last), - [] => unreachable!(), - }; - let removal_span = match &arg_spans[..] { - [first, ..] => snippet_span.until(*first), - [] => unreachable!(), - }; - let sugg_span = match &arg_spans[..] { - [.., last] => last.shrink_to_hi(), - [] => unreachable!(), - }; - let snippet = self.session.source_map().span_to_snippet(snippet_span).unwrap(); + let args_len = arg_spans.len(); let constraint_len = constraint_spans.len(); // ...and then error: self.err_handler() @@ -693,13 +706,14 @@ impl<'a> AstValidator<'a> { ), ) .span_labels(arg_spans, "generic argument") - .multipart_suggestion( - "move the constraints after the generic arguments", - vec![ - (removal_span, String::new()), - (sugg_span.shrink_to_lo(), ", ".to_string()), - (sugg_span, snippet), - ], + .span_suggestion_verbose( + data.span, + &format!( + "move the constraint{} after the generic argument{}", + pluralize!(constraint_len), + pluralize!(args_len) + ), + self.suggest_correct_generic_order(&data), Applicability::MachineApplicable, ) .emit(); diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs index 4aabbe7efbef4..5ac96c5c8cb55 100644 --- a/src/librustc_ast_pretty/pprust.rs +++ b/src/librustc_ast_pretty/pprust.rs @@ -870,7 +870,7 @@ impl<'a> State<'a> { } } - fn print_assoc_constraint(&mut self, constraint: &ast::AssocTyConstraint) { + pub fn print_assoc_constraint(&mut self, constraint: &ast::AssocTyConstraint) { self.print_ident(constraint.ident); self.s.space(); match &constraint.kind { @@ -884,7 +884,7 @@ impl<'a> State<'a> { } } - crate fn print_generic_arg(&mut self, generic_arg: &GenericArg) { + pub fn print_generic_arg(&mut self, generic_arg: &GenericArg) { match generic_arg { GenericArg::Lifetime(lt) => self.print_lifetime(*lt), GenericArg::Type(ty) => self.print_type(ty), diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index fffae0bfd24d9..2dbd9f4e52fad 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -315,6 +315,20 @@ impl<'a> DiagnosticBuilder<'a> { self } + pub fn span_suggestion_verbose( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + ) -> &mut Self { + if !self.0.allow_suggestions { + return self; + } + self.0.diagnostic.span_suggestion_verbose(sp, msg, suggestion, applicability); + self + } + pub fn span_suggestion_hidden( &mut self, sp: Span, diff --git a/src/test/ui/parser/issue-32214.stderr b/src/test/ui/parser/issue-32214.stderr index 959a1ab121b29..8daf76993bc06 100644 --- a/src/test/ui/parser/issue-32214.stderr +++ b/src/test/ui/parser/issue-32214.stderr @@ -6,10 +6,10 @@ LL | pub fn test >() {} | | | the constraint is provided here | -help: move the constraints after the generic arguments +help: move the constraint after the generic argument | -LL | pub fn test >() {} - | --^^^^^^^ +LL | pub fn test >() {} + | ^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-move-types.stderr b/src/test/ui/suggestions/suggest-move-types.stderr index 3d2b35fcf454d..e8cf54e276771 100644 --- a/src/test/ui/suggestions/suggest-move-types.stderr +++ b/src/test/ui/suggestions/suggest-move-types.stderr @@ -6,10 +6,10 @@ LL | struct A> { | | | the constraint is provided here | -help: move the constraints after the generic arguments +help: move the constraint after the generic argument | -LL | struct A> { - | --^^^^ +LL | struct A> { + | ^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:33:43 @@ -20,10 +20,10 @@ LL | struct Al<'a, T, M: OneWithLifetime> { | | generic argument | the constraint is provided here | -help: move the constraints after the generic arguments +help: move the constraint after the generic arguments | -LL | struct Al<'a, T, M: OneWithLifetime> { - | -- ^^^^ +LL | struct Al<'a, T, M: OneWithLifetime<'a, T, A = ()>> { + | ^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:40:46 @@ -39,8 +39,8 @@ LL | struct B> { | help: move the constraints after the generic arguments | -LL | struct B> { - | -- ^^^^^^^^^^^^^^^^ +LL | struct B> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:48:71 @@ -59,8 +59,8 @@ LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | -- ^^^^^^^^^^^^^^^^ +LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<'a, 'b, 'c, T, U, V, A = (), B = (), C = ()>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:57:28 @@ -76,8 +76,8 @@ LL | struct C> { | help: move the constraints after the generic arguments | -LL | struct C> { - | -- ^^^^^^^^^^^^^^^^ +LL | struct C> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:65:53 @@ -96,8 +96,8 @@ LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | -- ^^^^^^^^^^^^^^^^ +LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<'a, 'b, 'c, T, U, V, A = (), B = (), C = ()>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:74:28 @@ -113,8 +113,8 @@ LL | struct D> { | help: move the constraints after the generic arguments | -LL | struct D> { - | -- ^^^^^^^^^^^^^^^^^^^ +LL | struct D> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:82:53 @@ -133,8 +133,8 @@ LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | -- ^^^^^^^^^^^^^^^^^^^^^^^ +LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<'a, 'b, 'c, T, U, V, A = (), B = (), C = ()>> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0747]: type provided when a lifetime was expected --> $DIR/suggest-move-types.rs:33:43 From e84cb65fe1f883a017b91fcb1eda64215fd08626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 29 Mar 2020 11:46:00 -0700 Subject: [PATCH 4/6] review comment: wording --- src/librustc_ast_passes/ast_validation.rs | 9 +---- src/test/ui/parser/issue-32214.stderr | 2 +- .../ui/suggestions/suggest-move-types.stderr | 40 +++++++++---------- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 4f076c963c1b6..aaf0212ca0287 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -697,14 +697,7 @@ impl<'a> AstValidator<'a> { arg_spans.clone(), "generic arguments must come before the first constraint", ) - .span_labels( - constraint_spans, - &format!( - "the constraint{} {} provided here", - pluralize!(constraint_len), - if constraint_len == 1 { "is" } else { "are" } - ), - ) + .span_labels(constraint_spans, "constraint") .span_labels(arg_spans, "generic argument") .span_suggestion_verbose( data.span, diff --git a/src/test/ui/parser/issue-32214.stderr b/src/test/ui/parser/issue-32214.stderr index 8daf76993bc06..bc61b3b74e217 100644 --- a/src/test/ui/parser/issue-32214.stderr +++ b/src/test/ui/parser/issue-32214.stderr @@ -4,7 +4,7 @@ error: generic arguments must come before the first constraint LL | pub fn test >() {} | ------- ^ generic argument | | - | the constraint is provided here + | constraint | help: move the constraint after the generic argument | diff --git a/src/test/ui/suggestions/suggest-move-types.stderr b/src/test/ui/suggestions/suggest-move-types.stderr index e8cf54e276771..4d31a472fe4eb 100644 --- a/src/test/ui/suggestions/suggest-move-types.stderr +++ b/src/test/ui/suggestions/suggest-move-types.stderr @@ -4,7 +4,7 @@ error: generic arguments must come before the first constraint LL | struct A> { | ---- ^ generic argument | | - | the constraint is provided here + | constraint | help: move the constraint after the generic argument | @@ -18,7 +18,7 @@ LL | struct Al<'a, T, M: OneWithLifetime> { | ---- ^ ^^ generic argument | | | | | generic argument - | the constraint is provided here + | constraint | help: move the constraint after the generic arguments | @@ -33,9 +33,9 @@ LL | struct B> { | | | | | | | | | | | generic argument | | | | generic argument - | | | the constraints are provided here - | | the constraints are provided here - | the constraints are provided here + | | | constraint + | | constraint + | constraint | help: move the constraints after the generic arguments | @@ -53,9 +53,9 @@ LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { | ^ ---- ---- ---- ^ ^ generic argument | | | | | | | | | | | generic argument - | | | | the constraints are provided here - | | | the constraints are provided here - | | the constraints are provided here + | | | | constraint + | | | constraint + | | constraint | generic argument | help: move the constraints after the generic arguments @@ -88,9 +88,9 @@ LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { | ^ ---- ---- ^ ---- ^ generic argument | | | | | | - | | | | | the constraints are provided here + | | | | | constraint | | | | generic argument - | | | the constraints are provided here - | | the constraints are provided here + | | | constraint + | | constraint | generic argument | help: move the constraints after the generic arguments @@ -123,11 +123,11 @@ LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime Date: Sun, 5 Apr 2020 16:34:16 -0700 Subject: [PATCH 5/6] review comments: use `partition_map` --- Cargo.lock | 1 + src/librustc_ast_passes/Cargo.toml | 1 + src/librustc_ast_passes/ast_validation.rs | 33 +++++++++-------------- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a592c82225e6d..14c2630180785 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3537,6 +3537,7 @@ dependencies = [ name = "rustc_ast_passes" version = "0.0.0" dependencies = [ + "itertools 0.8.0", "log", "rustc_ast", "rustc_ast_pretty", diff --git a/src/librustc_ast_passes/Cargo.toml b/src/librustc_ast_passes/Cargo.toml index 5d096e4965ddc..e4d1d79abb2d6 100644 --- a/src/librustc_ast_passes/Cargo.toml +++ b/src/librustc_ast_passes/Cargo.toml @@ -9,6 +9,7 @@ name = "rustc_ast_passes" path = "lib.rs" [dependencies] +itertools = "0.8" log = "0.4" rustc_ast_pretty = { path = "../librustc_ast_pretty" } rustc_attr = { path = "../librustc_attr" } diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index aaf0212ca0287..82ba52a3f1466 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -6,6 +6,7 @@ // This pass is supposed to perform only simple checks not requiring name resolution // or type checking or some other kind of complex analysis. +use itertools::{Either, Itertools}; use rustc_ast::ast::*; use rustc_ast::attr; use rustc_ast::expand::is_proc_macro_attr; @@ -640,7 +641,7 @@ impl<'a> AstValidator<'a> { } } - fn suggest_correct_generic_order(&self, data: &AngleBracketedArgs) -> String { + fn correct_generic_order_suggestion(&self, data: &AngleBracketedArgs) -> String { // Lifetimes always come first. let lt_sugg = data.args.iter().filter_map(|arg| match arg { AngleBracketedArg::Arg(lt @ GenericArg::Lifetime(_)) => { @@ -649,11 +650,12 @@ impl<'a> AstValidator<'a> { _ => None, }); let args_sugg = data.args.iter().filter_map(|a| match a { - AngleBracketedArg::Arg(GenericArg::Lifetime(_)) => None, + AngleBracketedArg::Arg(GenericArg::Lifetime(_)) | AngleBracketedArg::Constraint(_) => { + None + } AngleBracketedArg::Arg(arg) => Some(pprust::to_string(|s| s.print_generic_arg(arg))), - AngleBracketedArg::Constraint(_) => None, }); - // Cosntraints always come last. + // Constraints always come last. let constraint_sugg = data.args.iter().filter_map(|a| match a { AngleBracketedArg::Arg(_) => None, AngleBracketedArg::Constraint(c) => { @@ -673,22 +675,11 @@ impl<'a> AstValidator<'a> { return; } // Find all generic argument coming after the first constraint... - let constraint_spans = data - .args - .iter() - .filter_map(|arg| match arg { - AngleBracketedArg::Constraint(c) => Some(c.span), - _ => None, - }) - .collect::>(); - let arg_spans = data - .args - .iter() - .filter_map(|arg| match arg { - AngleBracketedArg::Arg(a) => Some(a.span()), - _ => None, - }) - .collect::>(); + let (constraint_spans, arg_spans): (Vec, Vec) = + data.args.iter().partition_map(|arg| match arg { + AngleBracketedArg::Constraint(c) => Either::Left(c.span), + AngleBracketedArg::Arg(a) => Either::Right(a.span()), + }); let args_len = arg_spans.len(); let constraint_len = constraint_spans.len(); // ...and then error: @@ -706,7 +697,7 @@ impl<'a> AstValidator<'a> { pluralize!(constraint_len), pluralize!(args_len) ), - self.suggest_correct_generic_order(&data), + self.correct_generic_order_suggestion(&data), Applicability::MachineApplicable, ) .emit(); From 17a95232b35766a24d2575c7fa1ef52c9fbf97de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 17:02:44 -0700 Subject: [PATCH 6/6] Reduce the visual clutter Using a single label for constraints and generic arguments. --- src/librustc_ast_passes/ast_validation.rs | 9 ++- .../ui/suggestions/suggest-move-types.stderr | 74 +++++-------------- 2 files changed, 27 insertions(+), 56 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 82ba52a3f1466..9563325fe329e 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -688,8 +688,13 @@ impl<'a> AstValidator<'a> { arg_spans.clone(), "generic arguments must come before the first constraint", ) - .span_labels(constraint_spans, "constraint") - .span_labels(arg_spans, "generic argument") + .span_label(constraint_spans[0], &format!("constraint{}", pluralize!(constraint_len))) + .span_label( + *arg_spans.iter().last().unwrap(), + &format!("generic argument{}", pluralize!(args_len)), + ) + .span_labels(constraint_spans, "") + .span_labels(arg_spans, "") .span_suggestion_verbose( data.span, &format!( diff --git a/src/test/ui/suggestions/suggest-move-types.stderr b/src/test/ui/suggestions/suggest-move-types.stderr index 4d31a472fe4eb..3bb6fd6e4f423 100644 --- a/src/test/ui/suggestions/suggest-move-types.stderr +++ b/src/test/ui/suggestions/suggest-move-types.stderr @@ -15,9 +15,8 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:33:43 | LL | struct Al<'a, T, M: OneWithLifetime> { - | ---- ^ ^^ generic argument - | | | - | | generic argument + | ---- ^ ^^ generic arguments + | | | constraint | help: move the constraint after the generic arguments @@ -29,13 +28,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:40:46 | LL | struct B> { - | ---- ---- ---- ^ ^ ^ generic argument - | | | | | | - | | | | | generic argument - | | | | generic argument - | | | constraint - | | constraint - | constraint + | ---- ---- ---- ^ ^ ^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments | @@ -46,16 +41,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:48:71 | LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ---- ---- ---- ^ ^ ^ ^^ ^^ ^^ generic argument - | | | | | | | | | - | | | | | | | | generic argument - | | | | | | | generic argument - | | | | | | generic argument - | | | | | generic argument - | | | | generic argument - | | | constraint - | | constraint - | constraint + | ---- ---- ---- ^ ^ ^ ^^ ^^ ^^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments | @@ -66,13 +54,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:57:28 | LL | struct C> { - | ^ ---- ---- ---- ^ ^ generic argument - | | | | | | - | | | | | generic argument - | | | | constraint - | | | constraint - | | constraint - | generic argument + | ^ ---- ---- ---- ^ ^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments | @@ -83,16 +67,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:65:53 | LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ^ ^^ ---- ---- ---- ^ ^^ ^ ^^ generic argument - | | | | | | | | | - | | | | | | | | generic argument - | | | | | | | generic argument - | | | | | | generic argument - | | | | | constraint - | | | | constraint - | | | constraint - | | generic argument - | generic argument + | ^ ^^ ---- ---- ---- ^ ^^ ^ ^^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments | @@ -103,13 +80,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:74:28 | LL | struct D> { - | ^ ---- ---- ^ ---- ^ generic argument - | | | | | | - | | | | | constraint - | | | | generic argument - | | | constraint - | | constraint - | generic argument + | ^ ---- ---- ^ ---- ^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments | @@ -120,16 +93,9 @@ error: generic arguments must come before the first constraint --> $DIR/suggest-move-types.rs:82:53 | LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime> { - | ^ ^^ ---- ---- ^ ^^ ---- ^ ^^ generic argument - | | | | | | | | | - | | | | | | | | generic argument - | | | | | | | constraint - | | | | | | generic argument - | | | | | generic argument - | | | | constraint - | | | constraint - | | generic argument - | generic argument + | ^ ^^ ---- ---- ^ ^^ ---- ^ ^^ generic arguments + | | + | constraints | help: move the constraints after the generic arguments |