diff --git a/pkg/common/executor.go b/pkg/common/executor.go index cd92a6c..5e9b28d 100644 --- a/pkg/common/executor.go +++ b/pkg/common/executor.go @@ -136,7 +136,6 @@ func (e Executor) Then(then Executor) Executor { case Warning: log.Warning(err.Error()) default: - log.Debugf("%+v", err) return err } } diff --git a/pkg/exprparser/interpreter.go b/pkg/exprparser/interpreter.go index 584999b..df73fb4 100644 --- a/pkg/exprparser/interpreter.go +++ b/pkg/exprparser/interpreter.go @@ -274,7 +274,7 @@ func (impl *interperterImpl) evaluateNot(notNode *actionlint.NotOpNode) (interfa return nil, err } - return !impl.isTruthy(reflect.ValueOf(operand)), nil + return !IsTruthy(operand), nil } func (impl *interperterImpl) evaluateCompare(compareNode *actionlint.CompareOpNode) (interface{}, error) { @@ -434,7 +434,8 @@ func (impl *interperterImpl) compareNumber(left float64, right float64, kind act } } -func (impl *interperterImpl) isTruthy(value reflect.Value) bool { +func IsTruthy(input interface{}) bool { + value := reflect.ValueOf(input) switch value.Kind() { case reflect.Bool: return value.Bool() @@ -452,10 +453,7 @@ func (impl *interperterImpl) isTruthy(value reflect.Value) bool { return value.Float() != 0 - case reflect.Map: - return true - - case reflect.Slice: + case reflect.Map, reflect.Slice: return true default: @@ -503,14 +501,14 @@ func (impl *interperterImpl) evaluateLogicalCompare(compareNode *actionlint.Logi switch compareNode.Kind { case actionlint.LogicalOpNodeKindAnd: - if impl.isTruthy(leftValue) { + if IsTruthy(left) { return impl.getSafeValue(rightValue), nil } return impl.getSafeValue(leftValue), nil case actionlint.LogicalOpNodeKindOr: - if impl.isTruthy(leftValue) { + if IsTruthy(left) { return impl.getSafeValue(leftValue), nil } diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index b19e8bd..5be4f9e 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -2,7 +2,6 @@ package runner import ( "fmt" - "math" "regexp" "strings" @@ -128,7 +127,9 @@ type expressionEvaluator struct { } func (ee expressionEvaluator) evaluate(in string, isIfExpression bool) (interface{}, error) { + log.Debugf("evaluating expression '%s'", in) evaluated, err := ee.interpreter.Evaluate(in, isIfExpression) + log.Debugf("expression '%s' evaluated to '%t'", in, evaluated) return evaluated, err } @@ -141,9 +142,6 @@ func (ee expressionEvaluator) evaluateScalarYamlNode(node *yaml.Node) error { return nil } expr, _ := rewriteSubExpression(in, false) - if in != expr { - log.Debugf("expression '%s' rewritten to '%s'", in, expr) - } res, err := ee.evaluate(expr, false) if err != nil { return err @@ -214,18 +212,12 @@ func (ee expressionEvaluator) Interpolate(in string) string { } expr, _ := rewriteSubExpression(in, true) - if in != expr { - log.Debugf("expression '%s' rewritten to '%s'", in, expr) - } - evaluated, err := ee.evaluate(expr, false) if err != nil { log.Errorf("Unable to interpolate expression '%s': %s", expr, err) return "" } - log.Debugf("expression '%s' evaluated to '%s'", expr, evaluated) - value, ok := evaluated.(string) if !ok { panic(fmt.Sprintf("Expression %s did not evaluate to a string", expr)) @@ -237,37 +229,13 @@ func (ee expressionEvaluator) Interpolate(in string) string { // EvalBool evaluates an expression against given evaluator func EvalBool(evaluator ExpressionEvaluator, expr string) (bool, error) { nextExpr, _ := rewriteSubExpression(expr, false) - if expr != nextExpr { - log.Debugf("expression '%s' rewritten to '%s'", expr, nextExpr) - } evaluated, err := evaluator.evaluate(nextExpr, true) if err != nil { return false, err } - var result bool - - switch t := evaluated.(type) { - case bool: - result = t - case string: - result = t != "" - case int: - result = t != 0 - case float64: - if math.IsNaN(t) { - result = false - } else { - result = t != 0 - } - default: - return false, fmt.Errorf("Unable to map return type to boolean for '%s'", expr) - } - - log.Debugf("expression '%s' evaluated to '%t'", nextExpr, result) - - return result, nil + return exprparser.IsTruthy(evaluated), nil } func escapeFormatString(in string) string { @@ -334,5 +302,9 @@ func rewriteSubExpression(in string, forceFormat bool) (string, error) { return in, nil } - return fmt.Sprintf("format('%s', %s)", strings.ReplaceAll(formatOut, "'", "''"), strings.Join(results, ", ")), nil + out := fmt.Sprintf("format('%s', %s)", strings.ReplaceAll(formatOut, "'", "''"), strings.Join(results, ", ")) + if in != out { + log.Debugf("expression '%s' rewritten to '%s'", in, out) + } + return out, nil } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index abe0716..2131e96 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -275,7 +275,18 @@ func (rc *RunContext) steps() []*model.Step { // Executor returns a pipeline executor for all the steps in the job func (rc *RunContext) Executor() common.Executor { - return newJobExecutor(rc).If(rc.isEnabled) + return func(ctx context.Context) error { + isEnabled, err := rc.isEnabled(ctx) + if err != nil { + return err + } + + if isEnabled { + return newJobExecutor(rc)(ctx) + } + + return nil + } } // Executor returns a pipeline executor for all the steps in the job @@ -405,17 +416,16 @@ func (rc *RunContext) hostname() string { return *hostname } -func (rc *RunContext) isEnabled(ctx context.Context) bool { +func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { job := rc.Run.Job() l := common.Logger(ctx) runJob, err := EvalBool(rc.ExprEval, job.If.Value) if err != nil { - common.Logger(ctx).Errorf(" \u274C Error in if: expression - %s", job.Name) - return false + return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, err) } if !runJob { l.Debugf("Skipping job '%s' due to '%s'", job.Name, job.If.Value) - return false + return false, nil } img := rc.platformImage() @@ -428,9 +438,9 @@ func (rc *RunContext) isEnabled(ctx context.Context) bool { platformName := rc.ExprEval.Interpolate(runnerLabel) l.Infof("\U0001F6A7 Skipping unsupported platform -- Try running with `-P %+v=...`", platformName) } - return false + return false, nil } - return true + return true, nil } func mergeMaps(maps ...map[string]string) map[string]string { diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index b9de160..51ce945 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -171,13 +171,7 @@ func (sc *StepContext) interpolateEnv(exprEval ExpressionEvaluator) { func (sc *StepContext) isEnabled(ctx context.Context) (bool, error) { runStep, err := EvalBool(sc.NewExpressionEvaluator(), sc.Step.If.Value) if err != nil { - common.Logger(ctx).Errorf(" \u274C Error in if: expression - %s", sc.Step) - exprEval, err := sc.setupEnv(ctx) - if err != nil { - return false, err - } - sc.RunContext.ExprEval = exprEval - return false, err + return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", sc.Step.If.Value, err) } return runStep, nil diff --git a/pkg/runner/testdata/issue-597/spelling.yaml b/pkg/runner/testdata/issue-597/spelling.yaml index 280bdbf..d594dcb 100644 --- a/pkg/runner/testdata/issue-597/spelling.yaml +++ b/pkg/runner/testdata/issue-597/spelling.yaml @@ -1,10 +1,10 @@ name: issue-597 on: push - + jobs: my_first_job: - + runs-on: ubuntu-latest steps: - name: My first false step @@ -14,7 +14,7 @@ jobs: ref: refs/pull/${{github.event.pull_request.number}}/merge fetch-depth: 5 - name: My first true step - if: ${{"endsWith('Hello world', 'ld')"}} + if: ${{endsWith('Hello world', 'ld')}} uses: actions/hello-world-javascript-action@main with: who-to-greet: "Renst the Octocat" diff --git a/pkg/runner/testdata/uses-composite/push.yml b/pkg/runner/testdata/uses-composite/push.yml index 598c3b4..bc66aff 100755 --- a/pkg/runner/testdata/uses-composite/push.yml +++ b/pkg/runner/testdata/uses-composite/push.yml @@ -17,7 +17,7 @@ jobs: secret_input: ${{secrets.test_input_optional || 'NO SUCH SECRET'}} env: secret_input: ${{secrets.test_input_optional || 'NO SUCH SECRET'}} - - if: steps.composite.outputs.test_output != "test_output_value" + - if: steps.composite.outputs.test_output != 'test_output_value' run: | echo "steps.composite.outputs.test_output=${{ steps.composite.outputs.test_output }}" exit 1 @@ -38,7 +38,7 @@ jobs: test_input_optional_with_default_overriden: 'test_input_optional_with_default_overriden' test_input_required_with_default_overriden: 'test_input_required_with_default_overriden' - - if: steps.composite2.outputs.test_output != "test_output_value" + - if: steps.composite2.outputs.test_output != 'test_output_value' run: | echo "steps.composite.outputs.test_output=${{ steps.composite2.outputs.test_output }}" exit 1