Reject certain code that should return but doesn't

Fixes #497

crates/analyzer/src/traversal/functions.rs

@@ -174,7 +174,7 @@ fn all_paths_return_or_revert(block: &[Node<fe::FuncStmt>]) -> bool {
                 or_else,
             } => {
                 let body_returns = all_paths_return_or_revert(body);
-                let or_else_returns = or_else.is_empty() || all_paths_return_or_revert(or_else);
+                let or_else_returns = all_paths_return_or_revert(or_else);
                 if body_returns && or_else_returns {
                     return true;
                 }

crates/analyzer/tests/errors.rs

@@ -180,6 +180,7 @@ test_file! { issue_451 }
 test_file! { mismatch_return_type }
 test_file! { missing_return }
 test_file! { missing_return_in_else }
+test_file! { missing_return_after_if }
 test_file! { needs_mem_copy }
 test_file! { not_callable }
 test_file! { not_in_scope }

crates/analyzer/tests/snapshots/errors__missing_return_after_if.snap

@@ -0,0 +1,14 @@
+---
+source: crates/analyzer/tests/errors.rs
+expression: "error_string(&path, &src)"
+
+---
+error: function body is missing a return or revert statement
+  ┌─ compile_errors/missing_return_after_if.fe:2:13
+  │
+2 │     pub def bar(val: u256) -> u256:
+  │             ^^^               ---- expected function to return `u256`
+  │             │                  
+  │             all paths of this function must `return` or `revert`
+
+

crates/analyzer/tests/snapshots/errors__missing_return_in_else.snap

@@ -11,10 +11,4 @@ error: function body is missing a return or revert statement
   │             │                  
   │             all paths of this function must `return` or `revert`
 
-error: cannot find value `x` in this scope
-  ┌─ compile_errors/missing_return_in_else.fe:6:13
-  │
-6 │             x = 1
-  │             ^ undefined
-
 

crates/test-files/fixtures/compile_errors/missing_return_after_if.fe

@@ -0,0 +1,4 @@
+contract Foo:
+    pub def bar(val: u256) -> u256:
+        if val > 1:
+            return 5

crates/test-files/fixtures/compile_errors/missing_return_in_else.fe

@@ -3,4 +3,4 @@ contract Foo:
         if val > 1:
             return 5
         else:
-            x = 1
+            x:u256 = 1

crates/test-files/src/lib.rs

@@ -5,14 +5,14 @@ const FIXTURES: Dir = include_dir!("fixtures");
 pub fn fixture(path: &str) -> &'static str {
     FIXTURES
         .get_file(path)
-        .expect("bad fixture file path")
+        .unwrap_or_else(|| panic!("bad fixture file path {}", path))
         .contents_utf8()
         .expect("fixture file isn't utf8")
 }
 
 pub fn fixture_bytes(path: &str) -> &'static [u8] {
     FIXTURES
         .get_file(path)
-        .expect("bad fixture file path")
+        .unwrap_or_else(|| panic!("bad fixture file path {}", path))
         .contents()
 }

newsfragments/497.bugfix.md

@@ -0,0 +1,14 @@
+Fixed an issue with a missing return statement not properly detected.
+
+Previous to this fix, the following code compiles but it should not:
+
+```
+contract Foo:
+    pub def bar(val: u256) -> u256:
+        if val > 1:
+            return 5
+```
+
+With this change, the compiler rightfully detects that the code is missing
+a `return` or `revert` statement after the `if` statement since it is not
+guaranteed that the path of execution always follows the arm of the `if` statement.