Skip to content

Commit 18d300f

Browse files
davidtwcoVardhan Thigle
authored andcommitted
Extend trailing > detection for paths.
This commit extends the trailing `>` detection to also work for paths such as `Foo::<Bar>>:Baz`. This involves making the existing check take the token that is expected to follow the path being checked as a parameter. Care is taken to ensure that this only happens on the construction of a whole path segment and not a partial path segment (during recursion). Through this enhancement, it was also observed that the ordering of right shift token and greater than tokens was overfitted to the examples being tested. In practice, given a sequence of `>` characters: `>>>>>>>>>` ..then they will be split into `>>` eagerly: `>> >> >> >> >`. ..but when a `<` is prepended, then the first `>>` is split: `<T> > >> >> >> >` ..and then when another `<` is prepended, a right shift is first again: `Vec<<T>> >> >> >> >` In the previous commits, a example that had two `<<` characters was always used and therefore it was incorrectly assumed that `>>` would always be first - but when there is a single `<`, this is not the case.
1 parent 5a0a2df commit 18d300f

File tree

4 files changed

+131
-33
lines changed

4 files changed

+131
-33
lines changed

src/libsyntax/parse/parser.rs

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,7 +2188,27 @@ impl<'a> Parser<'a> {
21882188
enable_warning: bool)
21892189
-> PResult<'a, ()> {
21902190
loop {
2191-
segments.push(self.parse_path_segment(style, enable_warning)?);
2191+
let segment = self.parse_path_segment(style, enable_warning)?;
2192+
if style == PathStyle::Expr {
2193+
// In order to check for trailing angle brackets, we must have finished
2194+
// recursing (`parse_path_segment` can indirectly call this function),
2195+
// that is, the next token must be the highlighted part of the below example:
2196+
//
2197+
// `Foo::<Bar as Baz<T>>::Qux`
2198+
// ^ here
2199+
//
2200+
// As opposed to the below highlight (if we had only finished the first
2201+
// recursion):
2202+
//
2203+
// `Foo::<Bar as Baz<T>>::Qux`
2204+
// ^ here
2205+
//
2206+
// `PathStyle::Expr` is only provided at the root invocation and never in
2207+
// `parse_path_segment` to recurse and therefore can be checked to maintain
2208+
// this invariant.
2209+
self.check_trailing_angle_brackets(&segment, token::ModSep);
2210+
}
2211+
segments.push(segment);
21922212

21932213
if self.is_import_coupler() || !self.eat(&token::ModSep) {
21942214
return Ok(());
@@ -2821,7 +2841,7 @@ impl<'a> Parser<'a> {
28212841
// Assuming we have just parsed `.`, continue parsing into an expression.
28222842
fn parse_dot_suffix(&mut self, self_arg: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
28232843
let segment = self.parse_path_segment(PathStyle::Expr, true)?;
2824-
self.check_trailing_angle_brackets(&segment);
2844+
self.check_trailing_angle_brackets(&segment, token::OpenDelim(token::Paren));
28252845

28262846
Ok(match self.token {
28272847
token::OpenDelim(token::Paren) => {
@@ -2857,15 +2877,19 @@ impl<'a> Parser<'a> {
28572877
/// let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>();
28582878
/// ^^ help: remove extra angle brackets
28592879
/// ```
2860-
fn check_trailing_angle_brackets(&mut self, segment: &PathSegment) {
2861-
// This function is intended to be invoked from `parse_dot_suffix` where there are two
2880+
fn check_trailing_angle_brackets(&mut self, segment: &PathSegment, end: token::Token) {
2881+
// This function is intended to be invoked after parsing a path segment where there are two
28622882
// cases:
28632883
//
2864-
// - A field access (eg. `x.foo`)
2865-
// - A method call (eg. `x.foo()`)
2884+
// 1. A specific token is expected after the path segment.
2885+
// eg. `x.foo(`, `x.foo::<u32>(` (parenthesis - method call),
2886+
// `Foo::`, or `Foo::<Bar>::` (mod sep - continued path).
2887+
// 2. No specific token is expected after the path segment.
2888+
// eg. `x.foo` (field access)
28662889
//
2867-
// This function is called after parsing `.foo` and before parsing any parenthesis (if
2868-
// present). This includes any angle bracket arguments, such as `.foo::<u32>`.
2890+
// This function is called after parsing `.foo` and before parsing the token `end` (if
2891+
// present). This includes any angle bracket arguments, such as `.foo::<u32>` or
2892+
// `Foo::<Bar>`.
28692893

28702894
// We only care about trailing angle brackets if we previously parsed angle bracket
28712895
// arguments. This helps stop us incorrectly suggesting that extra angle brackets be
@@ -2900,43 +2924,47 @@ impl<'a> Parser<'a> {
29002924
// actual operators and not trailing characters - ie `x.foo >> 3`).
29012925
let mut position = 0;
29022926

2903-
// The first tokens we will encounter are shift right tokens (`>>`) since pairs of `>`
2904-
// characters will have been grouped together by the tokenizer.
2927+
// We can encounter `>` or `>>` tokens in any order, so we need to keep track of how
2928+
// many of each (so we can correctly pluralize our error messages) and continue to
2929+
// advance.
29052930
let mut number_of_shr = 0;
2906-
while self.look_ahead(position, |t| *t == token::BinOp(token::BinOpToken::Shr)) {
2907-
number_of_shr += 1;
2908-
position += 1;
2909-
}
2910-
2911-
// Afterwards, there will be at most one `>` character remaining (more than one and it'd
2912-
// have shown up as a `>>`).
2913-
let encountered_gt = self.look_ahead(position, |t| *t == token::Gt);
2914-
if encountered_gt {
2931+
let mut number_of_gt = 0;
2932+
while self.look_ahead(position, |t| {
2933+
trace!("check_trailing_angle_brackets: t={:?}", t);
2934+
if *t == token::BinOp(token::BinOpToken::Shr) {
2935+
number_of_shr += 1;
2936+
true
2937+
} else if *t == token::Gt {
2938+
number_of_gt += 1;
2939+
true
2940+
} else {
2941+
false
2942+
}
2943+
}) {
29152944
position += 1;
29162945
}
29172946

2918-
// If we didn't find any trailing `>>` characters or a trailing `>`, then we have
2919-
// nothing to error about.
2947+
// If we didn't find any trailing `>` characters, then we have nothing to error about.
29202948
debug!(
2921-
"check_trailing_angle_brackets: encountered_gt={:?} number_of_shr={:?}",
2922-
encountered_gt, number_of_shr,
2949+
"check_trailing_angle_brackets: number_of_gt={:?} number_of_shr={:?}",
2950+
number_of_gt, number_of_shr,
29232951
);
2924-
if !encountered_gt && number_of_shr < 1 {
2952+
if number_of_gt < 1 && number_of_shr < 1 {
29252953
return;
29262954
}
29272955

2928-
// Finally, double check that we have a left parenthesis next as otherwise this is the
2929-
// field case.
2930-
if self.look_ahead(position, |t| *t == token::OpenDelim(token::Paren)) {
2931-
// Eat from where we started until the left parenthesis so that parsing can continue
2956+
// Finally, double check that we have our end token as otherwise this is the
2957+
// second case.
2958+
if self.look_ahead(position, |t| {
2959+
trace!("check_trailing_angle_brackets: t={:?}", t);
2960+
*t == end
2961+
}) {
2962+
// Eat from where we started until the end token so that parsing can continue
29322963
// as if we didn't have those extra angle brackets.
2933-
self.eat_to_tokens(&[&token::OpenDelim(token::Paren)]);
2964+
self.eat_to_tokens(&[&end]);
29342965
let span = lo.until(self.span);
29352966

2936-
// We needn't check `encountered_gt` to determine if we should pluralize "bracket".
2937-
// `encountered_gt` can only represent a single `>` character, if `number_of_shr >= 1`
2938-
// then there is either `>>` or `>>>` - in either case a plural is warranted.
2939-
let plural = number_of_shr >= 1;
2967+
let plural = number_of_gt > 1 || number_of_shr >= 1;
29402968
self.diagnostic()
29412969
.struct_span_err(
29422970
span,
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// run-rustfix
2+
3+
// This test checks that the following error is emitted and the suggestion works:
4+
//
5+
// ```
6+
// let _ = Vec::<usize>>>::new();
7+
// ^^ help: remove extra angle brackets
8+
// ```
9+
10+
fn main() {
11+
let _ = Vec::<usize>::new();
12+
//~^ ERROR unmatched angle bracket
13+
14+
let _ = Vec::<usize>::new();
15+
//~^ ERROR unmatched angle bracket
16+
17+
let _ = Vec::<usize>::new();
18+
//~^ ERROR unmatched angle bracket
19+
20+
let _ = Vec::<usize>::new();
21+
//~^ ERROR unmatched angle bracket
22+
}

src/test/ui/issues/issue-54521-2.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// run-rustfix
2+
3+
// This test checks that the following error is emitted and the suggestion works:
4+
//
5+
// ```
6+
// let _ = Vec::<usize>>>::new();
7+
// ^^ help: remove extra angle brackets
8+
// ```
9+
10+
fn main() {
11+
let _ = Vec::<usize>>>>>::new();
12+
//~^ ERROR unmatched angle bracket
13+
14+
let _ = Vec::<usize>>>>::new();
15+
//~^ ERROR unmatched angle bracket
16+
17+
let _ = Vec::<usize>>>::new();
18+
//~^ ERROR unmatched angle bracket
19+
20+
let _ = Vec::<usize>>::new();
21+
//~^ ERROR unmatched angle bracket
22+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: unmatched angle brackets
2+
--> $DIR/issue-54521-2.rs:11:25
3+
|
4+
LL | let _ = Vec::<usize>>>>>::new();
5+
| ^^^^ help: remove extra angle brackets
6+
7+
error: unmatched angle brackets
8+
--> $DIR/issue-54521-2.rs:14:25
9+
|
10+
LL | let _ = Vec::<usize>>>>::new();
11+
| ^^^ help: remove extra angle brackets
12+
13+
error: unmatched angle brackets
14+
--> $DIR/issue-54521-2.rs:17:25
15+
|
16+
LL | let _ = Vec::<usize>>>::new();
17+
| ^^ help: remove extra angle brackets
18+
19+
error: unmatched angle bracket
20+
--> $DIR/issue-54521-2.rs:20:25
21+
|
22+
LL | let _ = Vec::<usize>>::new();
23+
| ^ help: remove extra angle bracket
24+
25+
error: aborting due to 4 previous errors
26+

0 commit comments

Comments
 (0)