Merge pull request 'fix: return an error when the argument count is wrong' (#59) from earl-warren/act:wip-arg-count into main

Reviewed-on: https://code.forgejo.org/forgejo/act/pulls/59
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>

.forgejo/workflows/test.yml

@@ -1,7 +1,9 @@
 name: checks
 on:
-  - push
-  - pull_request
+  push:
+    branches:
+      - 'main'
+  pull_request:
 
 env:
   GOPROXY: https://goproxy.io,direct
@@ -42,5 +44,5 @@ jobs:
       - name: build without docker
         run: go build -tags WITHOUT_DOCKER -v ./...
       - name: test
-        run: go test -v ./pkg/jobparser
+        run: go test -v ./pkg/jobparser ./pkg/model ./pkg/exprparser
         # TODO test more packages

pkg/container/docker_network.go

@@ -22,7 +22,6 @@ func NewDockerNetworkCreateExecutor(name string, config *types.NetworkCreate) co
 		if err != nil {
 			return err
 		}
-		common.Logger(ctx).Debugf("%v", networks)
 		for _, network := range networks {
 			if network.Name == name {
 				common.Logger(ctx).Debugf("Network %v exists", name)

pkg/exprparser/functions_test.go

@@ -43,6 +43,9 @@ func TestFunctionContains(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("contains('one')", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionStartsWith(t *testing.T) {
@@ -72,6 +75,9 @@ func TestFunctionStartsWith(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("startsWith('one')", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionEndsWith(t *testing.T) {
@@ -101,6 +107,9 @@ func TestFunctionEndsWith(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("endsWith('one')", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionJoin(t *testing.T) {
@@ -128,6 +137,9 @@ func TestFunctionJoin(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("join()", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionToJSON(t *testing.T) {
@@ -154,6 +166,9 @@ func TestFunctionToJSON(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("tojson()", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionFromJSON(t *testing.T) {
@@ -177,6 +192,9 @@ func TestFunctionFromJSON(t *testing.T) {
 			assert.Equal(t, tt.expected, output)
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("fromjson()", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }
 
 func TestFunctionHashFiles(t *testing.T) {
@@ -248,4 +266,7 @@ func TestFunctionFormat(t *testing.T) {
 			}
 		})
 	}
+
+	_, err := NewInterpeter(env, Config{}).Evaluate("format()", DefaultStatusCheckNone)
+	assert.Error(t, err)
 }

pkg/exprparser/interpreter.go

@@ -593,23 +593,58 @@ func (impl *interperterImpl) evaluateFuncCall(funcCallNode *actionlint.FuncCallN
 		args = append(args, reflect.ValueOf(value))
 	}
 
+	argCountCheck := func(argCount int) error {
+		if len(args) != argCount {
+			return fmt.Errorf("'%s' expected %d arguments but got %d instead", funcCallNode.Callee, argCount, len(args))
+		}
+		return nil
+	}
+
+	argAtLeastCheck := func(atLeast int) error {
+		if len(args) < atLeast {
+			return fmt.Errorf("'%s' expected at least %d arguments but got %d instead", funcCallNode.Callee, atLeast, len(args))
+		}
+		return nil
+	}
+
 	switch strings.ToLower(funcCallNode.Callee) {
 	case "contains":
+		if err := argCountCheck(2); err != nil {
+			return nil, err
+		}
 		return impl.contains(args[0], args[1])
 	case "startswith":
+		if err := argCountCheck(2); err != nil {
+			return nil, err
+		}
 		return impl.startsWith(args[0], args[1])
 	case "endswith":
+		if err := argCountCheck(2); err != nil {
+			return nil, err
+		}
 		return impl.endsWith(args[0], args[1])
 	case "format":
+		if err := argAtLeastCheck(1); err != nil {
+			return nil, err
+		}
 		return impl.format(args[0], args[1:]...)
 	case "join":
+		if err := argAtLeastCheck(1); err != nil {
+			return nil, err
+		}
 		if len(args) == 1 {
 			return impl.join(args[0], reflect.ValueOf(","))
 		}
 		return impl.join(args[0], args[1])
 	case "tojson":
+		if err := argCountCheck(1); err != nil {
+			return nil, err
+		}
 		return impl.toJSON(args[0])
 	case "fromjson":
+		if err := argCountCheck(1); err != nil {
+			return nil, err
+		}
 		return impl.fromJSON(args[0])
 	case "hashfiles":
 		if impl.env.HashFiles != nil {

pkg/model/workflow.go

@@ -749,7 +749,7 @@ func (w *Workflow) GetJobIDs() []string {
 }
 
 var OnDecodeNodeError = func(node yaml.Node, out interface{}, err error) {
-	log.Fatalf("Failed to decode node %v into %T: %v", node, out, err)
+	log.Errorf("Failed to decode node %v into %T: %v", node, out, err)
 }
 
 func decodeNode(node yaml.Node, out interface{}) bool {

pkg/model/workflow_test.go

@@ -153,6 +153,25 @@ jobs:
 	assert.Contains(t, workflow.On(), "pull_request")
 }
 
+func TestReadWorkflow_DecodeNodeError(t *testing.T) {
+	yaml := `
+on:
+  push:
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo
+        env:
+          foo: {{ a }}
+`
+
+	workflow, err := ReadWorkflow(strings.NewReader(yaml))
+	assert.NoError(t, err, "read workflow should succeed")
+	assert.Nil(t, workflow.GetJob("test").Steps[0].GetEnv())
+}
+
 func TestReadWorkflow_RunsOnLabels(t *testing.T) {
 	yaml := `
 name: local-action-docker-url

renovate.json

@@ -2,6 +2,11 @@
   "$schema": "https://docs.renovatebot.com/renovate-schema.json",
   "extends": ["local>forgejo/renovate-config"],
   "packageRules": [
+    {
+      "description": "Disable runner test data",
+      "matchFileNames": ["pkg/runner/testdata/**"],
+      "enabled": false
+    },
     {
       "description": "Require approval for all dependencies",
       "matchDepNames": ["/.+/"],