diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8213d604b91b2..4ce4da54cfac0 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4446,15 +4446,85 @@ impl<'a> Parser<'a> { continue; }; } + + let block_sp = lo.to(self.prev_span); + + self.warn_closure_in_block(block_sp, &stmts); + Ok(P(ast::Block { stmts, id: ast::DUMMY_NODE_ID, rules: s, - span: lo.to(self.prev_span), + span: block_sp, recovered, })) } + /// Possibly trying to write closure in a non-Rust language + /// + /// The following, given that `foo` takes a single argument, is incorrect: + /// + /// ```text + /// let x = "asdf".chars().filter({|x| + /// println!("{:?}", x); + /// x.is_whitespace() + /// }); + /// ``` + /// + /// Writing the above code _without_ the `println!()` _would_ work, as it is syntactically + /// correct, so we don't warn in that case, as the enclosing block implicitly returns the + /// closure, which evaluates correctly. That case would be caught by clippy. + /// + /// Originally reported as #27300. + fn warn_closure_in_block(&self, block_sp: Span, stmts: &Vec) { + match stmts.get(0) { + Some(Stmt { + node: StmtKind::Semi(expr), .. + }) if stmts.len() > 1 => { + if let ExprKind::Closure(_, _, _, ref cl_expr, cl_sp) = expr.node { + if let ExprKind::Block(_) = cl_expr.node { + // Only emit warning if the closure is not using a block for its body, as it + // more likely we're trying to write a closure in another's language syntax: + // + // let x = {|arg| stmts} + } else { + let mut warn = self.diagnostic().struct_span_warn( + cl_sp, + "a closure's body is not determined by its enclosing block" + ); + warn.span_label( + cl_sp, + "this closure's body is not determined by its enclosing block" + ); + warn.span_note(expr.span, "this is the closure's block..."); + + if let Some(Stmt { + node: StmtKind::Expr(expr), .. + }) = stmts.last() { + warn.span_note(block_sp, "...while this enclosing block..."); + warn.span_note(expr.span, "...implicitly returns this"); + } else { + warn.span_note(block_sp, "...while this is the enclosing block"); + } + match self.sess.codemap().span_to_snippet(cl_sp) { + Ok(snippet) => warn.span_suggestion( + block_sp.until(cl_sp).to(cl_sp), + "you should open the block *after* the closure's argument list", + format!("{} {{", snippet) + ), + _ => warn.help( + "you should open the block *after* the closure's argument list:\ + || {" + ), + }; + warn.emit(); + } + } + } + _ => (), + } + } + /// Parse a statement, including the trailing semicolon. pub fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option> { let mut stmt = match self.parse_stmt_without_recovery(macro_legacy_warnings)? { diff --git a/src/test/ui/suggestions/block-enclosed-closure.rs b/src/test/ui/suggestions/block-enclosed-closure.rs new file mode 100644 index 0000000000000..6acc14923737d --- /dev/null +++ b/src/test/ui/suggestions/block-enclosed-closure.rs @@ -0,0 +1,34 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// #27300 + +fn main() { + let _p = Some(45).and_then({|x| + //~^ ERROR the trait bound `std::option::Option<_>: std::ops::FnOnce<({integer},)>` + //~| NOTE the trait `std::ops::FnOnce<({integer},)>` is not implemented for + println!("hi"); + Some(x * 2) //~ NOTE ...implicitly returns this + }); + //~^^^^^^ WARN a closure's body is not determined by its enclosing block + //~| NOTE this closure's body is not determined by its enclosing block + //~| NOTE this is the closure's block... + //~| NOTE ...while this enclosing block... + //~| HELP you should open the block *after* the closure's argument list + //~^^^^^^^ ERROR cannot find value `x` in this scope + //~| NOTE not found in this scope + + // Don't alert on the folloing case, even though it is likely that the user is confused in the + // same way as the first test case, as 1) clippy will warn about this and 2) if they ever + // change it, the appropriate warning will trigger. + let _y = Some(45).and_then({|x| + Some(x * 2) + }); +} diff --git a/src/test/ui/suggestions/block-enclosed-closure.stderr b/src/test/ui/suggestions/block-enclosed-closure.stderr new file mode 100644 index 0000000000000..db9bdfe95098b --- /dev/null +++ b/src/test/ui/suggestions/block-enclosed-closure.stderr @@ -0,0 +1,50 @@ +warning: a closure's body is not determined by its enclosing block + --> $DIR/block-enclosed-closure.rs:14:33 + | +14 | let _p = Some(45).and_then({|x| + | ^^^ this closure's body is not determined by its enclosing block + | +note: this is the closure's block... + --> $DIR/block-enclosed-closure.rs:14:33 + | +14 | let _p = Some(45).and_then({|x| + | _________________________________^ +15 | | //~^ ERROR the trait bound `std::option::Option<_>: std::ops::FnOnce<({integer},)>` +16 | | //~| NOTE the trait `std::ops::FnOnce<({integer},)>` is not implemented for +17 | | println!("hi"); + | |______________________^ +note: ...while this enclosing block... + --> $DIR/block-enclosed-closure.rs:14:32 + | +14 | let _p = Some(45).and_then({|x| + | ________________________________^ +15 | | //~^ ERROR the trait bound `std::option::Option<_>: std::ops::FnOnce<({integer},)>` +16 | | //~| NOTE the trait `std::ops::FnOnce<({integer},)>` is not implemented for +17 | | println!("hi"); +18 | | Some(x * 2) //~ NOTE ...implicitly returns this +19 | | }); + | |_____^ +note: ...implicitly returns this + --> $DIR/block-enclosed-closure.rs:18:9 + | +18 | Some(x * 2) //~ NOTE ...implicitly returns this + | ^^^^^^^^^^^ +help: you should open the block *after* the closure's argument list + | +14 | let _p = Some(45).and_then(|x| { + | ^^^^^ + +error[E0425]: cannot find value `x` in this scope + --> $DIR/block-enclosed-closure.rs:18:14 + | +18 | Some(x * 2) //~ NOTE ...implicitly returns this + | ^ not found in this scope + +error[E0277]: the trait bound `std::option::Option<_>: std::ops::FnOnce<({integer},)>` is not satisfied + --> $DIR/block-enclosed-closure.rs:14:23 + | +14 | let _p = Some(45).and_then({|x| + | ^^^^^^^^ the trait `std::ops::FnOnce<({integer},)>` is not implemented for `std::option::Option<_>` + +error: aborting due to 2 previous errors +