diff options
| author | Esteban Küber <esteban@kuber.com.ar> | 2017-08-17 16:51:52 -0700 |
|---|---|---|
| committer | Esteban Küber <esteban@kuber.com.ar> | 2017-08-17 20:25:46 -0700 |
| commit | f06323337dd0cf748e8a0ca49d91dac2eae6b4a9 (patch) | |
| tree | d54890bcfaf44fe77098b6de042892eefba586b2 /src | |
| parent | 20a2716206ca879ad9848cbb4ac9fcc739dd3f77 (diff) | |
| download | rust-f06323337dd0cf748e8a0ca49d91dac2eae6b4a9.tar.gz rust-f06323337dd0cf748e8a0ca49d91dac2eae6b4a9.zip | |
Verify that an `if` condition block returns a value
Diffstat (limited to 'src')
| -rw-r--r-- | src/libsyntax/ast.rs | 26 | ||||
| -rw-r--r-- | src/libsyntax/parse/parser.rs | 7 | ||||
| -rw-r--r-- | src/test/ui/issue-13483.rs | 13 | ||||
| -rw-r--r-- | src/test/ui/issue-13483.stderr | 10 |
4 files changed, 51 insertions, 5 deletions
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 38ef79822c7..747d1a44923 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -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)) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 22999afb48e..85d2a485bfa 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -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"); diff --git a/src/test/ui/issue-13483.rs b/src/test/ui/issue-13483.rs index e94dc38c5e9..86378043912 100644 --- a/src/test/ui/issue-13483.rs +++ b/src/test/ui/issue-13483.rs @@ -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() {} diff --git a/src/test/ui/issue-13483.stderr b/src/test/ui/issue-13483.stderr index e49fdcf6d20..3446969dfd2 100644 --- a/src/test/ui/issue-13483.stderr +++ b/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 |
