From ff13844b8643abaff9daa47a5bc96aa91fddcf5e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Brauer?= <bjoern.brauer@new-work.se>
Date: Tue, 15 Feb 2022 21:03:00 +0100
Subject: [PATCH] refactor: default empty if: statement to if: success() in
 evaluator & remove `FixIfStatement()` (#990)

* refactor: default empty `if:` statement to `if: success()` in evaluator

Previously the code to default an empty `if:` statement in the yaml file
was implemented in different files in the model package.
Now an empty `if:` statement defaults to `success()` in the expression
evaluator.

* refactor: remove obsolete `FixIfStatement` functions

The introduction of the expression evaluator seems to have made these
functions obsolete, as the test case `TestRunEvent/issue-598` works even
without these functions.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 pkg/exprparser/interpreter.go |  3 +++
 pkg/model/action.go           |  7 ------
 pkg/model/planner.go          | 47 -----------------------------------
 3 files changed, 3 insertions(+), 54 deletions(-)

diff --git a/pkg/exprparser/interpreter.go b/pkg/exprparser/interpreter.go
index 8518a12..584999b 100644
--- a/pkg/exprparser/interpreter.go
+++ b/pkg/exprparser/interpreter.go
@@ -47,6 +47,9 @@ func NewInterpeter(env *EvaluationEnvironment, config Config) Interpreter {
 
 func (impl *interperterImpl) Evaluate(input string, isIfExpression bool) (interface{}, error) {
 	input = strings.TrimPrefix(input, "${{")
+	if isIfExpression && input == "" {
+		input = "success()"
+	}
 	parser := actionlint.NewExprParser()
 	exprNode, err := parser.Parse(actionlint.NewExprLexer(input + "}}"))
 	if err != nil {
diff --git a/pkg/model/action.go b/pkg/model/action.go
index e0323af..1cfce63 100644
--- a/pkg/model/action.go
+++ b/pkg/model/action.go
@@ -90,12 +90,5 @@ func ReadAction(in io.Reader) (*Action, error) {
 		return nil, err
 	}
 
-	for i := range a.Runs.Steps {
-		step := &a.Runs.Steps[i]
-		if step.If.Value == "" {
-			step.If.Value = "success()"
-		}
-	}
-
 	return a, nil
 }
diff --git a/pkg/model/planner.go b/pkg/model/planner.go
index d38e836..47c4c24 100644
--- a/pkg/model/planner.go
+++ b/pkg/model/planner.go
@@ -50,43 +50,6 @@ func (r *Run) Job() *Job {
 	return r.Workflow.GetJob(r.JobID)
 }
 
-// Helper function for FixIfstatement
-func FixIfStatement1(val string, lines [][][]byte, l int) (string, error) {
-	if val != "" {
-		line := lines[l-1][0]
-		outcome := regexp.MustCompile(`\s+if:\s+".*".*`).FindSubmatch(line)
-		if outcome != nil {
-			oldLines := regexp.MustCompile(`"(.*?)"`).FindAllSubmatch(line, 2)
-			val = "${{" + string(oldLines[0][1]) + "}}"
-		}
-	}
-	return val, nil
-}
-
-// Fixes faulty if statements from decoder
-func FixIfStatement(content []byte, wr *Workflow) error {
-	jobs := wr.Jobs
-	lines := regexp.MustCompile(".*\n|.+$").FindAllSubmatch(content, -1)
-	for j := range jobs {
-		val, err := FixIfStatement1(jobs[j].If.Value, lines, jobs[j].If.Line)
-		if err != nil {
-			return err
-		}
-		jobs[j].If.Value = val
-		for i := range jobs[j].Steps {
-			val, err = FixIfStatement1(jobs[j].Steps[i].If.Value, lines, jobs[j].Steps[i].If.Line)
-			if err != nil {
-				return err
-			}
-			if val == "" {
-				val = "success()"
-			}
-			jobs[j].Steps[i].If.Value = val
-		}
-	}
-	return nil
-}
-
 type WorkflowFiles struct {
 	workflowFileInfo os.FileInfo
 	dirPath          string
@@ -178,16 +141,6 @@ func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, e
 				f.Close()
 				return nil, errors.WithMessagef(err, "error occurring when resetting io pointer, %s", wf.workflowFileInfo.Name())
 			}
-			log.Debugf("Correcting if statements '%s'", f.Name())
-			content, err := ioutil.ReadFile(filepath.Join(wf.dirPath, wf.workflowFileInfo.Name()))
-			if err != nil {
-				return nil, errors.WithMessagef(err, "error occurring when reading file, %s", wf.workflowFileInfo.Name())
-			}
-
-			err = FixIfStatement(content, workflow)
-			if err != nil {
-				return nil, err
-			}
 
 			workflow.File = wf.workflowFileInfo.Name()
 			if workflow.Name == "" {