Skip to content

Tweak output of type params and constraints in the wrong order #70519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 6, 2020
Merged

Tweak output of type params and constraints in the wrong order #70519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
33d793c
Point at all constraints before args
estebank Mar 28, 2020
dcb4e81
Suggest correct order for args and constraints
estebank Mar 29, 2020
c2e0e71
Suggest correct order for arguments when encountering early constraints
estebank Mar 29, 2020
e84cb65
review comment: wording
estebank Mar 29, 2020
c9f55ea
review comments: use `partition_map`
estebank Apr 5, 2020
17a9523
Reduce the visual clutter
estebank Apr 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3537,6 +3537,7 @@ dependencies = [
name = "rustc_ast_passes"
version = "0.0.0"
dependencies = [
"itertools 0.8.0",
"log",
"rustc_ast",
"rustc_ast_pretty",
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_ast/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_ast_passes/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
68 changes: 54 additions & 14 deletions src/librustc_ast_passes/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -14,7 +15,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;
Expand Down Expand Up @@ -640,31 +641,70 @@ impl<'a> AstValidator<'a> {
}
}

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(_)) => {
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(_)) | AngleBracketedArg::Constraint(_) => {
None
}
AngleBracketedArg::Arg(arg) => Some(pprust::to_string(|s| s.print_generic_arg(arg))),
});
// Constraints 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::<Vec<String>>().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.
if data.args.iter().is_partitioned(|arg| matches!(arg, AngleBracketedArg::Arg(_))) {
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, arg_spans): (Vec<Span>, Vec<Span>) =
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:
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_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!(
"move the constraint{} after the generic argument{}",
pluralize!(constraint_len),
pluralize!(args_len)
),
self.correct_generic_order_suggestion(&data),
Applicability::MachineApplicable,
)
.emit();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_ast_pretty/pprust.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
Expand Down
14 changes: 14 additions & 0 deletions src/librustc_errors/diagnostic_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/parser/issue-32214.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ error: generic arguments must come before the first constraint
LL | pub fn test<W, I: Trait<Item=(), W> >() {}
| ------- ^ generic argument
| |
| the first constraint is provided here
| constraint
|
help: move the constraint after the generic argument
|
LL | pub fn test<W, I: Trait<W, Item = ()> >() {}
| ^^^^^^^^^^^^^^

error: aborting due to previous error

108 changes: 66 additions & 42 deletions src/test/ui/suggestions/suggest-move-types.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,79 +4,103 @@ error: generic arguments must come before the first constraint
LL | struct A<T, M: One<A=(), T>> {
| ---- ^ generic argument
| |
| the first constraint is provided here
| constraint
|
help: move the constraint after the generic argument
|
LL | struct A<T, M: One<T, A = ()>> {
| ^^^^^^^^^^^

error: generic arguments must come before the first constraint
--> $DIR/suggest-move-types.rs:33:43
|
LL | struct Al<'a, T, M: OneWithLifetime<A=(), T, 'a>> {
| ---- ^ ^^ generic argument
| | |
| | generic argument
| the first constraint is provided here
| ---- ^ ^^ generic arguments
| |
| constraint
|
help: move the constraint after the generic arguments
|
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
|
LL | struct B<T, U, V, M: Three<A=(), B=(), C=(), T, U, V>> {
| ---- ^ ^ ^ generic argument
| | | |
| | | generic argument
| | generic argument
| the first constraint is provided here
| ---- ---- ---- ^ ^ ^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
LL | struct B<T, U, V, M: Three<T, U, V, A = (), B = (), C = ()>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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<A=(), B=(), C=(), T, U, V, 'a, 'b, 'c>> {
| ---- ^ ^ ^ ^^ ^^ ^^ generic argument
| | | | | | |
| | | | | | generic argument
| | | | | generic argument
| | | | generic argument
| | | generic argument
| | generic argument
| the first constraint is provided here
| ---- ---- ---- ^ ^ ^ ^^ ^^ ^^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
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:49
--> $DIR/suggest-move-types.rs:57:28
|
LL | struct C<T, U, V, M: Three<T, A=(), B=(), C=(), U, V>> {
| ---- ^ ^ generic argument
| | |
| | generic argument
| the first constraint is provided here
| ^ ---- ---- ---- ^ ^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
LL | struct C<T, U, V, M: Three<T, U, V, A = (), B = (), C = ()>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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<T, 'a, A=(), B=(), C=(), U, 'b, V, 'c>> {
| ---- ^ ^^ ^ ^^ generic argument
| | | | |
| | | | generic argument
| | | generic argument
| | generic argument
| the first constraint is provided here
| ^ ^^ ---- ---- ---- ^ ^^ ^ ^^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
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:43
--> $DIR/suggest-move-types.rs:74:28
|
LL | struct D<T, U, V, M: Three<T, A=(), B=(), U, C=(), V>> {
| ---- ^ ^ generic argument
| | |
| | generic argument
| the first constraint is provided here
| ^ ---- ---- ^ ---- ^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
LL | struct D<T, U, V, M: Three<T, U, V, A = (), B = (), C = ()>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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<T, 'a, A=(), B=(), U, 'b, C=(), V, 'c>> {
| ---- ^ ^^ ^ ^^ generic argument
| | | | |
| | | | generic argument
| | | generic argument
| | generic argument
| the first constraint is provided here
| ^ ^^ ---- ---- ^ ^^ ---- ^ ^^ generic arguments
| |
| constraints
|
help: move the constraints after the generic arguments
|
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
Expand Down