From 9b5e014f740a94224b0b1c652940b34d1c61e7dd Mon Sep 17 00:00:00 2001
From: Franco Bulgarelli <franco@mumuki.org>
Date: Sat, 18 Apr 2020 02:11:19 -0300
Subject: [PATCH] Implementing broken return expectation

---
 spec/JavaScriptSpec.hs                        |  3 ++
 spec/SmellSpec.hs                             | 34 +++++++++++++++++++
 .../Mulang/Inspector/Generic/Smell.hs         | 16 +++++++++
 src/Language/Mulang/Parsers/JavaScript.hs     |  2 ++
 4 files changed, 55 insertions(+)

diff --git a/spec/JavaScriptSpec.hs b/spec/JavaScriptSpec.hs
index 6f0df4abb..420a9a833 100644
--- a/spec/JavaScriptSpec.hs
+++ b/spec/JavaScriptSpec.hs
@@ -38,6 +38,9 @@ spec = do
     it "differentiates procedures and functions" $ do
       (js "function f() { return 1 }" /= js "function f() { 1 }") `shouldBe` True
 
+    it "differentiates procedures and functions with returns in if" $ do
+      (js "function f(){ if (x) { } else { return 4 } }") `shouldBe` (SimpleFunction "f" [] (If (Reference "x") None (Return (MuNumber 4.0))))
+
     it "handles lambdas likes haskell does" $ do
       js "var m = function(x) { return 1 }" `shouldBe` hs "m = \\x -> 1"
 
diff --git a/spec/SmellSpec.hs b/spec/SmellSpec.hs
index fc5451c28..099329cbf 100644
--- a/spec/SmellSpec.hs
+++ b/spec/SmellSpec.hs
@@ -128,6 +128,40 @@ spec = do
     it "is False when local variable is used as a cache" $ do
       hasRedundantLocalVariableReturn (js "function x(m) { var x = 5; var y = 1 + x; g(y); return x; }") `shouldBe` False
 
+  describe "hasBrokenReturn" $ do
+    it "is False when there are no functions" $ do
+      hasBrokenReturn (js "let y = 0; if (x) { y = 0 } ") `shouldBe` False
+
+    it "is False when there is a function that has only a return" $ do
+      hasBrokenReturn (js "function f(){ return 4 } ") `shouldBe` False
+
+    it "is False when there is a function that has a single flow and return" $ do
+      hasBrokenReturn (js "function f(){ console.log(2); return 4 } ") `shouldBe` False
+
+    it "is False when there is a function that has an if with no returns " $ do
+      hasBrokenReturn (js "function f(){ if (x) { console.log(4) } else { console.log(5) } return 4 } ") `shouldBe` False
+
+    it "is False when there is a function that has an if with no returns and no else " $ do
+        hasBrokenReturn (js "function f(){ if (x) { console.log(4) } return 4 } ") `shouldBe` False
+
+    it "is True when there is a function that has an if with a return and no else and not trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { return 4 } } ") `shouldBe` True
+
+    it "is True when there is a function that has an if with a return in else but not then and not trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { } else { return 4 } }") `shouldBe` True
+
+    it "is True when there is a function that nested ifs with incomplete return and no trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { if (y) { return 5 } } else { return 4 } }") `shouldBe` True
+
+    it "is False when there is a function that has an if with a return and no else and trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { return 4 } return 5; } ") `shouldBe` False
+
+    it "is False when there is a function that has an if with a return in else but not then and trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { } else { return 4 } return 5; } ") `shouldBe` False
+
+    it "is False when there is a function that nested ifs with complete return and no trailing return " $ do
+        hasBrokenReturn (js "function f(){ if (x) { if (y) { return 5 } else { return 6 } } else { return 4 } }") `shouldBe` False
+
 
   describe "hasAssignmentCondition" $ do
     it "is True when assigns within an if condition" $ do
diff --git a/src/Language/Mulang/Inspector/Generic/Smell.hs b/src/Language/Mulang/Inspector/Generic/Smell.hs
index 7b9acbab5..018d6d834 100644
--- a/src/Language/Mulang/Inspector/Generic/Smell.hs
+++ b/src/Language/Mulang/Inspector/Generic/Smell.hs
@@ -5,6 +5,7 @@ module Language.Mulang.Inspector.Generic.Smell (
   doesConsolePrint,
   doesNilTest,
   doesTypeTest,
+  hasBrokenReturn,
   hasAssignmentReturn,
   hasAssignmentCondition,
   hasEmptyIfBranches,
@@ -140,6 +141,21 @@ hasRedundantLocalVariableReturn = containsExpression f
                       Return (Reference returnedVariable)]) = returnedVariable == declaredVariable
         f _                                                 = False
 
+hasBrokenReturn :: Inspection
+hasBrokenReturn = containsDeclaration f
+  where
+    f (SimpleFunction _ _ body) = not . fullyReturns $ body
+    f _                         = False
+
+    fullyReturns :: Expression -> Bool
+    fullyReturns (Sequence (Return _:_))    = True
+    fullyReturns (Sequence (If _ b1 b2:es)) = fullyReturns b1 && fullyReturns b2 || fullyReturns (Sequence es)
+    fullyReturns (Sequence (_:es))          = fullyReturns (Sequence es)
+    fullyReturns (Return _)                 = True
+    fullyReturns (If _ b1 b2)               = fullyReturns b1 && fullyReturns b2
+    fullyReturns _                          = False
+
+
 hasAssignmentCondition :: Inspection
 hasAssignmentCondition = containsExpression f
   where f (If (Unification _ _) _ _)  = True
diff --git a/src/Language/Mulang/Parsers/JavaScript.hs b/src/Language/Mulang/Parsers/JavaScript.hs
index f571cb489..eb7dbe0e0 100644
--- a/src/Language/Mulang/Parsers/JavaScript.hs
+++ b/src/Language/Mulang/Parsers/JavaScript.hs
@@ -121,7 +121,9 @@ computationFor body | containsReturn body = Function
                     | otherwise           = Procedure
 
 containsReturn :: Expression -> Bool
+containsReturn (Return None) = False
 containsReturn (Return _)    = True
+containsReturn (If _ t e)    = containsReturn t || containsReturn e
 containsReturn (Sequence xs) = any containsReturn xs
 containsReturn _             = False