From e9aa5532162ce0c4dce02a0a3ff2da996f1f27c4 Mon Sep 17 00:00:00 2001
From: mappu <mappu04@gmail.com>
Date: Tue, 7 Apr 2020 23:25:43 +1200
Subject: [PATCH] stmt: propagate err checks to top-level function calls and
 assignment calls

---
 fixtures/0008-try-catch-finally.php |   2 +-
 node.go                             | 111 ++++++++++++++++++++--------
 2 files changed, 81 insertions(+), 32 deletions(-)

diff --git a/fixtures/0008-try-catch-finally.php b/fixtures/0008-try-catch-finally.php
index f273ec6..501cc0d 100644
--- a/fixtures/0008-try-catch-finally.php
+++ b/fixtures/0008-try-catch-finally.php
@@ -6,7 +6,7 @@ function Foo(): int {
 }
 
 try {
-	echo Foo();
+	Foo();
 } catch (\BarException | \BazException $e) {
 	echo "bar or baz exception " . $e->getMessage() . "\n";
 } catch (\Exception $e) {
diff --git a/node.go b/node.go
index 1ba49a8..96df5b4 100644
--- a/node.go
+++ b/node.go
@@ -528,15 +528,34 @@ func (this *conversionState) convertNoFreeFloating(n_ node.Node) (string, error)
 			}
 		}
 
-		// Regular case
+		// Assignment expressions can take on better error-handling behaviour
+		// when we know the assignment is a top-level expression
+		if a, ok := n.Expr.(*assign.Assign); ok {
+			return this.convertAssignment(a, true)
+		}
 
+		// Non-assignment expression
 		child, err := this.convert(n.Expr)
 		if err != nil {
 			return "", parseErr{n, err}
 		}
 
-		ret := child + "\n" // standalone expression statement
-		return ret, nil
+		// If this is a simple expression (func call; indirect/method call; assignment of func/method call) then
+		// we need to propagate errors
+		switch n.Expr.(type) {
+		case *expr.FunctionCall, *expr.StaticCall, *expr.New:
+			ret := "_, err = " + child + "\n"
+			ret += "if err != nil {\n"
+			ret += this.currentErrHandler
+			ret += "}\n"
+			return ret, nil
+
+		default:
+			// Some other kind of general expression - no special error handling needed
+			ret := child + "\n" // standalone expression statement
+
+			return ret, nil
+		}
 
 	case *stmt.Echo:
 		// Convert into fmt.Print
@@ -699,34 +718,7 @@ func (this *conversionState) convertNoFreeFloating(n_ node.Node) (string, error)
 	//
 
 	case *assign.Assign:
-
-		rvalue, err := this.convert(n.Expression)
-		if err != nil {
-			return "", parseErr{n, err}
-		}
-
-		if dimf, ok := n.Variable.(*expr.ArrayDimFetch); ok && dimf.Dim == nil {
-			// Special handling for the case of foo[] = bar
-			// Transform into append()
-			arrayVar, err := this.convert(dimf.Variable)
-			if err != nil {
-				return "", parseErr{dimf, err}
-			}
-
-			return arrayVar + ` = append(` + arrayVar + `, ` + rvalue + `)`, nil
-
-		} else {
-
-			// Normal assignment
-
-			lvalue, err := this.convert(n.Variable) // might be a more complicated lvalue
-			if err != nil {
-				return "", parseErr{n, err}
-			}
-
-			// TODO this may need to use `:=`
-			return lvalue + " = " + rvalue, nil
-		}
+		return this.convertAssignment(n, false /* no reason to believe we are a top-level statement */)
 
 	//
 	// expr
@@ -1152,6 +1144,63 @@ func (this *conversionState) resolveName(n node.Node) (string, error) {
 	return paramType, nil
 }
 
+func (this *conversionState) convertAssignment(n *assign.Assign, isTopLevelStatement bool) (string, error) {
+
+	rvalue, err := this.convert(n.Expression)
+	if err != nil {
+		return "", parseErr{n, err}
+	}
+
+	if dimf, ok := n.Variable.(*expr.ArrayDimFetch); ok && dimf.Dim == nil {
+		// Special handling for the case of foo[] = bar
+		// Transform into append()
+		arrayVar, err := this.convert(dimf.Variable)
+		if err != nil {
+			return "", parseErr{dimf, err}
+		}
+
+		ret := arrayVar + ` = append(` + arrayVar + `, ` + rvalue + `)`
+		if isTopLevelStatement {
+			ret += "\n"
+		}
+		return ret, nil
+
+	} else {
+
+		// Normal assignment
+
+		lvalue, err := this.convert(n.Variable) // might be a more complicated lvalue
+		if err != nil {
+			return "", parseErr{n, err}
+		}
+
+		// TODO this may need to use `:=`
+
+		if isTopLevelStatement {
+
+			// If this is a simple expression (func call; indirect/method call; assignment of func/method call) then
+			// we need to propagate errors
+			switch n.Expression.(type) {
+			case *expr.FunctionCall, *expr.StaticCall, *expr.New:
+				ret := "_, " + lvalue + " = " + rvalue + "\n"
+				ret += "if err != nil {\n"
+				ret += this.currentErrHandler
+				ret += "}\n"
+				return ret, nil
+
+			default:
+				ret := lvalue + " = " + rvalue + "\n" // Just the basic assignment - with trailing NL
+				return ret, nil
+			}
+
+		} else {
+			return lvalue + " = " + rvalue, nil // Just the basic assignment - without trailing NL
+
+		}
+	}
+
+}
+
 // convertToStmtList asserts that the node is either a StmtList or wraps it in a
 //  single-stmt StmtList if not.
 // Loop bodies may be a StmtList if it is wrapped in {}, or a single statement