improve linting

9 files changed, 161 insertions, 68 deletions

diff --git a/.golangci.yml b/.golangci.yml
new file mode 100644
index 0000000..085539b
--- /dev/null
+++ b/.golangci.yml
@@ -0,0 +1,22 @@
+linters-settings:
+  gocyclo:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 10
+  gocritic:
+     disabled-checks:
+      - ifElseChain
+
+linters:
+  enable:
+  - megacheck
+  - govet
+  - golint
+  - gocyclo
+  - gosec
+  - unconvert
+  - dupl
+  - maligned
+  - nakedret
+  - prealloc
+  - scopelint
+  - gocritic
\ No newline at end of file
diff --git a/Makefile b/Makefile
index 1c4cfc2..310ff96 100644
--- a/Makefile
+++ b/Makefile
@@ -15,15 +15,11 @@ TAG_VERSION = v$(VERSION)
 default: check
 
 deps:
-	@GO111MODULE=off go get honnef.co/go/tools/cmd/staticcheck
-	@GO111MODULE=off go get golang.org/x/lint/golint
-	@GO111MODULE=off go get github.com/fzipp/gocyclo
+	curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $$(go env GOPATH)/bin v1.12.5
+
 
 check:
-	go vet ./...
-	golint -set_exit_status ./...
-	staticcheck ./...
-	gocyclo -over 10 .
+	golangci-lint run
 	go test -cover ./...
 
 build: deps check
diff --git a/actions/parser.go b/actions/parser.go
index c787172..f6eb25f 100644
--- a/actions/parser.go
+++ b/actions/parser.go
@@ -2,11 +2,11 @@ package actions
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/hashicorp/hcl"
 	"github.com/hashicorp/hcl/hcl/ast"
@@ -32,7 +32,10 @@ func ParseWorkflows(workingDir string, workflowPath string) (Workflows, error) {
 	}
 
 	buf := new(bytes.Buffer)
-	buf.ReadFrom(workflowReader)
+	_, err = buf.ReadFrom(workflowReader)
+	if err != nil {
+		log.Error(err)
+	}
 
 	workflows := new(workflowsFile)
 	workflows.WorkingDir = workingDir
@@ -70,20 +73,19 @@ func cleanWorkflowsAST(node ast.Node) (ast.Node, bool) {
 		case "args", "runs":
 			if literalType, ok := objectItem.Val.(*ast.LiteralType); ok {
 				listType := new(ast.ListType)
-				parts := strings.Split(literalType.Token.Value().(string), " ")
-				log.Debugf("got list: %v", parts)
-				if len(parts) > 0 {
-					quote := literalType.Token.Text[0]
-					for _, part := range parts {
-						part = fmt.Sprintf("%c%s%c", quote, part, quote)
-						log.Debugf("Adding part %s", part)
-						listType.Add(&ast.LiteralType{
-							Token: token.Token{
-								Type: token.STRING,
-								Text: part,
-							},
-						})
-					}
+				parts, err := parseCommand(literalType.Token.Value().(string))
+				if err != nil {
+					return nil, false
+				}
+				quote := literalType.Token.Text[0]
+				for _, part := range parts {
+					part = fmt.Sprintf("%c%s%c", quote, part, quote)
+					listType.Add(&ast.LiteralType{
+						Token: token.Token{
+							Type: token.STRING,
+							Text: part,
+						},
+					})
 				}
 				objectItem.Val = listType
 
@@ -99,3 +101,64 @@ func cleanWorkflowsAST(node ast.Node) (ast.Node, bool) {
 	}
 	return node, true
 }
+
+// reused from: https://github.com/laurent22/massren/blob/ae4c57da1e09a95d9383f7eb645a9f69790dec6c/main.go#L172
+// nolint: gocyclo
+func parseCommand(cmd string) ([]string, error) {
+	var args []string
+	state := "start"
+	current := ""
+	quote := "\""
+	for i := 0; i < len(cmd); i++ {
+		c := cmd[i]
+
+		if state == "quotes" {
+			if string(c) != quote {
+				current += string(c)
+			} else {
+				args = append(args, current)
+				current = ""
+				state = "start"
+			}
+			continue
+		}
+
+		if c == '"' || c == '\'' {
+			state = "quotes"
+			quote = string(c)
+			continue
+		}
+
+		if state == "arg" {
+			if c == ' ' || c == '\t' {
+				args = append(args, current)
+				current = ""
+				state = "start"
+			} else {
+				current += string(c)
+			}
+			continue
+		}
+
+		if c != ' ' && c != '\t' {
+			state = "arg"
+			current += string(c)
+		}
+	}
+
+	if state == "quotes" {
+		return []string{}, fmt.Errorf("unclosed quote in command line: %s", cmd)
+	}
+
+	if current != "" {
+		args = append(args, current)
+	}
+
+	if len(args) == 0 {
+		return []string{}, errors.New("empty command line")
+	}
+
+	log.Debugf("Parsed literal %+q to list %+q", cmd, args)
+
+	return args, nil
+}
diff --git a/actions/runner.go b/actions/runner.go
index acc8a6b..862c2f2 100644
--- a/actions/runner.go
+++ b/actions/runner.go
@@ -22,10 +22,10 @@ import (
 
 var secretCache map[string]string
 
-func (w *workflowsFile) ListEvents() []string {
+func (wFile *workflowsFile) ListEvents() []string {
 	log.Debugf("Listing all events")
 	events := make([]string, 0)
-	for _, w := range w.Workflow {
+	for _, w := range wFile.Workflow {
 		events = append(events, w.On)
 	}
 
@@ -37,53 +37,55 @@ func (w *workflowsFile) ListEvents() []string {
 	return events
 }
 
-func (w *workflowsFile) GraphEvent(eventName string) ([][]string, error) {
+func (wFile *workflowsFile) GraphEvent(eventName string) ([][]string, error) {
 	log.Debugf("Listing actions for event '%s'", eventName)
-	workflow, _, err := w.getWorkflow(eventName)
+	workflow, _, err := wFile.getWorkflow(eventName)
 	if err != nil {
 		return nil, err
 	}
-	return w.newExecutionGraph(workflow.Resolves...), nil
+	return wFile.newExecutionGraph(workflow.Resolves...), nil
 }
 
-func (w *workflowsFile) RunAction(ctx context.Context, dryrun bool, actionName string) error {
+func (wFile *workflowsFile) RunAction(ctx context.Context, dryrun bool, actionName string) error {
 	log.Debugf("Running action '%s'", actionName)
-	return w.newActionExecutor(ctx, dryrun, "", actionName)()
+	return wFile.newActionExecutor(ctx, dryrun, "", actionName)()
 }
 
-func (w *workflowsFile) RunEvent(ctx context.Context, dryrun bool, eventName string) error {
+func (wFile *workflowsFile) RunEvent(ctx context.Context, dryrun bool, eventName string) error {
 	log.Debugf("Running event '%s'", eventName)
-	workflow, _, err := w.getWorkflow(eventName)
+	workflow, _, err := wFile.getWorkflow(eventName)
 	if err != nil {
 		return err
 	}
 
 	log.Debugf("Running actions %s -> %s", eventName, workflow.Resolves)
-	return w.newActionExecutor(ctx, dryrun, eventName, workflow.Resolves...)()
+	return wFile.newActionExecutor(ctx, dryrun, eventName, workflow.Resolves...)()
 }
 
-func (w *workflowsFile) getWorkflow(eventName string) (*workflowDef, string, error) {
-	for wName, w := range w.Workflow {
+func (wFile *workflowsFile) getWorkflow(eventName string) (*workflowDef, string, error) {
+	var rtn workflowDef
+	for wName, w := range wFile.Workflow {
 		if w.On == eventName {
-			return &w, wName, nil
+			rtn = w
+			return &rtn, wName, nil
 		}
 	}
 	return nil, "", fmt.Errorf("unsupported event: %v", eventName)
 }
 
-func (w *workflowsFile) getAction(actionName string) (*actionDef, error) {
-	if a, ok := w.Action[actionName]; ok {
+func (wFile *workflowsFile) getAction(actionName string) (*actionDef, error) {
+	if a, ok := wFile.Action[actionName]; ok {
 		return &a, nil
 	}
 	return nil, fmt.Errorf("unsupported action: %v", actionName)
 }
 
-func (w *workflowsFile) Close() {
-	os.RemoveAll(w.TempDir)
+func (wFile *workflowsFile) Close() {
+	os.RemoveAll(wFile.TempDir)
 }
 
 // return a pipeline that is run in series.  pipeline is a list of steps to run in parallel
-func (w *workflowsFile) newExecutionGraph(actionNames ...string) [][]string {
+func (wFile *workflowsFile) newExecutionGraph(actionNames ...string) [][]string {
 	// first, build a list of all the necessary actions to run, and their dependencies
 	actionDependencies := make(map[string][]string)
 	for len(actionNames) > 0 {
@@ -91,8 +93,8 @@ func (w *workflowsFile) newExecutionGraph(actionNames ...string) [][]string {
 		for _, aName := range actionNames {
 			// make sure we haven't visited this action yet
 			if _, ok := actionDependencies[aName]; !ok {
-				actionDependencies[aName] = w.Action[aName].Needs
-				newActionNames = append(newActionNames, w.Action[aName].Needs...)
+				actionDependencies[aName] = wFile.Action[aName].Needs
+				newActionNames = append(newActionNames, wFile.Action[aName].Needs...)
 			}
 		}
 		actionNames = newActionNames
@@ -136,18 +138,18 @@ func listInLists(srcList []string, searchLists ...[]string) bool {
 	return true
 }
 
-func (w *workflowsFile) newActionExecutor(ctx context.Context, dryrun bool, eventName string, actionNames ...string) common.Executor {
-	graph := w.newExecutionGraph(actionNames...)
+func (wFile *workflowsFile) newActionExecutor(ctx context.Context, dryrun bool, eventName string, actionNames ...string) common.Executor {
+	graph := wFile.newExecutionGraph(actionNames...)
 
 	pipeline := make([]common.Executor, 0)
 	for _, actions := range graph {
 		stage := make([]common.Executor, 0)
 		for _, actionName := range actions {
-			action, err := w.getAction(actionName)
+			action, err := wFile.getAction(actionName)
 			if err != nil {
 				return common.NewErrorExecutor(err)
 			}
-			actionExecutor := action.asExecutor(ctx, dryrun, w.WorkingDir, w.TempDir, actionName, w.setupEnvironment(eventName, actionName, dryrun))
+			actionExecutor := action.asExecutor(ctx, dryrun, wFile.WorkingDir, wFile.TempDir, actionName, wFile.setupEnvironment(eventName, actionName, dryrun))
 			stage = append(stage, actionExecutor)
 		}
 		pipeline = append(pipeline, common.NewParallelExecutor(stage...))
@@ -273,11 +275,11 @@ func (action *actionDef) createGithubTarball() (io.Reader, error) {
 
 }
 
-func (w *workflowsFile) setupEnvironment(eventName string, actionName string, dryrun bool) []string {
+func (wFile *workflowsFile) setupEnvironment(eventName string, actionName string, dryrun bool) []string {
 	env := make([]string, 0)
-	repoPath := w.WorkingDir
+	repoPath := wFile.WorkingDir
 
-	_, workflowName, _ := w.getWorkflow(eventName)
+	_, workflowName, _ := wFile.getWorkflow(eventName)
 
 	env = append(env, fmt.Sprintf("HOME=/github/home"))
 	env = append(env, fmt.Sprintf("GITHUB_ACTOR=nektos/act"))
@@ -308,7 +310,7 @@ func (w *workflowsFile) setupEnvironment(eventName string, actionName string, dr
 		env = append(env, fmt.Sprintf("GITHUB_REF=refs/heads/%s", branch))
 	}
 
-	action, err := w.getAction(actionName)
+	action, err := wFile.getAction(actionName)
 	if err == nil && !dryrun {
 		action.applyEnvironmentSecrets(&env)
 	}
diff --git a/common/executor_test.go b/common/executor_test.go
index 73b0ea6..9ed6015 100644
--- a/common/executor_test.go
+++ b/common/executor_test.go
@@ -22,11 +22,11 @@ func TestNewWorkflow(t *testing.T) {
 	runcount := 0
 	successWorkflow := NewPipelineExecutor(
 		func() error {
-			runcount = runcount + 1
+			runcount ++
 			return nil
 		},
 		func() error {
-			runcount = runcount + 1
+			runcount ++
 			return nil
 		})
 	assert.Nil(successWorkflow())
diff --git a/common/file.go b/common/file.go
index f2fbfd5..268b272 100644
--- a/common/file.go
+++ b/common/file.go
@@ -75,5 +75,5 @@ func CopyDir(source string, dest string) (err error) {
 		}
 
 	}
-	return
+	return err
 }
diff --git a/common/git.go b/common/git.go
index c9154b1..2c8efc2 100644
--- a/common/git.go
+++ b/common/git.go
@@ -39,7 +39,7 @@ func FindGitRevision(file string) (shortSha string, sha string, err error) {
 	if err != nil {
 		return "", "", err
 	}
-	return string(string(refBuf)[:7]), string(refBuf), nil
+	return string(refBuf[:7]), string(refBuf), nil
 }
 
 // FindGitBranch get the current git branch
@@ -72,9 +72,15 @@ func findGitHead(file string) (string, error) {
 	}()
 
 	headBuffer := new(bytes.Buffer)
-	headBuffer.ReadFrom(bufio.NewReader(headFile))
+	_, err = headBuffer.ReadFrom(bufio.NewReader(headFile))
+	if err != nil {
+		log.Error(err)
+	}
 	head := make(map[string]string)
-	yaml.Unmarshal(headBuffer.Bytes(), head)
+	err = yaml.Unmarshal(headBuffer.Bytes(), head)
+	if err != nil {
+		log.Error(err)
+	}
 
 	log.Debugf("HEAD points to '%s'", head["ref"])
 
@@ -204,9 +210,12 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) Executor {
 			return err
 		}
 
-		w.Pull(&git.PullOptions{
+		err = w.Pull(&git.PullOptions{
 			ReferenceName: refName,
 		})
+		if err != nil {
+			input.Logger.Errorf("Unable to pull %s: %v", refName, err)
+		}
 		input.Logger.Debugf("Cloned %s to %s", input.URL.String(), input.Dir)
 
 		err = w.Checkout(&git.CheckoutOptions{
diff --git a/container/docker_common.go b/container/docker_common.go
index 10b0e3e..311f1f5 100644
--- a/container/docker_common.go
+++ b/container/docker_common.go
@@ -29,18 +29,17 @@ type dockerMessage struct {
 	Progress string `json:"progress"`
 }
 
-func (i *DockerExecutorInput) logDockerOutput(dockerResponse io.Reader) error {
+func (i *DockerExecutorInput) logDockerOutput(dockerResponse io.Reader) {
 	scanner := bufio.NewScanner(dockerResponse)
 	if i.Logger == nil {
-		return nil
+		return 
 	}
 	for scanner.Scan() {
 		i.Logger.Infof(scanner.Text())
 	}
-	return nil
 }
 
-func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) error {
+func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) {
 	out := os.Stdout
 	go func() {
 		<-i.Ctx.Done()
@@ -48,7 +47,9 @@ func (i *DockerExecutorInput) streamDockerOutput(dockerResponse io.Reader) error
 	}()
 
 	_, err := io.Copy(out, dockerResponse)
-	return err
+	if err != nil {
+		i.Logger.Error(err)
+	}
 }
 
 func (i *DockerExecutorInput) writeLog(isError bool, format string, args ...interface{}) {
@@ -63,9 +64,9 @@ func (i *DockerExecutorInput) writeLog(isError bool, format string, args ...inte
 
 }
 
-func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) error {
+func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, isError bool) {
 	if dockerResponse == nil {
-		return nil
+		return 
 	}
 	defer dockerResponse.Close()
 
@@ -81,7 +82,8 @@ func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, is
 		msg.Progress = ""
 		if err := json.Unmarshal(line, &msg); err == nil {
 			if msg.Error != "" {
-				return fmt.Errorf("%s", msg.Error)
+				i.writeLog(isError, "%s", msg.Error)
+				return 
 			}
 
 			if msg.Status != "" {
@@ -100,5 +102,4 @@ func (i *DockerExecutorInput) logDockerResponse(dockerResponse io.ReadCloser, is
 		}
 	}
 
-	return nil
 }
diff --git a/container/docker_run.go b/container/docker_run.go
index 4c12ae4..3350276 100644
--- a/container/docker_run.go
+++ b/container/docker_run.go
@@ -32,7 +32,7 @@ type NewDockerRunExecutorInput struct {
 func NewDockerRunExecutor(input NewDockerRunExecutorInput) common.Executor {
 	return func() error {
 
-		input.Logger.Infof("docker run %s %s %s", input.Image, input.Entrypoint, input.Cmd)
+		input.Logger.Infof("docker run image=%s entrypoint=%+q cmd=%+q", input.Image, input.Entrypoint, input.Cmd)
 		if input.Dryrun {
 			return nil
 		}