From ed60cf2475cabd3d9ad1afdc03bd6952d99b744c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 29 Sep 2019 19:07:26 -0700 Subject: [PATCH 1/8] When encountering chained operators use heuristics to recover from bad turbofish --- src/libsyntax/parse/diagnostics.rs | 100 +++++++++++++++--- src/libsyntax/parse/parser/expr.rs | 4 +- src/test/ui/did_you_mean/issue-40396.stderr | 16 +-- .../require-parens-for-chained-comparison.rs | 5 +- ...quire-parens-for-chained-comparison.stderr | 23 ++-- 5 files changed, 121 insertions(+), 27 deletions(-) diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index e8d7b7663ed52..0e3d873b252e6 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -543,16 +543,25 @@ impl<'a> Parser<'a> { } /// Produces an error if comparison operators are chained (RFC #558). - /// We only need to check the LHS, not the RHS, because all comparison ops - /// have same precedence and are left-associative. - crate fn check_no_chained_comparison(&self, lhs: &Expr, outer_op: &AssocOp) -> PResult<'a, ()> { - debug_assert!(outer_op.is_comparison(), - "check_no_chained_comparison: {:?} is not comparison", - outer_op); + /// We only need to check the LHS, not the RHS, because all comparison ops have same + /// precedence and are left-associative. + /// + /// This can also be hit if someone incorrectly writes `foo()` when they should have used + /// the turbofish syntax. We attempt some heuristic recovery if that is the case. + crate fn check_no_chained_comparison( + &mut self, + lhs: &Expr, + outer_op: &AssocOp, + ) -> PResult<'a, Option>> { + debug_assert!( + outer_op.is_comparison(), + "check_no_chained_comparison: {:?} is not comparison", + outer_op, + ); match lhs.kind { ExprKind::Binary(op, _, _) if op.node.is_comparison() => { // Respan to include both operators. - let op_span = op.span.to(self.token.span); + let op_span = op.span.to(self.prev_span); let mut err = self.struct_span_err( op_span, "chained comparison operators require parentheses", @@ -561,17 +570,84 @@ impl<'a> Parser<'a> { *outer_op == AssocOp::Less || // Include `<` to provide this recommendation *outer_op == AssocOp::Greater // even in a case like the following: { // Foo>> - err.help( - "use `::<...>` instead of `<...>` if you meant to specify type arguments"); - err.help("or use `(...)` if you meant to specify fn arguments"); - // These cases cause too many knock-down errors, bail out (#61329). + let msg = "use `::<...>` instead of `<...>` if you meant to specify type \ + arguments"; + if *outer_op == AssocOp::Less { + // if self.look_ahead(1, |t| t.kind == token::Lt || t.kind == token::ModSep) { + let snapshot = self.clone(); + self.bump(); + // So far we have parsed `foo 0 { + match &self.token.kind { + token::Lt => { + acc += 1; + } + token::Gt => { + acc -= 1; + } + token::BinOp(token::Shr) => { + acc -= 2; + } + token::Eof => { + break; + } + _ => {} + } + self.bump(); + } + if self.token.kind != token::OpenDelim(token::Paren) { + mem::replace(self, snapshot.clone()); + } + } + if self.token.kind == token::OpenDelim(token::Paren) { + err.span_suggestion( + op_span.shrink_to_lo(), + msg, + "::".to_string(), + Applicability::MaybeIncorrect, + ); + let snapshot = self.clone(); + self.bump(); + let mut acc = 1; + while acc > 0 { + match &self.token.kind { + token::OpenDelim(token::Paren) => { + acc += 1; + } + token::CloseDelim(token::Paren) => { + acc -= 1; + } + token::Eof => { + break; + } + _ => {} + } + self.bump(); + } + if self.token.kind == token::Eof { + mem::replace(self, snapshot); + return Err(err); + } else { + err.emit(); + return Ok(Some(self.mk_expr( + lhs.span.to(self.prev_span), + ExprKind::Err, + ThinVec::new(), + ))); + } + } else { + err.help(msg); + err.help("or use `(...)` if you meant to specify fn arguments"); + // These cases cause too many knock-down errors, bail out (#61329). + } return Err(err); } err.emit(); } _ => {} } - Ok(()) + Ok(None) } crate fn maybe_report_ambiguous_plus( diff --git a/src/libsyntax/parse/parser/expr.rs b/src/libsyntax/parse/parser/expr.rs index 23674ad589dc5..b459782d237c7 100644 --- a/src/libsyntax/parse/parser/expr.rs +++ b/src/libsyntax/parse/parser/expr.rs @@ -238,7 +238,9 @@ impl<'a> Parser<'a> { self.bump(); if op.is_comparison() { - self.check_no_chained_comparison(&lhs, &op)?; + if let Some(expr) = self.check_no_chained_comparison(&lhs, &op)? { + return Ok(expr); + } } // Special cases: if op == AssocOp::As { diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr index 7a08fda27e355..d0448cc265c31 100644 --- a/src/test/ui/did_you_mean/issue-40396.stderr +++ b/src/test/ui/did_you_mean/issue-40396.stderr @@ -2,16 +2,17 @@ error: chained comparison operators require parentheses --> $DIR/issue-40396.rs:2:20 | LL | (0..13).collect>(); - | ^^^^^^^^ + | ^^^^^ +help: use `::<...>` instead of `<...>` if you meant to specify type arguments | - = help: use `::<...>` instead of `<...>` if you meant to specify type arguments - = help: or use `(...)` if you meant to specify fn arguments +LL | (0..13).collect::>(); + | ^^ error: chained comparison operators require parentheses --> $DIR/issue-40396.rs:7:8 | LL | Vec::new(); - | ^^^^^^^ + | ^^^^^ | = help: use `::<...>` instead of `<...>` if you meant to specify type arguments = help: or use `(...)` if you meant to specify fn arguments @@ -20,10 +21,11 @@ error: chained comparison operators require parentheses --> $DIR/issue-40396.rs:12:20 | LL | (0..13).collect(); - | ^^^^^^^^ + | ^^^^^ +help: use `::<...>` instead of `<...>` if you meant to specify type arguments | - = help: use `::<...>` instead of `<...>` if you meant to specify type arguments - = help: or use `(...)` if you meant to specify fn arguments +LL | (0..13).collect::(); + | ^^ error: aborting due to 3 previous errors diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.rs b/src/test/ui/parser/require-parens-for-chained-comparison.rs index 3dcc0c8f3d496..11839938cb022 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.rs +++ b/src/test/ui/parser/require-parens-for-chained-comparison.rs @@ -13,5 +13,8 @@ fn main() { f(); //~^ ERROR chained comparison operators require parentheses //~| HELP: use `::<...>` instead of `<...>` - //~| HELP: or use `(...)` + + f, Option>>(1, 2); + //~^ ERROR chained comparison operators require parentheses + //~| HELP: use `::<...>` instead of `<...>` } diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.stderr b/src/test/ui/parser/require-parens-for-chained-comparison.stderr index e927f4c32484e..02fb56a7f9be2 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.stderr +++ b/src/test/ui/parser/require-parens-for-chained-comparison.stderr @@ -2,22 +2,33 @@ error: chained comparison operators require parentheses --> $DIR/require-parens-for-chained-comparison.rs:5:11 | LL | false == false == false; - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^ error: chained comparison operators require parentheses --> $DIR/require-parens-for-chained-comparison.rs:8:11 | LL | false == 0 < 2; - | ^^^^^^^^ + | ^^^^^^ error: chained comparison operators require parentheses --> $DIR/require-parens-for-chained-comparison.rs:13:6 | LL | f(); - | ^^^^ + | ^^^ +help: use `::<...>` instead of `<...>` if you meant to specify type arguments | - = help: use `::<...>` instead of `<...>` if you meant to specify type arguments - = help: or use `(...)` if you meant to specify fn arguments +LL | f::(); + | ^^ + +error: chained comparison operators require parentheses + --> $DIR/require-parens-for-chained-comparison.rs:17:6 + | +LL | f, Option>>(1, 2); + | ^^^^^^^^ +help: use `::<...>` instead of `<...>` if you meant to specify type arguments + | +LL | f::, Option>>(1, 2); + | ^^ error[E0308]: mismatched types --> $DIR/require-parens-for-chained-comparison.rs:8:14 @@ -37,6 +48,6 @@ LL | false == 0 < 2; = note: expected type `bool` found type `{integer}` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0308`. From 6c9f298a8bee9b1716b2e6fcdb8305c3f4874fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 30 Sep 2019 12:19:22 -0700 Subject: [PATCH 2/8] review comments --- src/libsyntax/parse/diagnostics.rs | 87 +++++++++++++++++------------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 0e3d873b252e6..584c7c2ded55b 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -560,6 +560,7 @@ impl<'a> Parser<'a> { ); match lhs.kind { ExprKind::Binary(op, _, _) if op.node.is_comparison() => { + // Respan to include both operators. let op_span = op.span.to(self.prev_span); let mut err = self.struct_span_err( @@ -573,63 +574,54 @@ impl<'a> Parser<'a> { let msg = "use `::<...>` instead of `<...>` if you meant to specify type \ arguments"; if *outer_op == AssocOp::Less { - // if self.look_ahead(1, |t| t.kind == token::Lt || t.kind == token::ModSep) { let snapshot = self.clone(); self.bump(); - // So far we have parsed `foo 0 { - match &self.token.kind { - token::Lt => { - acc += 1; - } - token::Gt => { - acc -= 1; - } - token::BinOp(token::Shr) => { - acc -= 2; - } - token::Eof => { - break; - } - _ => {} - } - self.bump(); - } + // So far we have parsed `foo(`, so we rewind the parser and bail out. mem::replace(self, snapshot.clone()); } } if self.token.kind == token::OpenDelim(token::Paren) { + // We have high certainty that this was a bad turbofish at this point. + // `foo< bar >(` err.span_suggestion( op_span.shrink_to_lo(), msg, "::".to_string(), Applicability::MaybeIncorrect, ); + let snapshot = self.clone(); - self.bump(); - let mut acc = 1; - while acc > 0 { - match &self.token.kind { - token::OpenDelim(token::Paren) => { - acc += 1; - } - token::CloseDelim(token::Paren) => { - acc -= 1; - } - token::Eof => { - break; - } - _ => {} - } - self.bump(); - } + + // Consume the fn call arguments. + let modifiers = vec![ + (token::OpenDelim(token::Paren), 1), + (token::CloseDelim(token::Paren), -1), + ]; + let early_return = vec![token::Eof]; + self.bump(); // `(` + self.consume_tts(1, &modifiers[..], &early_return[..]); + if self.token.kind == token::Eof { + // Not entirely sure now, but we bubble the error up with the + // suggestion. mem::replace(self, snapshot); return Err(err); } else { + // 99% certain that the suggestion is correct, continue parsing. err.emit(); + // FIXME: actually check that the two expressions in the binop are + // paths and resynthesize new fn call expression instead of using + // `ExprKind::Err` placeholder. return Ok(Some(self.mk_expr( lhs.span.to(self.prev_span), ExprKind::Err, @@ -637,6 +629,8 @@ impl<'a> Parser<'a> { ))); } } else { + // All we know is that this is `foo < bar >` and *nothing* else. Try to + // be helpful, but don't attempt to recover. err.help(msg); err.help("or use `(...)` if you meant to specify fn arguments"); // These cases cause too many knock-down errors, bail out (#61329). @@ -1424,6 +1418,23 @@ impl<'a> Parser<'a> { err } + fn consume_tts( + &mut self, + mut acc: i64, + modifier: &[(token::TokenKind, i64)], // Not using FxHasMap and FxHashSet due to + early_return: &[token::TokenKind], // `token::TokenKind: !Eq + !Hash`. + ) { + while acc > 0 { + if let Some((_, val)) = modifier.iter().filter(|(t, _)| *t == self.token.kind).next() { + acc += *val; + } + if early_return.contains(&self.token.kind) { + break; + } + self.bump(); + } + } + /// Replace duplicated recovered parameters with `_` pattern to avoid unecessary errors. /// /// This is necessary because at this point we don't know whether we parsed a function with From d7dceaa0c57759d37a48b5a0aa1064b7c89f957b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 30 Sep 2019 12:36:44 -0700 Subject: [PATCH 3/8] Account for missing turbofish in paths too --- src/libsyntax/parse/diagnostics.rs | 47 +++++++++++++++++++-- src/test/ui/did_you_mean/issue-40396.stderr | 5 ++- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 584c7c2ded55b..4fbd36cfefc82 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -585,12 +585,51 @@ impl<'a> Parser<'a> { let early_return = vec![token::Eof]; self.consume_tts(1, &modifiers[..], &early_return[..]); - if self.token.kind != token::OpenDelim(token::Paren) { - // We don't have `foo< bar >(`, so we rewind the parser and bail out. + if !&[ + token::OpenDelim(token::Paren), + token::ModSep, + ].contains(&self.token.kind) { + // We don't have `foo< bar >(` or `foo< bar >::`, so we rewind the + // parser and bail out. mem::replace(self, snapshot.clone()); } } - if self.token.kind == token::OpenDelim(token::Paren) { + if token::ModSep == self.token.kind { + // We have some certainty that this was a bad turbofish at this point. + // `foo< bar >::` + err.span_suggestion( + op_span.shrink_to_lo(), + msg, + "::".to_string(), + Applicability::MaybeIncorrect, + ); + + let snapshot = self.clone(); + + self.bump(); // `::` + // Consume the rest of the likely `foo::new()` or return at `foo`. + match self.parse_expr() { + Ok(_) => { + // 99% certain that the suggestion is correct, continue parsing. + err.emit(); + // FIXME: actually check that the two expressions in the binop are + // paths and resynthesize new fn call expression instead of using + // `ExprKind::Err` placeholder. + return Ok(Some(self.mk_expr( + lhs.span.to(self.prev_span), + ExprKind::Err, + ThinVec::new(), + ))); + } + Err(mut err) => { + err.cancel(); + // Not entirely sure now, but we bubble the error up with the + // suggestion. + mem::replace(self, snapshot); + return Err(err); + } + } + } else if token::OpenDelim(token::Paren) == self.token.kind { // We have high certainty that this was a bad turbofish at this point. // `foo< bar >(` err.span_suggestion( @@ -601,6 +640,7 @@ impl<'a> Parser<'a> { ); let snapshot = self.clone(); + self.bump(); // `(` // Consume the fn call arguments. let modifiers = vec![ @@ -608,7 +648,6 @@ impl<'a> Parser<'a> { (token::CloseDelim(token::Paren), -1), ]; let early_return = vec![token::Eof]; - self.bump(); // `(` self.consume_tts(1, &modifiers[..], &early_return[..]); if self.token.kind == token::Eof { diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr index d0448cc265c31..5e3771002b685 100644 --- a/src/test/ui/did_you_mean/issue-40396.stderr +++ b/src/test/ui/did_you_mean/issue-40396.stderr @@ -13,9 +13,10 @@ error: chained comparison operators require parentheses | LL | Vec::new(); | ^^^^^ +help: use `::<...>` instead of `<...>` if you meant to specify type arguments | - = help: use `::<...>` instead of `<...>` if you meant to specify type arguments - = help: or use `(...)` if you meant to specify fn arguments +LL | Vec::::new(); + | ^^ error: chained comparison operators require parentheses --> $DIR/issue-40396.rs:12:20 From d27683a39f5924205d73e0015b8c25c97aadaa63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 30 Sep 2019 13:22:58 -0700 Subject: [PATCH 4/8] Prove bad turbofish parser recovery in test --- src/test/ui/did_you_mean/issue-40396.rs | 10 +--------- src/test/ui/did_you_mean/issue-40396.stderr | 4 ++-- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/src/test/ui/did_you_mean/issue-40396.rs b/src/test/ui/did_you_mean/issue-40396.rs index 6902779f33d23..1893355205433 100644 --- a/src/test/ui/did_you_mean/issue-40396.rs +++ b/src/test/ui/did_you_mean/issue-40396.rs @@ -1,16 +1,8 @@ -fn foo() { +fn main() { (0..13).collect>(); //~^ ERROR chained comparison -} - -fn bar() { Vec::new(); //~^ ERROR chained comparison -} - -fn qux() { (0..13).collect(); //~^ ERROR chained comparison } - -fn main() {} diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr index 5e3771002b685..dd19bc137e30d 100644 --- a/src/test/ui/did_you_mean/issue-40396.stderr +++ b/src/test/ui/did_you_mean/issue-40396.stderr @@ -9,7 +9,7 @@ LL | (0..13).collect::>(); | ^^ error: chained comparison operators require parentheses - --> $DIR/issue-40396.rs:7:8 + --> $DIR/issue-40396.rs:4:8 | LL | Vec::new(); | ^^^^^ @@ -19,7 +19,7 @@ LL | Vec::::new(); | ^^ error: chained comparison operators require parentheses - --> $DIR/issue-40396.rs:12:20 + --> $DIR/issue-40396.rs:6:20 | LL | (0..13).collect(); | ^^^^^ From dfdc369b40da72eb9ff466fab89584c7815d7a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 1 Oct 2019 11:24:05 -0700 Subject: [PATCH 5/8] review comments --- src/libsyntax/parse/diagnostics.rs | 79 ++++++++++--------- src/test/ui/did_you_mean/issue-40396.stderr | 6 +- .../require-parens-for-chained-comparison.rs | 18 +++-- ...quire-parens-for-chained-comparison.stderr | 15 +++- 4 files changed, 70 insertions(+), 48 deletions(-) diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 4fbd36cfefc82..14c5dc6132cf8 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -17,6 +17,8 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan, SpanSnippetError}; use log::{debug, trace}; use std::mem; +const TURBOFISH: &'static str = "use the \"turbofish\" `::<...>` instead of `<...>` to specify \ + type arguments"; /// Creates a placeholder argument. crate fn dummy_arg(ident: Ident) -> Param { let pat = P(Pat { @@ -544,10 +546,20 @@ impl<'a> Parser<'a> { /// Produces an error if comparison operators are chained (RFC #558). /// We only need to check the LHS, not the RHS, because all comparison ops have same - /// precedence and are left-associative. + /// precedence (see `fn precedence`) and are left-associative (see `fn fixity`). /// /// This can also be hit if someone incorrectly writes `foo()` when they should have used - /// the turbofish syntax. We attempt some heuristic recovery if that is the case. + /// the turbofish (`foo::()`) syntax. We attempt some heuristic recovery if that is the + /// case. + /// + /// Keep in mind that given that `outer_op.is_comparison()` holds and comparison ops are left + /// associative we can infer that we have: + /// + /// outer_op + /// / \ + /// inner_op r2 + /// / \ + /// l1 r1 crate fn check_no_chained_comparison( &mut self, lhs: &Expr, @@ -560,30 +572,36 @@ impl<'a> Parser<'a> { ); match lhs.kind { ExprKind::Binary(op, _, _) if op.node.is_comparison() => { - // Respan to include both operators. let op_span = op.span.to(self.prev_span); let mut err = self.struct_span_err( op_span, "chained comparison operators require parentheses", ); + + let suggest = |err: &mut DiagnosticBuilder<'_>| { + err.span_suggestion( + op_span.shrink_to_lo(), + TURBOFISH, + "::".to_string(), + Applicability::MaybeIncorrect, + ); + }; + if op.node == BinOpKind::Lt && *outer_op == AssocOp::Less || // Include `<` to provide this recommendation *outer_op == AssocOp::Greater // even in a case like the following: { // Foo>> - let msg = "use `::<...>` instead of `<...>` if you meant to specify type \ - arguments"; if *outer_op == AssocOp::Less { let snapshot = self.clone(); self.bump(); - // So far we have parsed `foo Parser<'a> { if token::ModSep == self.token.kind { // We have some certainty that this was a bad turbofish at this point. // `foo< bar >::` - err.span_suggestion( - op_span.shrink_to_lo(), - msg, - "::".to_string(), - Applicability::MaybeIncorrect, - ); + suggest(&mut err); let snapshot = self.clone(); - self.bump(); // `::` + // Consume the rest of the likely `foo::new()` or return at `foo`. match self.parse_expr() { Ok(_) => { @@ -621,8 +634,8 @@ impl<'a> Parser<'a> { ThinVec::new(), ))); } - Err(mut err) => { - err.cancel(); + Err(mut expr_err) => { + expr_err.cancel(); // Not entirely sure now, but we bubble the error up with the // suggestion. mem::replace(self, snapshot); @@ -632,45 +645,39 @@ impl<'a> Parser<'a> { } else if token::OpenDelim(token::Paren) == self.token.kind { // We have high certainty that this was a bad turbofish at this point. // `foo< bar >(` - err.span_suggestion( - op_span.shrink_to_lo(), - msg, - "::".to_string(), - Applicability::MaybeIncorrect, - ); + suggest(&mut err); let snapshot = self.clone(); self.bump(); // `(` // Consume the fn call arguments. - let modifiers = vec![ + let modifiers = [ (token::OpenDelim(token::Paren), 1), (token::CloseDelim(token::Paren), -1), ]; - let early_return = vec![token::Eof]; - self.consume_tts(1, &modifiers[..], &early_return[..]); + self.consume_tts(1, &modifiers[..], &[]); - if self.token.kind == token::Eof { + return if self.token.kind == token::Eof { // Not entirely sure now, but we bubble the error up with the // suggestion. mem::replace(self, snapshot); - return Err(err); + Err(err) } else { // 99% certain that the suggestion is correct, continue parsing. err.emit(); // FIXME: actually check that the two expressions in the binop are // paths and resynthesize new fn call expression instead of using // `ExprKind::Err` placeholder. - return Ok(Some(self.mk_expr( + Ok(Some(self.mk_expr( lhs.span.to(self.prev_span), ExprKind::Err, ThinVec::new(), - ))); + ))) } } else { // All we know is that this is `foo < bar >` and *nothing* else. Try to // be helpful, but don't attempt to recover. - err.help(msg); + err.help(TURBOFISH); err.help("or use `(...)` if you meant to specify fn arguments"); // These cases cause too many knock-down errors, bail out (#61329). } @@ -1459,15 +1466,15 @@ impl<'a> Parser<'a> { fn consume_tts( &mut self, - mut acc: i64, - modifier: &[(token::TokenKind, i64)], // Not using FxHasMap and FxHashSet due to + mut acc: i64, // `i64` because malformed code can have more closing delims than opening. + modifier: &[(token::TokenKind, i64)], // Not using `FxHashMap` and `FxHashSet` due to early_return: &[token::TokenKind], // `token::TokenKind: !Eq + !Hash`. ) { while acc > 0 { - if let Some((_, val)) = modifier.iter().filter(|(t, _)| *t == self.token.kind).next() { + if let Some((_, val)) = modifier.iter().find(|(t, _)| *t == self.token.kind) { acc += *val; } - if early_return.contains(&self.token.kind) { + if self.token.kind == token::Eof || early_return.contains(&self.token.kind) { break; } self.bump(); diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr index dd19bc137e30d..9757f8258c131 100644 --- a/src/test/ui/did_you_mean/issue-40396.stderr +++ b/src/test/ui/did_you_mean/issue-40396.stderr @@ -3,7 +3,7 @@ error: chained comparison operators require parentheses | LL | (0..13).collect>(); | ^^^^^ -help: use `::<...>` instead of `<...>` if you meant to specify type arguments +help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments | LL | (0..13).collect::>(); | ^^ @@ -13,7 +13,7 @@ error: chained comparison operators require parentheses | LL | Vec::new(); | ^^^^^ -help: use `::<...>` instead of `<...>` if you meant to specify type arguments +help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments | LL | Vec::::new(); | ^^ @@ -23,7 +23,7 @@ error: chained comparison operators require parentheses | LL | (0..13).collect(); | ^^^^^ -help: use `::<...>` instead of `<...>` if you meant to specify type arguments +help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments | LL | (0..13).collect::(); | ^^ diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.rs b/src/test/ui/parser/require-parens-for-chained-comparison.rs index 11839938cb022..f3bfe2d482f59 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.rs +++ b/src/test/ui/parser/require-parens-for-chained-comparison.rs @@ -3,18 +3,24 @@ struct X; fn main() { false == false == false; - //~^ ERROR: chained comparison operators require parentheses + //~^ ERROR chained comparison operators require parentheses false == 0 < 2; - //~^ ERROR: chained comparison operators require parentheses - //~| ERROR: mismatched types - //~| ERROR: mismatched types + //~^ ERROR chained comparison operators require parentheses + //~| ERROR mismatched types + //~| ERROR mismatched types f(); //~^ ERROR chained comparison operators require parentheses - //~| HELP: use `::<...>` instead of `<...>` + //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments f, Option>>(1, 2); //~^ ERROR chained comparison operators require parentheses - //~| HELP: use `::<...>` instead of `<...>` + //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + + use std::convert::identity; + let _ = identity; + //~^ ERROR chained comparison operators require parentheses + //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + //~| HELP or use `(...)` if you meant to specify fn arguments } diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.stderr b/src/test/ui/parser/require-parens-for-chained-comparison.stderr index 02fb56a7f9be2..4b108e1db87b7 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.stderr +++ b/src/test/ui/parser/require-parens-for-chained-comparison.stderr @@ -15,7 +15,7 @@ error: chained comparison operators require parentheses | LL | f(); | ^^^ -help: use `::<...>` instead of `<...>` if you meant to specify type arguments +help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments | LL | f::(); | ^^ @@ -25,11 +25,20 @@ error: chained comparison operators require parentheses | LL | f, Option>>(1, 2); | ^^^^^^^^ -help: use `::<...>` instead of `<...>` if you meant to specify type arguments +help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments | LL | f::, Option>>(1, 2); | ^^ +error: chained comparison operators require parentheses + --> $DIR/require-parens-for-chained-comparison.rs:22:21 + | +LL | let _ = identity; + | ^^^^ + | + = help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + = help: or use `(...)` if you meant to specify fn arguments + error[E0308]: mismatched types --> $DIR/require-parens-for-chained-comparison.rs:8:14 | @@ -48,6 +57,6 @@ LL | false == 0 < 2; = note: expected type `bool` found type `{integer}` -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0308`. From f1499a864688a484c04c4e53962dc8ec44f79a03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 1 Oct 2019 15:51:50 -0700 Subject: [PATCH 6/8] review comments --- src/libsyntax/parse/diagnostics.rs | 37 ++++++++++++++---------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 14c5dc6132cf8..72206ffb28df0 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -570,6 +570,11 @@ impl<'a> Parser<'a> { "check_no_chained_comparison: {:?} is not comparison", outer_op, ); + + let mk_err_expr = |this: &Self, span| { + Ok(Some(this.mk_expr(span, ExprKind::Err, ThinVec::new()))) + }; + match lhs.kind { ExprKind::Binary(op, _, _) if op.node.is_comparison() => { // Respan to include both operators. @@ -601,7 +606,7 @@ impl<'a> Parser<'a> { (token::Gt, -1), (token::BinOp(token::Shr), -2), ]; - self.consume_tts(1, &modifiers[..], &[]); + self.consume_tts(1, &modifiers[..]); if !&[ token::OpenDelim(token::Paren), @@ -612,7 +617,7 @@ impl<'a> Parser<'a> { mem::replace(self, snapshot.clone()); } } - if token::ModSep == self.token.kind { + return if token::ModSep == self.token.kind { // We have some certainty that this was a bad turbofish at this point. // `foo< bar >::` suggest(&mut err); @@ -628,18 +633,14 @@ impl<'a> Parser<'a> { // FIXME: actually check that the two expressions in the binop are // paths and resynthesize new fn call expression instead of using // `ExprKind::Err` placeholder. - return Ok(Some(self.mk_expr( - lhs.span.to(self.prev_span), - ExprKind::Err, - ThinVec::new(), - ))); + mk_err_expr(self, lhs.span.to(self.prev_span)) } Err(mut expr_err) => { expr_err.cancel(); // Not entirely sure now, but we bubble the error up with the // suggestion. mem::replace(self, snapshot); - return Err(err); + Err(err) } } } else if token::OpenDelim(token::Paren) == self.token.kind { @@ -655,9 +656,9 @@ impl<'a> Parser<'a> { (token::OpenDelim(token::Paren), 1), (token::CloseDelim(token::Paren), -1), ]; - self.consume_tts(1, &modifiers[..], &[]); + self.consume_tts(1, &modifiers[..]); - return if self.token.kind == token::Eof { + if self.token.kind == token::Eof { // Not entirely sure now, but we bubble the error up with the // suggestion. mem::replace(self, snapshot); @@ -668,11 +669,7 @@ impl<'a> Parser<'a> { // FIXME: actually check that the two expressions in the binop are // paths and resynthesize new fn call expression instead of using // `ExprKind::Err` placeholder. - Ok(Some(self.mk_expr( - lhs.span.to(self.prev_span), - ExprKind::Err, - ThinVec::new(), - ))) + mk_err_expr(self, lhs.span.to(self.prev_span)) } } else { // All we know is that this is `foo < bar >` and *nothing* else. Try to @@ -680,8 +677,8 @@ impl<'a> Parser<'a> { err.help(TURBOFISH); err.help("or use `(...)` if you meant to specify fn arguments"); // These cases cause too many knock-down errors, bail out (#61329). - } - return Err(err); + Err(err) + }; } err.emit(); } @@ -1467,14 +1464,14 @@ impl<'a> Parser<'a> { fn consume_tts( &mut self, mut acc: i64, // `i64` because malformed code can have more closing delims than opening. - modifier: &[(token::TokenKind, i64)], // Not using `FxHashMap` and `FxHashSet` due to - early_return: &[token::TokenKind], // `token::TokenKind: !Eq + !Hash`. + // Not using `FxHashMap` due to `token::TokenKind: !Eq + !Hash`. + modifier: &[(token::TokenKind, i64)], ) { while acc > 0 { if let Some((_, val)) = modifier.iter().find(|(t, _)| *t == self.token.kind) { acc += *val; } - if self.token.kind == token::Eof || early_return.contains(&self.token.kind) { + if self.token.kind == token::Eof { break; } self.bump(); From 02f57f83a9ea5903cb02bdc304800661c8f4296f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 3 Oct 2019 13:22:18 -0700 Subject: [PATCH 7/8] review comments --- src/librustc_errors/diagnostic.rs | 41 +++++++++++-- src/librustc_errors/emitter.rs | 4 +- src/librustc_errors/lib.rs | 2 + src/libsyntax/parse/diagnostics.rs | 57 +++++++++++-------- src/test/ui/did_you_mean/issue-40396.stderr | 6 +- .../require-parens-for-chained-comparison.rs | 6 +- ...quire-parens-for-chained-comparison.stderr | 6 +- 7 files changed, 81 insertions(+), 41 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 3f1b91256c468..811a48a39f0df 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -298,9 +298,13 @@ impl Diagnostic { /// * may contain a name of a function, variable, or type, but not whole expressions /// /// See `CodeSuggestion` for more information. - pub fn span_suggestion(&mut self, sp: Span, msg: &str, - suggestion: String, - applicability: Applicability) -> &mut Self { + pub fn span_suggestion( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + ) -> &mut Self { self.suggestions.push(CodeSuggestion { substitutions: vec![Substitution { parts: vec![SubstitutionPart { @@ -315,10 +319,35 @@ impl Diagnostic { self } + pub fn span_suggestion_verbose( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: vec![SubstitutionPart { + snippet: suggestion, + span: sp, + }], + }], + msg: msg.to_owned(), + style: SuggestionStyle::ShowAlways, + applicability, + }); + self + } + /// Prints out a message with multiple suggested edits of the code. - pub fn span_suggestions(&mut self, sp: Span, msg: &str, - suggestions: impl Iterator, applicability: Applicability) -> &mut Self - { + pub fn span_suggestions( + &mut self, + sp: Span, + msg: &str, + suggestions: impl Iterator, + applicability: Applicability, + ) -> &mut Self { self.suggestions.push(CodeSuggestion { substitutions: suggestions.map(|snippet| Substitution { parts: vec![SubstitutionPart { diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 0c7aa3582ac23..bd8191065eeca 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -221,7 +221,9 @@ pub trait Emitter { // when this style is set we want the suggestion to be a message, not inline sugg.style != SuggestionStyle::HideCodeAlways && // trivial suggestion for tooling's sake, never shown - sugg.style != SuggestionStyle::CompletelyHidden + sugg.style != SuggestionStyle::CompletelyHidden && + // subtle suggestion, never shown inline + sugg.style != SuggestionStyle::ShowAlways { let substitution = &sugg.substitutions[0].parts[0].snippet.trim(); let msg = if substitution.len() == 0 || sugg.style.hide_inline() { diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index f9dc13ce97eea..2fae584c15362 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -81,6 +81,8 @@ pub enum SuggestionStyle { /// This will *not* show the code if the suggestion is inline *and* the suggested code is /// empty. ShowCode, + /// Always show the suggested code independently. + ShowAlways, } impl SuggestionStyle { diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 72206ffb28df0..e3abf8ffc6c8f 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -17,8 +17,7 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan, SpanSnippetError}; use log::{debug, trace}; use std::mem; -const TURBOFISH: &'static str = "use the \"turbofish\" `::<...>` instead of `<...>` to specify \ - type arguments"; +const TURBOFISH: &'static str = "use `::<...>` instead of `<...>` to specify type arguments"; /// Creates a placeholder argument. crate fn dummy_arg(ident: Ident) -> Param { let pat = P(Pat { @@ -585,7 +584,7 @@ impl<'a> Parser<'a> { ); let suggest = |err: &mut DiagnosticBuilder<'_>| { - err.span_suggestion( + err.span_suggestion_verbose( op_span.shrink_to_lo(), TURBOFISH, "::".to_string(), @@ -647,29 +646,16 @@ impl<'a> Parser<'a> { // We have high certainty that this was a bad turbofish at this point. // `foo< bar >(` suggest(&mut err); - - let snapshot = self.clone(); - self.bump(); // `(` - // Consume the fn call arguments. - let modifiers = [ - (token::OpenDelim(token::Paren), 1), - (token::CloseDelim(token::Paren), -1), - ]; - self.consume_tts(1, &modifiers[..]); - - if self.token.kind == token::Eof { - // Not entirely sure now, but we bubble the error up with the - // suggestion. - mem::replace(self, snapshot); - Err(err) - } else { - // 99% certain that the suggestion is correct, continue parsing. - err.emit(); - // FIXME: actually check that the two expressions in the binop are - // paths and resynthesize new fn call expression instead of using - // `ExprKind::Err` placeholder. - mk_err_expr(self, lhs.span.to(self.prev_span)) + match self.consume_fn_args() { + Err(()) => Err(err), + Ok(()) => { + err.emit(); + // FIXME: actually check that the two expressions in the binop are + // paths and resynthesize new fn call expression instead of using + // `ExprKind::Err` placeholder. + mk_err_expr(self, lhs.span.to(self.prev_span)) + } } } else { // All we know is that this is `foo < bar >` and *nothing* else. Try to @@ -687,6 +673,27 @@ impl<'a> Parser<'a> { Ok(None) } + fn consume_fn_args(&mut self) -> Result<(), ()> { + let snapshot = self.clone(); + self.bump(); // `(` + + // Consume the fn call arguments. + let modifiers = [ + (token::OpenDelim(token::Paren), 1), + (token::CloseDelim(token::Paren), -1), + ]; + self.consume_tts(1, &modifiers[..]); + + if self.token.kind == token::Eof { + // Not entirely sure that what we consumed were fn arguments, rollback. + mem::replace(self, snapshot); + Err(()) + } else { + // 99% certain that the suggestion is correct, continue parsing. + Ok(()) + } + } + crate fn maybe_report_ambiguous_plus( &mut self, allow_plus: bool, diff --git a/src/test/ui/did_you_mean/issue-40396.stderr b/src/test/ui/did_you_mean/issue-40396.stderr index 9757f8258c131..7fc7c2628c472 100644 --- a/src/test/ui/did_you_mean/issue-40396.stderr +++ b/src/test/ui/did_you_mean/issue-40396.stderr @@ -3,7 +3,7 @@ error: chained comparison operators require parentheses | LL | (0..13).collect>(); | ^^^^^ -help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments +help: use `::<...>` instead of `<...>` to specify type arguments | LL | (0..13).collect::>(); | ^^ @@ -13,7 +13,7 @@ error: chained comparison operators require parentheses | LL | Vec::new(); | ^^^^^ -help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments +help: use `::<...>` instead of `<...>` to specify type arguments | LL | Vec::::new(); | ^^ @@ -23,7 +23,7 @@ error: chained comparison operators require parentheses | LL | (0..13).collect(); | ^^^^^ -help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments +help: use `::<...>` instead of `<...>` to specify type arguments | LL | (0..13).collect::(); | ^^ diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.rs b/src/test/ui/parser/require-parens-for-chained-comparison.rs index f3bfe2d482f59..9c7a25d589a1f 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.rs +++ b/src/test/ui/parser/require-parens-for-chained-comparison.rs @@ -12,15 +12,15 @@ fn main() { f(); //~^ ERROR chained comparison operators require parentheses - //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + //~| HELP use `::<...>` instead of `<...>` to specify type arguments f, Option>>(1, 2); //~^ ERROR chained comparison operators require parentheses - //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + //~| HELP use `::<...>` instead of `<...>` to specify type arguments use std::convert::identity; let _ = identity; //~^ ERROR chained comparison operators require parentheses - //~| HELP use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + //~| HELP use `::<...>` instead of `<...>` to specify type arguments //~| HELP or use `(...)` if you meant to specify fn arguments } diff --git a/src/test/ui/parser/require-parens-for-chained-comparison.stderr b/src/test/ui/parser/require-parens-for-chained-comparison.stderr index 4b108e1db87b7..5aa37a40cbd3d 100644 --- a/src/test/ui/parser/require-parens-for-chained-comparison.stderr +++ b/src/test/ui/parser/require-parens-for-chained-comparison.stderr @@ -15,7 +15,7 @@ error: chained comparison operators require parentheses | LL | f(); | ^^^ -help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments +help: use `::<...>` instead of `<...>` to specify type arguments | LL | f::(); | ^^ @@ -25,7 +25,7 @@ error: chained comparison operators require parentheses | LL | f, Option>>(1, 2); | ^^^^^^^^ -help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments +help: use `::<...>` instead of `<...>` to specify type arguments | LL | f::, Option>>(1, 2); | ^^ @@ -36,7 +36,7 @@ error: chained comparison operators require parentheses LL | let _ = identity; | ^^^^ | - = help: use the "turbofish" `::<...>` instead of `<...>` to specify type arguments + = help: use `::<...>` instead of `<...>` to specify type arguments = help: or use `(...)` if you meant to specify fn arguments error[E0308]: mismatched types From 76456e74066d7594f23757ebade169c33276ea4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 3 Oct 2019 19:32:56 -0700 Subject: [PATCH 8/8] review comments --- src/librustc_errors/diagnostic.rs | 84 ++++++++++++++++--------------- src/librustc_errors/emitter.rs | 14 +++--- 2 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 811a48a39f0df..fd74d8673da4d 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -304,6 +304,24 @@ impl Diagnostic { msg: &str, suggestion: String, applicability: Applicability, + ) -> &mut Self { + self.span_suggestion_with_style( + sp, + msg, + suggestion, + applicability, + SuggestionStyle::ShowCode, + ); + self + } + + pub fn span_suggestion_with_style( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + style: SuggestionStyle, ) -> &mut Self { self.suggestions.push(CodeSuggestion { substitutions: vec![Substitution { @@ -313,7 +331,7 @@ impl Diagnostic { }], }], msg: msg.to_owned(), - style: SuggestionStyle::ShowCode, + style, applicability, }); self @@ -326,17 +344,13 @@ impl Diagnostic { suggestion: String, applicability: Applicability, ) -> &mut Self { - self.suggestions.push(CodeSuggestion { - substitutions: vec![Substitution { - parts: vec![SubstitutionPart { - snippet: suggestion, - span: sp, - }], - }], - msg: msg.to_owned(), - style: SuggestionStyle::ShowAlways, + self.span_suggestion_with_style( + sp, + msg, + suggestion, applicability, - }); + SuggestionStyle::ShowAlways, + ); self } @@ -369,17 +383,13 @@ impl Diagnostic { pub fn span_suggestion_short( &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability ) -> &mut Self { - self.suggestions.push(CodeSuggestion { - substitutions: vec![Substitution { - parts: vec![SubstitutionPart { - snippet: suggestion, - span: sp, - }], - }], - msg: msg.to_owned(), - style: SuggestionStyle::HideCodeInline, + self.span_suggestion_with_style( + sp, + msg, + suggestion, applicability, - }); + SuggestionStyle::HideCodeInline, + ); self } @@ -392,17 +402,13 @@ impl Diagnostic { pub fn span_suggestion_hidden( &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability ) -> &mut Self { - self.suggestions.push(CodeSuggestion { - substitutions: vec![Substitution { - parts: vec![SubstitutionPart { - snippet: suggestion, - span: sp, - }], - }], - msg: msg.to_owned(), - style: SuggestionStyle::HideCodeAlways, + self.span_suggestion_with_style( + sp, + msg, + suggestion, applicability, - }); + SuggestionStyle::HideCodeAlways, + ); self } @@ -413,17 +419,13 @@ impl Diagnostic { pub fn tool_only_span_suggestion( &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability ) -> &mut Self { - self.suggestions.push(CodeSuggestion { - substitutions: vec![Substitution { - parts: vec![SubstitutionPart { - snippet: suggestion, - span: sp, - }], - }], - msg: msg.to_owned(), - style: SuggestionStyle::CompletelyHidden, + self.span_suggestion_with_style( + sp, + msg, + suggestion, applicability, - }); + SuggestionStyle::CompletelyHidden, + ); self } diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index bd8191065eeca..54b0353d992fb 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -218,12 +218,14 @@ pub trait Emitter { sugg.msg.split_whitespace().count() < 10 && // don't display multiline suggestions as labels !sugg.substitutions[0].parts[0].snippet.contains('\n') && - // when this style is set we want the suggestion to be a message, not inline - sugg.style != SuggestionStyle::HideCodeAlways && - // trivial suggestion for tooling's sake, never shown - sugg.style != SuggestionStyle::CompletelyHidden && - // subtle suggestion, never shown inline - sugg.style != SuggestionStyle::ShowAlways + ![ + // when this style is set we want the suggestion to be a message, not inline + SuggestionStyle::HideCodeAlways, + // trivial suggestion for tooling's sake, never shown + SuggestionStyle::CompletelyHidden, + // subtle suggestion, never shown inline + SuggestionStyle::ShowAlways, + ].contains(&sugg.style) { let substitution = &sugg.substitutions[0].parts[0].snippet.trim(); let msg = if substitution.len() == 0 || sugg.style.hide_inline() {