diff --git a/src/libsyntax_ext/assert.rs b/src/libsyntax_ext/assert.rs index d2c397e0ecc..cd69733571d 100644 --- a/src/libsyntax_ext/assert.rs +++ b/src/libsyntax_ext/assert.rs @@ -1,10 +1,11 @@ -use errors::DiagnosticBuilder; +use errors::{Applicability, DiagnosticBuilder}; use syntax::ast::{self, *}; use syntax::source_map::Spanned; use syntax::ext::base::*; use syntax::ext::build::AstBuilder; use syntax::parse::token; +use syntax::parse::parser::Parser; use syntax::print::pprust; use syntax::ptr::P; use syntax::symbol::Symbol; @@ -74,17 +75,69 @@ fn parse_assert<'a>( return Err(err); } - Ok(Assert { - cond_expr: parser.parse_expr()?, - custom_message: if parser.eat(&token::Comma) { - let ts = parser.parse_tokens(); - if !ts.is_empty() { - Some(ts) - } else { - None - } - } else { - None - }, - }) + let cond_expr = parser.parse_expr()?; + + // Some crates use the `assert!` macro in the following form (note extra semicolon): + // + // assert!( + // my_function(); + // ); + // + // Warn about semicolon and suggest removing it. Eventually, this should be turned into an + // error. + if parser.token == token::Semi { + let mut err = cx.struct_span_warn(sp, "macro requires an expression as an argument"); + err.span_suggestion( + parser.span, + "try removing semicolon", + String::new(), + Applicability::MaybeIncorrect + ); + err.note("this is going to be an error in the future"); + err.emit(); + + parser.bump(); + } + + // Some crates use the `assert!` macro in the following form (note missing comma before + // message): + // + // assert!(true "error message"); + // + // Parse this as an actual message, and suggest inserting a comma. Eventually, this should be + // turned into an error. + let custom_message = if let token::Literal(token::Lit::Str_(_), _) = parser.token { + let mut err = cx.struct_span_warn(parser.span, "unexpected string literal"); + let comma_span = cx.source_map().next_point(parser.prev_span); + err.span_suggestion_short( + comma_span, + "try adding a comma", + ", ".to_string(), + Applicability::MaybeIncorrect + ); + err.note("this is going to be an error in the future"); + err.emit(); + + parse_custom_message(&mut parser) + } else if parser.eat(&token::Comma) { + parse_custom_message(&mut parser) + } else { + None + }; + + if parser.token != token::Eof { + parser.expect_one_of(&[], &[])?; + unreachable!(); + } + + Ok(Assert { cond_expr, custom_message }) +} + +fn parse_custom_message<'a>(parser: &mut Parser<'a>) -> Option { + let ts = parser.parse_tokens(); + if !ts.is_empty() { + Some(ts) + } else { + None + } } diff --git a/src/test/ui/macros/assert-trailing-junk.rs b/src/test/ui/macros/assert-trailing-junk.rs new file mode 100644 index 00000000000..676ae05bf0f --- /dev/null +++ b/src/test/ui/macros/assert-trailing-junk.rs @@ -0,0 +1,24 @@ +// Ensure assert macro does not ignore trailing garbage. +// +// See https://github.com/rust-lang/rust/issues/60024 for details. + +fn main() { + assert!(true some extra junk, "whatever"); + //~^ ERROR expected one of + + assert!(true some extra junk); + //~^ ERROR expected one of + + assert!(true, "whatever" blah); + //~^ ERROR no rules expected + + assert!(true "whatever" blah); + //~^ WARN unexpected string literal + //~^^ ERROR no rules expected + + assert!(true;); + //~^ WARN macro requires an expression + + assert!(false || true "error message"); + //~^ WARN unexpected string literal +} diff --git a/src/test/ui/macros/assert-trailing-junk.stderr b/src/test/ui/macros/assert-trailing-junk.stderr new file mode 100644 index 00000000000..6fc0a278461 --- /dev/null +++ b/src/test/ui/macros/assert-trailing-junk.stderr @@ -0,0 +1,60 @@ +error: expected one of `,`, `.`, `?`, or an operator, found `some` + --> $DIR/assert-trailing-junk.rs:6:18 + | +LL | assert!(true some extra junk, "whatever"); + | ^^^^ expected one of `,`, `.`, `?`, or an operator here + +error: expected one of `,`, `.`, `?`, or an operator, found `some` + --> $DIR/assert-trailing-junk.rs:9:18 + | +LL | assert!(true some extra junk); + | ^^^^ expected one of `,`, `.`, `?`, or an operator here + +error: no rules expected the token `blah` + --> $DIR/assert-trailing-junk.rs:12:30 + | +LL | assert!(true, "whatever" blah); + | -^^^^ no rules expected this token in macro call + | | + | help: missing comma here + +warning: unexpected string literal + --> $DIR/assert-trailing-junk.rs:15:18 + | +LL | assert!(true "whatever" blah); + | -^^^^^^^^^^ + | | + | help: try adding a comma + | + = note: this is going to be an error in the future + +error: no rules expected the token `blah` + --> $DIR/assert-trailing-junk.rs:15:29 + | +LL | assert!(true "whatever" blah); + | -^^^^ no rules expected this token in macro call + | | + | help: missing comma here + +warning: macro requires an expression as an argument + --> $DIR/assert-trailing-junk.rs:19:5 + | +LL | assert!(true;); + | ^^^^^^^^^^^^-^^ + | | + | help: try removing semicolon + | + = note: this is going to be an error in the future + +warning: unexpected string literal + --> $DIR/assert-trailing-junk.rs:22:27 + | +LL | assert!(false || true "error message"); + | -^^^^^^^^^^^^^^^ + | | + | help: try adding a comma + | + = note: this is going to be an error in the future + +error: aborting due to 4 previous errors +