From a16722d2210caa5eea941ffa20837859e249fd05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 16 Nov 2023 21:21:26 +0000 Subject: [PATCH] Handle attempts to have multiple `cfg`d tail expressions When encountering code that seems like it might be trying to have multiple tail expressions depending on `cfg` information, suggest alternatives that will success to parse. ```rust fn foo() -> String { #[cfg(feature = "validation")] [1, 2, 3].iter().map(|c| c.to_string()).collect::() #[cfg(not(feature = "validation"))] String::new() } ``` ``` error: expected `;`, found `#` --> $DIR/multiple-tail-expr-behind-cfg.rs:5:64 | LL | #[cfg(feature = "validation")] | ------------------------------ only `;` terminated statements or tail expressions are allowed after this attribute LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::() | ^ expected `;` here LL | #[cfg(not(feature = "validation"))] | - unexpected token | help: add `;` here | LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::(); | + help: alternatively, consider surrounding the expression with a block | LL | { [1, 2, 3].iter().map(|c| c.to_string()).collect::() } | + + help: it seems like you are trying to provide different expressions depending on `cfg`, consider using `if cfg!(..)` | LL ~ if cfg!(feature = "validation") { LL ~ [1, 2, 3].iter().map(|c| c.to_string()).collect::() LL ~ } else if cfg!(not(feature = "validation")) { LL ~ String::new() LL + } | ``` Fix #106020. --- .../rustc_parse/src/parser/diagnostics.rs | 96 +++++++++++++++++++ compiler/rustc_parse/src/parser/stmt.rs | 14 +++ .../multiple-tail-expr-behind-cfg.rs | 19 ++++ .../multiple-tail-expr-behind-cfg.stderr | 54 +++++++++++ 4 files changed, 183 insertions(+) create mode 100644 tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.rs create mode 100644 tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index f2acb70ac45..37183bbc721 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -21,6 +21,7 @@ use crate::fluent_generated as fluent; use crate::parser; +use crate::parser::attr::InnerAttrPolicy; use rustc_ast as ast; use rustc_ast::ptr::P; use rustc_ast::token::{self, Delimiter, Lit, LitKind, TokenKind}; @@ -722,6 +723,101 @@ fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool { Err(err) } + pub(super) fn attr_on_non_tail_expr(&self, expr: &Expr) { + // Missing semicolon typo error. + let span = self.prev_token.span.shrink_to_hi(); + let mut err = self.sess.create_err(ExpectedSemi { + span, + token: self.token.clone(), + unexpected_token_label: Some(self.token.span), + sugg: ExpectedSemiSugg::AddSemi(span), + }); + let attr_span = match &expr.attrs[..] { + [] => unreachable!(), + [only] => only.span, + [first, rest @ ..] => { + for attr in rest { + err.span_label(attr.span, ""); + } + first.span + } + }; + err.span_label( + attr_span, + format!( + "only `;` terminated statements or tail expressions are allowed after {}", + if expr.attrs.len() == 1 { "this attribute" } else { "these attributes" }, + ), + ); + if self.token == token::Pound + && self.look_ahead(1, |t| t.kind == token::OpenDelim(Delimiter::Bracket)) + { + // We have + // #[attr] + // expr + // #[not_attr] + // other_expr + err.span_label(span, "expected `;` here"); + err.multipart_suggestion( + "alternatively, consider surrounding the expression with a block", + vec![ + (expr.span.shrink_to_lo(), "{ ".to_string()), + (expr.span.shrink_to_hi(), " }".to_string()), + ], + Applicability::MachineApplicable, + ); + let mut snapshot = self.create_snapshot_for_diagnostic(); + if let [attr] = &expr.attrs[..] + && let ast::AttrKind::Normal(attr_kind) = &attr.kind + && let [segment] = &attr_kind.item.path.segments[..] + && segment.ident.name == sym::cfg + && let Ok(next_attr) = snapshot.parse_attribute(InnerAttrPolicy::Forbidden(None)) + && let ast::AttrKind::Normal(next_attr_kind) = next_attr.kind + && let [next_segment] = &next_attr_kind.item.path.segments[..] + && segment.ident.name == sym::cfg + && let Ok(next_expr) = snapshot.parse_expr() + { + // We have for sure + // #[cfg(..)] + // expr + // #[cfg(..)] + // other_expr + // So we suggest using `if cfg!(..) { expr } else if cfg!(..) { other_expr }`. + let margin = self.sess.source_map().span_to_margin(next_expr.span).unwrap_or(0); + let sugg = vec![ + (attr.span.with_hi(segment.span().hi()), "if cfg!".to_string()), + ( + attr_kind.item.args.span().unwrap().shrink_to_hi().with_hi(attr.span.hi()), + " {".to_string(), + ), + (expr.span.shrink_to_lo(), " ".to_string()), + ( + next_attr.span.with_hi(next_segment.span().hi()), + "} else if cfg!".to_string(), + ), + ( + next_attr_kind + .item + .args + .span() + .unwrap() + .shrink_to_hi() + .with_hi(next_attr.span.hi()), + " {".to_string(), + ), + (next_expr.span.shrink_to_lo(), " ".to_string()), + (next_expr.span.shrink_to_hi(), format!("\n{}}}", " ".repeat(margin))), + ]; + err.multipart_suggestion( + "it seems like you are trying to provide different expressions depending on \ + `cfg`, consider using `if cfg!(..)`", + sugg, + Applicability::MachineApplicable, + ); + } + } + err.emit(); + } fn check_too_many_raw_str_terminators(&mut self, err: &mut Diagnostic) -> bool { let sm = self.sess.source_map(); match (&self.prev_token.kind, &self.token.kind) { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index aa939a71d63..758e9aafb04 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -617,6 +617,20 @@ pub fn parse_full_stmt( let mut add_semi_to_stmt = false; match &mut stmt.kind { + // Expression without semicolon. + StmtKind::Expr(expr) + if classify::expr_requires_semi_to_be_stmt(expr) + && !expr.attrs.is_empty() + && ![token::Eof, token::Semi, token::CloseDelim(Delimiter::Brace)] + .contains(&self.token.kind) => + { + // The user has written `#[attr] expr` which is unsupported. (#106020) + self.attr_on_non_tail_expr(&expr); + // We already emitted an error, so don't emit another type error + let sp = expr.span.to(self.prev_token.span); + *expr = self.mk_expr_err(sp); + } + // Expression without semicolon. StmtKind::Expr(expr) if self.token != token::Eof && classify::expr_requires_semi_to_be_stmt(expr) => diff --git a/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.rs b/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.rs new file mode 100644 index 00000000000..d97f24a3d29 --- /dev/null +++ b/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.rs @@ -0,0 +1,19 @@ +#![feature(stmt_expr_attributes)] + +fn foo() -> String { + #[cfg(feature = "validation")] + [1, 2, 3].iter().map(|c| c.to_string()).collect::() //~ ERROR expected `;`, found `#` + #[cfg(not(feature = "validation"))] + String::new() +} + +fn bar() -> String { + #[attr] + [1, 2, 3].iter().map(|c| c.to_string()).collect::() //~ ERROR expected `;`, found `#` + #[attr] //~ ERROR cannot find attribute `attr` in this scope + String::new() +} + +fn main() { + println!("{}", foo()); +} diff --git a/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.stderr b/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.stderr new file mode 100644 index 00000000000..a71253a5e42 --- /dev/null +++ b/tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.stderr @@ -0,0 +1,54 @@ +error: expected `;`, found `#` + --> $DIR/multiple-tail-expr-behind-cfg.rs:5:64 + | +LL | #[cfg(feature = "validation")] + | ------------------------------ only `;` terminated statements or tail expressions are allowed after this attribute +LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::() + | ^ expected `;` here +LL | #[cfg(not(feature = "validation"))] + | - unexpected token + | +help: add `;` here + | +LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::(); + | + +help: alternatively, consider surrounding the expression with a block + | +LL | { [1, 2, 3].iter().map(|c| c.to_string()).collect::() } + | + + +help: it seems like you are trying to provide different expressions depending on `cfg`, consider using `if cfg!(..)` + | +LL ~ if cfg!(feature = "validation") { +LL ~ [1, 2, 3].iter().map(|c| c.to_string()).collect::() +LL ~ } else if cfg!(not(feature = "validation")) { +LL ~ String::new() +LL + } + | + +error: expected `;`, found `#` + --> $DIR/multiple-tail-expr-behind-cfg.rs:12:64 + | +LL | #[attr] + | ------- only `;` terminated statements or tail expressions are allowed after this attribute +LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::() + | ^ expected `;` here +LL | #[attr] + | - unexpected token + | +help: add `;` here + | +LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::(); + | + +help: alternatively, consider surrounding the expression with a block + | +LL | { [1, 2, 3].iter().map(|c| c.to_string()).collect::() } + | + + + +error: cannot find attribute `attr` in this scope + --> $DIR/multiple-tail-expr-behind-cfg.rs:13:7 + | +LL | #[attr] + | ^^^^ + +error: aborting due to 3 previous errors +