Skip to content

Commit

Permalink
Verify that an if condition block returns a value
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Aug 18, 2017
1 parent 20a2716 commit f063233
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 5 deletions.
26 changes: 26 additions & 0 deletions src/libsyntax/ast.rs
Expand Up @@ -844,6 +844,32 @@ pub struct Expr {
pub attrs: ThinVec<Attribute>
}

impl Expr {
/// Wether this expression would be valid somewhere that expects a value, for example, an `if`
/// condition.
pub fn returns(&self) -> bool {
if let ExprKind::Block(ref block) = self.node {
match block.stmts.last().map(|last_stmt| &last_stmt.node) {
// implicit return
Some(&StmtKind::Expr(_)) => true,
Some(&StmtKind::Semi(ref expr)) => {
if let ExprKind::Ret(_) = expr.node {
// last statement is explicit return
true
} else {
false
}
}
// This is a block that doesn't end in either an implicit or explicit return
_ => false,
}
} else {
// This is not a block, it is a value
true
}
}
}

impl fmt::Debug for Expr {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "expr({}: {})", self.id, pprust::expr_to_string(self))
Expand Down
7 changes: 6 additions & 1 deletion src/libsyntax/parse/parser.rs
Expand Up @@ -2968,7 +2968,12 @@ impl<'a> Parser<'a> {
}
let lo = self.prev_span;
let cond = self.parse_expr_res(RESTRICTION_NO_STRUCT_LITERAL, None)?;
if self.eat_keyword(keywords::Else) {

// Verify that the parsed `if` condition makes sense as a condition. If it is a block, then
// verify that the last statement is either an implicit return (no `;`) or an explicit
// return. This won't catch blocks with an explicit `return`, but that would be caught by
// the dead code lint.
if self.eat_keyword(keywords::Else) || !cond.returns() {
let sp = lo.next_point();
let mut err = self.diagnostic()
.struct_span_err(sp, "missing condition for `if` statemement");
Expand Down
13 changes: 11 additions & 2 deletions src/test/ui/issue-13483.rs
Expand Up @@ -10,7 +10,16 @@

fn main() {
if true {
} else if { //ERROR: MISSING CONDITIONAL
} else if {
} else {
};
}
}

fn foo() {
if true {
} else if {
}
bar();
}

fn bar() {}
10 changes: 8 additions & 2 deletions src/test/ui/issue-13483.stderr
@@ -1,8 +1,14 @@
error: missing condition for `if` statemement
--> $DIR/issue-13483.rs:13:14
|
13 | } else if { //ERROR: MISSING CONDITIONAL
13 | } else if {
| ^ expected if condition here

error: aborting due to previous error
error: missing condition for `if` statemement
--> $DIR/issue-13483.rs:20:14
|
20 | } else if {
| ^ expected if condition here

error: aborting due to 2 previous errors

0 comments on commit f063233

Please sign in to comment.