diff options
Diffstat (limited to 'src/librustc_parse/parser')
| -rw-r--r-- | src/librustc_parse/parser/diagnostics.rs | 83 | ||||
| -rw-r--r-- | src/librustc_parse/parser/expr.rs | 42 |
2 files changed, 93 insertions, 32 deletions
diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs index a5056c1665e..6a18c63017e 100644 --- a/src/librustc_parse/parser/diagnostics.rs +++ b/src/librustc_parse/parser/diagnostics.rs @@ -4,6 +4,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_error_codes::*; use rustc_errors::{pluralize, struct_span_err}; use rustc_errors::{Applicability, DiagnosticBuilder, Handler, PResult}; +use rustc_span::source_map::Spanned; use rustc_span::symbol::kw; use rustc_span::{MultiSpan, Span, SpanSnippetError, DUMMY_SP}; use syntax::ast::{ @@ -491,6 +492,58 @@ impl<'a> Parser<'a> { } } + /// Check to see if a pair of chained operators looks like an attempt at chained comparison, + /// e.g. `1 < x <= 3`. If so, suggest either splitting the comparison into two, or + /// parenthesising the leftmost comparison. + fn attempt_chained_comparison_suggestion( + &mut self, + err: &mut DiagnosticBuilder<'_>, + inner_op: &Expr, + outer_op: &Spanned<AssocOp>, + ) { + if let ExprKind::Binary(op, ref l1, ref r1) = inner_op.kind { + match (op.node, &outer_op.node) { + // `x < y < z` and friends. + (BinOpKind::Lt, AssocOp::Less) | (BinOpKind::Lt, AssocOp::LessEqual) | + (BinOpKind::Le, AssocOp::LessEqual) | (BinOpKind::Le, AssocOp::Less) | + // `x > y > z` and friends. + (BinOpKind::Gt, AssocOp::Greater) | (BinOpKind::Gt, AssocOp::GreaterEqual) | + (BinOpKind::Ge, AssocOp::GreaterEqual) | (BinOpKind::Ge, AssocOp::Greater) => { + let expr_to_str = |e: &Expr| { + self.span_to_snippet(e.span) + .unwrap_or_else(|_| pprust::expr_to_string(&e)) + }; + err.span_suggestion( + inner_op.span.to(outer_op.span), + "split the comparison into two...", + format!( + "{} {} {} && {} {}", + expr_to_str(&l1), + op.node.to_string(), + expr_to_str(&r1), + expr_to_str(&r1), + outer_op.node.to_ast_binop().unwrap().to_string(), + ), + Applicability::MaybeIncorrect, + ); + err.span_suggestion( + inner_op.span.to(outer_op.span), + "...or parenthesize one of the comparisons", + format!( + "({} {} {}) {}", + expr_to_str(&l1), + op.node.to_string(), + expr_to_str(&r1), + outer_op.node.to_ast_binop().unwrap().to_string(), + ), + Applicability::MaybeIncorrect, + ); + } + _ => {} + } + } + } + /// 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 (see `fn precedence`) and are left-associative (see `fn fixity`). @@ -506,27 +559,31 @@ impl<'a> Parser<'a> { /// / \ /// inner_op r2 /// / \ - /// l1 r1 + /// l1 r1 pub(super) fn check_no_chained_comparison( &mut self, - lhs: &Expr, - outer_op: &AssocOp, + inner_op: &Expr, + outer_op: &Spanned<AssocOp>, ) -> PResult<'a, Option<P<Expr>>> { debug_assert!( - outer_op.is_comparison(), + outer_op.node.is_comparison(), "check_no_chained_comparison: {:?} is not comparison", - outer_op, + outer_op.node, ); let mk_err_expr = |this: &Self, span| Ok(Some(this.mk_expr(span, ExprKind::Err, AttrVec::new()))); - match lhs.kind { + match inner_op.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 mut err = + self.struct_span_err(op_span, "comparison operators cannot be chained"); + + // If it looks like a genuine attempt to chain operators (as opposed to a + // misformatted turbofish, for instance), suggest a correct form. + self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op); let suggest = |err: &mut DiagnosticBuilder<'_>| { err.span_suggestion_verbose( @@ -538,12 +595,12 @@ impl<'a> Parser<'a> { }; if op.node == BinOpKind::Lt && - *outer_op == AssocOp::Less || // Include `<` to provide this recommendation - *outer_op == AssocOp::Greater + outer_op.node == AssocOp::Less || // Include `<` to provide this recommendation + outer_op.node == AssocOp::Greater // even in a case like the following: { // Foo<Bar<Baz<Qux, ()>>> - if *outer_op == AssocOp::Less { + if outer_op.node == AssocOp::Less { let snapshot = self.clone(); self.bump(); // So far we have parsed `foo<bar<`, consume the rest of the type args. @@ -575,7 +632,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. - mk_err_expr(self, lhs.span.to(self.prev_span)) + mk_err_expr(self, inner_op.span.to(self.prev_span)) } Err(mut expr_err) => { expr_err.cancel(); @@ -597,7 +654,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. - mk_err_expr(self, lhs.span.to(self.prev_span)) + mk_err_expr(self, inner_op.span.to(self.prev_span)) } } } else { diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index dd2b6ab9971..098c8355ab9 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -4,7 +4,7 @@ use super::{SemiColonMode, SeqSep, TokenExpectType}; use crate::maybe_recover_from_interpolated_ty_qpath; use rustc_errors::{Applicability, PResult}; -use rustc_span::source_map::{self, Span}; +use rustc_span::source_map::{self, Span, Spanned}; use rustc_span::symbol::{kw, sym, Symbol}; use std::mem; use syntax::ast::{self, AttrStyle, AttrVec, CaptureBy, Field, Ident, Lit, DUMMY_NODE_ID}; @@ -180,17 +180,17 @@ impl<'a> Parser<'a> { }; let cur_op_span = self.token.span; - let restrictions = if op.is_assign_like() { + let restrictions = if op.node.is_assign_like() { self.restrictions & Restrictions::NO_STRUCT_LITERAL } else { self.restrictions }; - let prec = op.precedence(); + let prec = op.node.precedence(); if prec < min_prec { break; } // Check for deprecated `...` syntax - if self.token == token::DotDotDot && op == AssocOp::DotDotEq { + if self.token == token::DotDotDot && op.node == AssocOp::DotDotEq { self.err_dotdotdot_syntax(self.token.span); } @@ -199,11 +199,12 @@ impl<'a> Parser<'a> { } self.bump(); - if op.is_comparison() { + if op.node.is_comparison() { if let Some(expr) = self.check_no_chained_comparison(&lhs, &op)? { return Ok(expr); } } + let op = op.node; // Special cases: if op == AssocOp::As { lhs = self.parse_assoc_op_cast(lhs, lhs_span, ExprKind::Cast)?; @@ -297,7 +298,7 @@ impl<'a> Parser<'a> { } fn should_continue_as_assoc_expr(&mut self, lhs: &Expr) -> bool { - match (self.expr_is_complete(lhs), self.check_assoc_op()) { + match (self.expr_is_complete(lhs), self.check_assoc_op().map(|op| op.node)) { // Semi-statement forms are odd: // See https://github.com/rust-lang/rust/issues/29071 (true, None) => false, @@ -342,19 +343,22 @@ impl<'a> Parser<'a> { /// The method does not advance the current token. /// /// Also performs recovery for `and` / `or` which are mistaken for `&&` and `||` respectively. - fn check_assoc_op(&self) -> Option<AssocOp> { - match (AssocOp::from_token(&self.token), &self.token.kind) { - (op @ Some(_), _) => op, - (None, token::Ident(sym::and, false)) => { - self.error_bad_logical_op("and", "&&", "conjunction"); - Some(AssocOp::LAnd) - } - (None, token::Ident(sym::or, false)) => { - self.error_bad_logical_op("or", "||", "disjunction"); - Some(AssocOp::LOr) - } - _ => None, - } + fn check_assoc_op(&self) -> Option<Spanned<AssocOp>> { + Some(Spanned { + node: match (AssocOp::from_token(&self.token), &self.token.kind) { + (Some(op), _) => op, + (None, token::Ident(sym::and, false)) => { + self.error_bad_logical_op("and", "&&", "conjunction"); + AssocOp::LAnd + } + (None, token::Ident(sym::or, false)) => { + self.error_bad_logical_op("or", "||", "disjunction"); + AssocOp::LOr + } + _ => return None, + }, + span: self.token.span, + }) } /// Error on `and` and `or` suggesting `&&` and `||` respectively. |