Skip to content

Commit

Permalink
Include bounds in generic reordering diagnostic.
Browse files Browse the repository at this point in the history
This commit extends the existing generic re-ordering diagnostic to
include any bounds on the generic parameter, thus producing correct
suggestions.
  • Loading branch information
davidtwco committed Mar 30, 2019
1 parent e782d79 commit 3829746
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 12 deletions.
37 changes: 25 additions & 12 deletions src/librustc_passes/ast_validation.rs
Expand Up @@ -361,17 +361,24 @@ enum GenericPosition {

fn validate_generics_order<'a>(
handler: &errors::Handler,
generics: impl Iterator<Item = (ParamKindOrd, Span, Option<String>)>,
generics: impl Iterator<
Item = (
ParamKindOrd,
Option<&'a [GenericBound]>,
Span,
Option<String>
),
>,
pos: GenericPosition,
span: Span,
) {
let mut max_param: Option<ParamKindOrd> = None;
let mut out_of_order = FxHashMap::default();
let mut param_idents = vec![];

for (kind, span, ident) in generics {
for (kind, bounds, span, ident) in generics {
if let Some(ident) = ident {
param_idents.push((kind, param_idents.len(), ident));
param_idents.push((kind, bounds, param_idents.len(), ident));
}
let max_param = &mut max_param;
match max_param {
Expand All @@ -385,13 +392,19 @@ fn validate_generics_order<'a>(

let mut ordered_params = "<".to_string();
if !out_of_order.is_empty() {
param_idents.sort_by_key(|&(po, i, _)| (po, i));
param_idents.sort_by_key(|&(po, _, i, _)| (po, i));
let mut first = true;
for (_, _, ident) in param_idents {
for (_, bounds, _, ident) in param_idents {
if !first {
ordered_params += ", ";
}
ordered_params += &ident;
if let Some(bounds) = bounds {
if !bounds.is_empty() {
ordered_params += ": ";
ordered_params += &pprust::bounds_to_string(&bounds);
}
}
first = false;
}
}
Expand Down Expand Up @@ -701,7 +714,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
GenericArg::Lifetime(..) => ParamKindOrd::Lifetime,
GenericArg::Type(..) => ParamKindOrd::Type,
GenericArg::Const(..) => ParamKindOrd::Const,
}, arg.span(), None)
}, None, arg.span(), None)
}), GenericPosition::Arg, generic_args.span());

// Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
Expand Down Expand Up @@ -736,16 +749,16 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}

validate_generics_order(self.err_handler(), generics.params.iter().map(|param| {
let span = param.ident.span;
let ident = Some(param.ident.to_string());
match &param.kind {
GenericParamKind::Lifetime { .. } => (ParamKindOrd::Lifetime, span, ident),
GenericParamKind::Type { .. } => (ParamKindOrd::Type, span, ident),
let (kind, ident) = match &param.kind {
GenericParamKind::Lifetime { .. } => (ParamKindOrd::Lifetime, ident),
GenericParamKind::Type { .. } => (ParamKindOrd::Type, ident),
GenericParamKind::Const { ref ty } => {
let ty = pprust::ty_to_string(ty);
(ParamKindOrd::Const, span, Some(format!("const {}: {}", param.ident, ty)))
(ParamKindOrd::Const, Some(format!("const {}: {}", param.ident, ty)))
}
}
};
(kind, Some(&*param.bounds), param.ident.span, ident)
}), GenericPosition::Param, generics.span);

for predicate in &generics.where_clause.predicates {
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/issue-59508.fixed
@@ -0,0 +1,16 @@
// run-rustfix

#![allow(dead_code)]

// This test checks that generic parameter re-ordering diagnostic suggestions contain bounds.

struct A;

impl A {
pub fn do_things<'a, 'b: 'a, T>() {
//~^ ERROR lifetime parameters must be declared prior to type parameters
println!("panic");
}
}

fn main() {}
16 changes: 16 additions & 0 deletions src/test/ui/issue-59508.rs
@@ -0,0 +1,16 @@
// run-rustfix

#![allow(dead_code)]

// This test checks that generic parameter re-ordering diagnostic suggestions contain bounds.

struct A;

impl A {
pub fn do_things<T, 'a, 'b: 'a>() {
//~^ ERROR lifetime parameters must be declared prior to type parameters
println!("panic");
}
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/issue-59508.stderr
@@ -0,0 +1,8 @@
error: lifetime parameters must be declared prior to type parameters
--> $DIR/issue-59508.rs:10:25
|
LL | pub fn do_things<T, 'a, 'b: 'a>() {
| ----^^--^^----- help: reorder the parameters: lifetimes, then types, then consts: `<'a, 'b: 'a, T>`

error: aborting due to previous error

0 comments on commit 3829746

Please sign in to comment.