Skip to content
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.

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

Add error for trailing angle brackets. #57817

Merged
merged 3 commits into from
Jan 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,20 @@ pub enum GenericArgs {
}

impl GenericArgs {
pub fn is_parenthesized(&self) -> bool {
match *self {
Parenthesized(..) => true,
_ => false,
}
}

pub fn is_angle_bracketed(&self) -> bool {
match *self {
AngleBracketed(..) => true,
_ => false,
}
}

pub fn span(&self) -> Span {
match *self {
AngleBracketed(ref data) => data.span,
Expand Down
134 changes: 133 additions & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2149,7 +2149,27 @@ impl<'a> Parser<'a> {
enable_warning: bool)
-> PResult<'a, ()> {
loop {
segments.push(self.parse_path_segment(style, enable_warning)?);
let segment = self.parse_path_segment(style, enable_warning)?;
if style == PathStyle::Expr {
// In order to check for trailing angle brackets, we must have finished
// recursing (`parse_path_segment` can indirectly call this function),
// that is, the next token must be the highlighted part of the below example:
//
// `Foo::<Bar as Baz<T>>::Qux`
// ^ here
//
// As opposed to the below highlight (if we had only finished the first
// recursion):
//
// `Foo::<Bar as Baz<T>>::Qux`
// ^ here
//
// `PathStyle::Expr` is only provided at the root invocation and never in
// `parse_path_segment` to recurse and therefore can be checked to maintain
// this invariant.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@estebank do you know if this last sentence is accurate? It works and passes tests but I'm not sure if PathStyle::Expr was intended to make this condition hold.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be ok.

self.check_trailing_angle_brackets(&segment, token::ModSep);
}
segments.push(segment);

if self.is_import_coupler() || !self.eat(&token::ModSep) {
return Ok(());
Expand Down Expand Up @@ -2757,6 +2777,8 @@ impl<'a> Parser<'a> {
// Assuming we have just parsed `.`, continue parsing into an expression.
fn parse_dot_suffix(&mut self, self_arg: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
let segment = self.parse_path_segment(PathStyle::Expr, true)?;
self.check_trailing_angle_brackets(&segment, token::OpenDelim(token::Paren));

Ok(match self.token {
token::OpenDelim(token::Paren) => {
// Method call `expr.f()`
Expand Down Expand Up @@ -2784,6 +2806,116 @@ impl<'a> Parser<'a> {
})
}

/// This function checks if there are trailing angle brackets and produces
/// a diagnostic to suggest removing them.
///
/// ```ignore (diagnostic)
/// let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
/// ^^ help: remove extra angle brackets
/// ```
fn check_trailing_angle_brackets(&mut self, segment: &PathSegment, end: token::Token) {
// This function is intended to be invoked after parsing a path segment where there are two
// cases:
//
// 1. A specific token is expected after the path segment.
// eg. `x.foo(`, `x.foo::<u32>(` (parenthesis - method call),
// `Foo::`, or `Foo::<Bar>::` (mod sep - continued path).
// 2. No specific token is expected after the path segment.
// eg. `x.foo` (field access)
//
// This function is called after parsing `.foo` and before parsing the token `end` (if
// present). This includes any angle bracket arguments, such as `.foo::<u32>` or
// `Foo::<Bar>`.

// We only care about trailing angle brackets if we previously parsed angle bracket
// arguments. This helps stop us incorrectly suggesting that extra angle brackets be
// removed in this case:
//
// `x.foo >> (3)` (where `x.foo` is a `u32` for example)
//
// This case is particularly tricky as we won't notice it just looking at the tokens -
// it will appear the same (in terms of upcoming tokens) as below (since the `::<u32>` will
// have already been parsed):
//
// `x.foo::<u32>>>(3)`
let parsed_angle_bracket_args = segment.args
.as_ref()
.map(|args| args.is_angle_bracketed())
.unwrap_or(false);

debug!(
"check_trailing_angle_brackets: parsed_angle_bracket_args={:?}",
parsed_angle_bracket_args,
);
if !parsed_angle_bracket_args {
return;
}

// Keep the span at the start so we can highlight the sequence of `>` characters to be
// removed.
let lo = self.span;

// We need to look-ahead to see if we have `>` characters without moving the cursor forward
// (since we might have the field access case and the characters we're eating are
// actual operators and not trailing characters - ie `x.foo >> 3`).
let mut position = 0;

// We can encounter `>` or `>>` tokens in any order, so we need to keep track of how
// many of each (so we can correctly pluralize our error messages) and continue to
// advance.
let mut number_of_shr = 0;
let mut number_of_gt = 0;
while self.look_ahead(position, |t| {
trace!("check_trailing_angle_brackets: t={:?}", t);
if *t == token::BinOp(token::BinOpToken::Shr) {
number_of_shr += 1;
true
} else if *t == token::Gt {
number_of_gt += 1;
true
} else {
false
}
}) {
position += 1;
}

// If we didn't find any trailing `>` characters, then we have nothing to error about.
debug!(
"check_trailing_angle_brackets: number_of_gt={:?} number_of_shr={:?}",
number_of_gt, number_of_shr,
);
if number_of_gt < 1 && number_of_shr < 1 {
return;
}

// Finally, double check that we have our end token as otherwise this is the
// second case.
if self.look_ahead(position, |t| {
trace!("check_trailing_angle_brackets: t={:?}", t);
*t == end
}) {
// Eat from where we started until the end token so that parsing can continue
// as if we didn't have those extra angle brackets.
self.eat_to_tokens(&[&end]);
let span = lo.until(self.span);

let plural = number_of_gt > 1 || number_of_shr >= 1;
self.diagnostic()
.struct_span_err(
span,
&format!("unmatched angle bracket{}", if plural { "s" } else { "" }),
)
.span_suggestion_with_applicability(
span,
&format!("remove extra angle bracket{}", if plural { "s" } else { "" }),
String::new(),
Applicability::MachineApplicable,
)
.emit();
}
}

fn parse_dot_or_call_expr_with_(&mut self, e0: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
let mut e = e0;
let mut hi;
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/issues/issue-54521-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// compile-pass

// This test checks that the `remove extra angle brackets` error doesn't happen for some
// potential edge-cases..

struct X {
len: u32,
}

fn main() {
let x = X { len: 3 };

let _ = x.len > (3);

let _ = x.len >> (3);
}
22 changes: 22 additions & 0 deletions src/test/ui/issues/issue-54521-2.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix

// This test checks that the following error is emitted and the suggestion works:
//
// ```
// let _ = Vec::<usize>>>::new();
// ^^ help: remove extra angle brackets
// ```

fn main() {
let _ = Vec::<usize>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>::new();
//~^ ERROR unmatched angle bracket
}
22 changes: 22 additions & 0 deletions src/test/ui/issues/issue-54521-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix

// This test checks that the following error is emitted and the suggestion works:
//
// ```
// let _ = Vec::<usize>>>::new();
// ^^ help: remove extra angle brackets
// ```

fn main() {
let _ = Vec::<usize>>>>>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>>>>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>>>::new();
//~^ ERROR unmatched angle bracket

let _ = Vec::<usize>>::new();
//~^ ERROR unmatched angle bracket
}
26 changes: 26 additions & 0 deletions src/test/ui/issues/issue-54521-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: unmatched angle brackets
--> $DIR/issue-54521-2.rs:11:25
|
LL | let _ = Vec::<usize>>>>>::new();
| ^^^^ help: remove extra angle brackets

error: unmatched angle brackets
--> $DIR/issue-54521-2.rs:14:25
|
LL | let _ = Vec::<usize>>>>::new();
| ^^^ help: remove extra angle brackets

error: unmatched angle brackets
--> $DIR/issue-54521-2.rs:17:25
|
LL | let _ = Vec::<usize>>>::new();
| ^^ help: remove extra angle brackets

error: unmatched angle bracket
--> $DIR/issue-54521-2.rs:20:25
|
LL | let _ = Vec::<usize>>::new();
| ^ help: remove extra angle bracket

error: aborting due to 4 previous errors

22 changes: 22 additions & 0 deletions src/test/ui/issues/issue-54521.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix

// This test checks that the following error is emitted and the suggestion works:
//
// ```
// let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
// ^^ help: remove extra angle brackets
// ```

fn main() {
let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>();
//~^ ERROR unmatched angle bracket
}
22 changes: 22 additions & 0 deletions src/test/ui/issues/issue-54521.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// run-rustfix

// This test checks that the following error is emitted and the suggestion works:
//
// ```
// let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
// ^^ help: remove extra angle brackets
// ```

fn main() {
let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
//~^ ERROR unmatched angle bracket

let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>();
//~^ ERROR unmatched angle bracket
}
26 changes: 26 additions & 0 deletions src/test/ui/issues/issue-54521.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: unmatched angle brackets
--> $DIR/issue-54521.rs:11:60
|
LL | let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>>>();
| ^^^^ help: remove extra angle brackets

error: unmatched angle brackets
--> $DIR/issue-54521.rs:14:60
|
LL | let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>>();
| ^^^ help: remove extra angle brackets

error: unmatched angle brackets
--> $DIR/issue-54521.rs:17:60
|
LL | let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
| ^^ help: remove extra angle brackets

error: unmatched angle bracket
--> $DIR/issue-54521.rs:20:60
|
LL | let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>();
| ^ help: remove extra angle bracket

error: aborting due to 4 previous errors