From 724ec918c92c162e537ba879e6d1ccc997f63483 Mon Sep 17 00:00:00 2001 From: Casey Lee Date: Mon, 10 Jul 2023 17:12:12 -0700 Subject: [PATCH] chore: upgrade golangci-lint and address findings (#1904) --- .github/workflows/checks.yml | 3 ++- .golangci.yml | 20 +++++++++---------- cmd/root.go | 11 +++++------ pkg/artifacts/server.go | 2 +- pkg/common/git/git.go | 2 +- pkg/container/docker_run.go | 2 +- pkg/container/docker_run_test.go | 2 +- pkg/container/host_environment.go | 14 +++++++------- pkg/container/util.go | 2 +- pkg/exprparser/interpreter.go | 2 +- pkg/runner/action.go | 2 +- pkg/runner/command.go | 2 +- pkg/runner/expression.go | 6 +++--- pkg/runner/job_executor.go | 4 ++-- pkg/runner/job_executor_test.go | 2 +- pkg/runner/run_context.go | 28 +++++++++++++-------------- pkg/runner/step.go | 2 +- pkg/runner/step_action_local.go | 2 +- pkg/runner/step_action_local_test.go | 2 +- pkg/runner/step_action_remote_test.go | 2 +- pkg/runner/step_docker.go | 2 +- pkg/runner/step_run.go | 2 +- 22 files changed, 57 insertions(+), 59 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index ca035aa..40f060f 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -25,7 +25,8 @@ jobs: check-latest: true - uses: golangci/golangci-lint-action@v3.6.0 with: - version: v1.47.2 + version: v1.53 + only-new-issues: true - uses: megalinter/megalinter/flavors/go@v7.1.0 env: DEFAULT_BRANCH: master diff --git a/.golangci.yml b/.golangci.yml index d31aa34..4082864 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,17 +19,15 @@ linters-settings: - pkg: 'github.com/stretchr/testify/assert' alias: assert depguard: - list-type: blacklist - include-go-root: true - packages: - - github.com/pkg/errors - - gotest.tools/v3/assert - - log - packages-with-error-message: - - github.com/pkg/errors: 'Please use "errors" package from standard library' - - gotest.tools/v3: 'Please keep tests unified using only github.com/stretchr/testify' - - log: 'Please keep logging unified using only github.com/sirupsen/logrus' - + rules: + main: + deny: + - pkg: github.com/pkg/errors + desc: Please use "errors" package from standard library + - pkg: gotest.tools/v3 + desc: Please keep tests unified using only github.com/stretchr/testify + - pkg: log + desc: Please keep logging unified using only github.com/sirupsen/logrus linters: enable: - megacheck diff --git a/cmd/root.go b/cmd/root.go index 59a571e..f7b6515 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -273,7 +273,7 @@ func readArgsFile(file string, split bool) []string { return args } -func setup(inputs *Input) func(*cobra.Command, []string) { +func setup(_ *Input) func(*cobra.Command, []string) { return func(cmd *cobra.Command, _ []string) { verbose, _ := cmd.Flags().GetBool("verbose") if verbose { @@ -343,12 +343,11 @@ func parseMatrix(matrix []string) map[string]map[string]bool { matrix := r.Split(m, 2) if len(matrix) < 2 { log.Fatalf("Invalid matrix format. Failed to parse %s", m) - } else { - if _, ok := matrixes[matrix[0]]; !ok { - matrixes[matrix[0]] = make(map[string]bool) - } - matrixes[matrix[0]][matrix[1]] = true } + if _, ok := matrixes[matrix[0]]; !ok { + matrixes[matrix[0]] = make(map[string]bool) + } + matrixes[matrix[0]][matrix[1]] = true } return matrixes } diff --git a/pkg/artifacts/server.go b/pkg/artifacts/server.go index daaacf6..4b88ea4 100644 --- a/pkg/artifacts/server.go +++ b/pkg/artifacts/server.go @@ -79,7 +79,7 @@ func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) { return nil, err } - _, err = file.Seek(0, os.SEEK_END) + _, err = file.Seek(0, io.SeekEnd) if err != nil { return nil, err } diff --git a/pkg/common/git/git.go b/pkg/common/git/git.go index 954c2cc..bf77155 100644 --- a/pkg/common/git/git.go +++ b/pkg/common/git/git.go @@ -174,7 +174,7 @@ func FindGithubRepo(ctx context.Context, file, githubInstance, remoteName string return slug, err } -func findGitRemoteURL(ctx context.Context, file, remoteName string) (string, error) { +func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error) { repo, err := git.PlainOpenWithOptions( file, &git.PlainOpenOptions{ diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 65a373f..0abbf56 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -604,7 +604,7 @@ func (cr *containerReference) tryReadGID() common.Executor { return cr.tryReadID("-g", func(id int) { cr.GID = id }) } -func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal bool, resp types.HijackedResponse, idResp types.IDResponse, user string, workdir string) error { +func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal bool, resp types.HijackedResponse, _ types.IDResponse, _ string, _ string) error { logger := common.Logger(ctx) cmdResponse := make(chan error) diff --git a/pkg/container/docker_run_test.go b/pkg/container/docker_run_test.go index bc3ab4b..8309df6 100644 --- a/pkg/container/docker_run_test.go +++ b/pkg/container/docker_run_test.go @@ -78,7 +78,7 @@ type endlessReader struct { io.Reader } -func (r endlessReader) Read(p []byte) (n int, err error) { +func (r endlessReader) Read(_ []byte) (n int, err error) { return 1, nil } diff --git a/pkg/container/host_environment.go b/pkg/container/host_environment.go index c6a2228..a547db1 100644 --- a/pkg/container/host_environment.go +++ b/pkg/container/host_environment.go @@ -34,7 +34,7 @@ type HostEnvironment struct { StdOut io.Writer } -func (e *HostEnvironment) Create(capAdd []string, capDrop []string) common.Executor { +func (e *HostEnvironment) Create(_ []string, _ []string) common.Executor { return func(ctx context.Context) error { return nil } @@ -140,13 +140,13 @@ func (e *HostEnvironment) GetContainerArchive(ctx context.Context, srcPath strin return io.NopCloser(buf), nil } -func (e *HostEnvironment) Pull(forcePull bool) common.Executor { +func (e *HostEnvironment) Pull(_ bool) common.Executor { return func(ctx context.Context) error { return nil } } -func (e *HostEnvironment) Start(attach bool) common.Executor { +func (e *HostEnvironment) Start(_ bool) common.Executor { return func(ctx context.Context) error { return nil } @@ -240,7 +240,7 @@ func copyPtyOutput(writer io.Writer, ppty io.Reader, finishLog context.CancelFun } } -func (e *HostEnvironment) UpdateFromImageEnv(env *map[string]string) common.Executor { +func (e *HostEnvironment) UpdateFromImageEnv(_ *map[string]string) common.Executor { return func(ctx context.Context) error { return nil } @@ -254,7 +254,7 @@ func getEnvListFromMap(env map[string]string) []string { return envList } -func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, user, workdir string) error { +func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, _, workdir string) error { envList := getEnvListFromMap(env) var wd string if workdir != "" { @@ -411,7 +411,7 @@ func goOsToActionOs(os string) string { return os } -func (e *HostEnvironment) GetRunnerContext(ctx context.Context) map[string]interface{} { +func (e *HostEnvironment) GetRunnerContext(_ context.Context) map[string]interface{} { return map[string]interface{}{ "os": goOsToActionOs(runtime.GOOS), "arch": goArchToActionArch(runtime.GOARCH), @@ -420,7 +420,7 @@ func (e *HostEnvironment) GetRunnerContext(ctx context.Context) map[string]inter } } -func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, stderr io.Writer) (io.Writer, io.Writer) { +func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, _ io.Writer) (io.Writer, io.Writer) { org := e.StdOut e.StdOut = stdout return org, org diff --git a/pkg/container/util.go b/pkg/container/util.go index eb7f46c..96143a5 100644 --- a/pkg/container/util.go +++ b/pkg/container/util.go @@ -9,7 +9,7 @@ import ( "github.com/creack/pty" ) -func getSysProcAttr(cmdLine string, tty bool) *syscall.SysProcAttr { +func getSysProcAttr(_ string, tty bool) *syscall.SysProcAttr { if tty { return &syscall.SysProcAttr{ Setsid: true, diff --git a/pkg/exprparser/interpreter.go b/pkg/exprparser/interpreter.go index e2e2ca5..8bf4e57 100644 --- a/pkg/exprparser/interpreter.go +++ b/pkg/exprparser/interpreter.go @@ -149,7 +149,7 @@ func (impl *interperterImpl) evaluateNode(exprNode actionlint.ExprNode) (interfa } } -// nolint:gocyclo +//nolint:gocyclo func (impl *interperterImpl) evaluateVariable(variableNode *actionlint.VariableNode) (interface{}, error) { switch strings.ToLower(variableNode.Name) { case "github": diff --git a/pkg/runner/action.go b/pkg/runner/action.go index bd74b2e..4d6b8fc 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -182,7 +182,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } } -func setupActionEnv(ctx context.Context, step actionStep, remoteAction *remoteAction) error { +func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error { rc := step.getRunContext() // A few fields in the environment (e.g. GITHUB_ACTION_REPOSITORY) diff --git a/pkg/runner/command.go b/pkg/runner/command.go index e0abb64..d79ac03 100644 --- a/pkg/runner/command.go +++ b/pkg/runner/command.go @@ -171,7 +171,7 @@ func unescapeKvPairs(kvPairs map[string]string) map[string]string { return kvPairs } -func (rc *RunContext) saveState(ctx context.Context, kvPairs map[string]string, arg string) { +func (rc *RunContext) saveState(_ context.Context, kvPairs map[string]string, arg string) { stepID := rc.CurrentStep if stepID != "" { if rc.IntraActionState == nil { diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index 24b16f8..23c76ab 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -181,7 +181,7 @@ func (ee expressionEvaluator) evaluateScalarYamlNode(ctx context.Context, node * } func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) { - var ret *yaml.Node = nil + var ret *yaml.Node // GitHub has this undocumented feature to merge maps, called insert directive insertDirective := regexp.MustCompile(`\${{\s*insert\s*}}`) for i := 0; i < len(node.Content)/2; i++ { @@ -239,7 +239,7 @@ func (ee expressionEvaluator) evaluateMappingYamlNode(ctx context.Context, node } func (ee expressionEvaluator) evaluateSequenceYamlNode(ctx context.Context, node *yaml.Node) (*yaml.Node, error) { - var ret *yaml.Node = nil + var ret *yaml.Node for i := 0; i < len(node.Content); i++ { v := node.Content[i] // Preserve nested sequences @@ -503,6 +503,6 @@ func getWorkflowSecrets(ctx context.Context, rc *RunContext) map[string]string { return rc.Config.Secrets } -func getWorkflowVars(ctx context.Context, rc *RunContext) map[string]string { +func getWorkflowVars(_ context.Context, rc *RunContext) map[string]string { return rc.Config.Vars } diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index 2f98ada..3f2e41e 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -101,7 +101,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo // always allow 1 min for stopping and removing the runner, even if we were cancelled ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) defer cancel() - err = info.stopContainer()(ctx) + err = info.stopContainer()(ctx) //nolint:contextcheck } setJobResult(ctx, info, rc, jobError == nil) setJobOutputs(ctx, rc) @@ -114,7 +114,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo pipeline = append(pipeline, steps...) return common.NewPipelineExecutor(info.startContainer(), common.NewPipelineExecutor(pipeline...). - Finally(func(ctx context.Context) error { + Finally(func(ctx context.Context) error { //nolint:contextcheck var cancel context.CancelFunc if ctx.Err() == context.Canceled { // in case of an aborted run, we still should execute the diff --git a/pkg/runner/job_executor_test.go b/pkg/runner/job_executor_test.go index 87c5888..ac7725f 100644 --- a/pkg/runner/job_executor_test.go +++ b/pkg/runner/job_executor_test.go @@ -82,7 +82,7 @@ type jobContainerMock struct { container.LinuxContainerEnvironmentExtensions } -func (jcm *jobContainerMock) ReplaceLogWriter(stdout, stderr io.Writer) (io.Writer, io.Writer) { +func (jcm *jobContainerMock) ReplaceLogWriter(_, _ io.Writer) (io.Writer, io.Writer) { return nil, nil } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 6dd52b0..fbc20a5 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -515,7 +515,7 @@ func (rc *RunContext) platformImage(ctx context.Context) string { return rc.runsOnImage(ctx) } -func (rc *RunContext) options(ctx context.Context) string { +func (rc *RunContext) options(_ context.Context) string { job := rc.Run.Job() c := job.Container() if c == nil { @@ -813,35 +813,35 @@ func setActionRuntimeVars(rc *RunContext, env map[string]string) { env["ACTIONS_RUNTIME_TOKEN"] = actionsRuntimeToken } -func (rc *RunContext) handleCredentials(ctx context.Context) (username, password string, err error) { +func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, error) { // TODO: remove below 2 lines when we can release act with breaking changes - username = rc.Config.Secrets["DOCKER_USERNAME"] - password = rc.Config.Secrets["DOCKER_PASSWORD"] + username := rc.Config.Secrets["DOCKER_USERNAME"] + password := rc.Config.Secrets["DOCKER_PASSWORD"] container := rc.Run.Job().Container() if container == nil || container.Credentials == nil { - return + return username, password, nil } if container.Credentials != nil && len(container.Credentials) != 2 { - err = fmt.Errorf("invalid property count for key 'credentials:'") - return + err := fmt.Errorf("invalid property count for key 'credentials:'") + return "", "", err } ee := rc.NewExpressionEvaluator(ctx) if username = ee.Interpolate(ctx, container.Credentials["username"]); username == "" { - err = fmt.Errorf("failed to interpolate container.credentials.username") - return + err := fmt.Errorf("failed to interpolate container.credentials.username") + return "", "", err } if password = ee.Interpolate(ctx, container.Credentials["password"]); password == "" { - err = fmt.Errorf("failed to interpolate container.credentials.password") - return + err := fmt.Errorf("failed to interpolate container.credentials.password") + return "", "", err } if container.Credentials["username"] == "" || container.Credentials["password"] == "" { - err = fmt.Errorf("container.credentials cannot be empty") - return + err := fmt.Errorf("container.credentials cannot be empty") + return "", "", err } - return username, password, err + return username, password, nil } diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 9cc6aea..ffb2efb 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -256,7 +256,7 @@ func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage) return runStep, nil } -func isContinueOnError(ctx context.Context, expr string, step step, stage stepStage) (bool, error) { +func isContinueOnError(ctx context.Context, expr string, step step, _ stepStage) (bool, error) { // https://github.com/github/docs/blob/3ae84420bd10997bb5f35f629ebb7160fe776eae/content/actions/reference/workflow-syntax-for-github-actions.md?plain=true#L962 if len(strings.TrimSpace(expr)) == 0 { return false, nil diff --git a/pkg/runner/step_action_local.go b/pkg/runner/step_action_local.go index 6b4fc06..a745e68 100644 --- a/pkg/runner/step_action_local.go +++ b/pkg/runner/step_action_local.go @@ -85,7 +85,7 @@ func (sal *stepActionLocal) getEnv() *map[string]string { return &sal.env } -func (sal *stepActionLocal) getIfExpression(context context.Context, stage stepStage) string { +func (sal *stepActionLocal) getIfExpression(_ context.Context, stage stepStage) string { switch stage { case stepStageMain: return sal.Step.If.Value diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go index 5fe7f29..c4b6345 100644 --- a/pkg/runner/step_action_local_test.go +++ b/pkg/runner/step_action_local_test.go @@ -24,7 +24,7 @@ func (salm *stepActionLocalMocks) runAction(step actionStep, actionDir string, r return args.Get(0).(func(context.Context) error) } -func (salm *stepActionLocalMocks) readAction(ctx context.Context, step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) { +func (salm *stepActionLocalMocks) readAction(_ context.Context, step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) { args := salm.Called(step, actionDir, actionPath, readFile, writeFile) return args.Get(0).(*model.Action), args.Error(1) } diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go index dfc49d2..ec1028a 100644 --- a/pkg/runner/step_action_remote_test.go +++ b/pkg/runner/step_action_remote_test.go @@ -21,7 +21,7 @@ type stepActionRemoteMocks struct { mock.Mock } -func (sarm *stepActionRemoteMocks) readAction(ctx context.Context, step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) { +func (sarm *stepActionRemoteMocks) readAction(_ context.Context, step *model.Step, actionDir string, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) { args := sarm.Called(step, actionDir, actionPath, readFile, writeFile) return args.Get(0).(*model.Action), args.Error(1) } diff --git a/pkg/runner/step_docker.go b/pkg/runner/step_docker.go index c955b16..1c7e39e 100644 --- a/pkg/runner/step_docker.go +++ b/pkg/runner/step_docker.go @@ -51,7 +51,7 @@ func (sd *stepDocker) getEnv() *map[string]string { return &sd.env } -func (sd *stepDocker) getIfExpression(context context.Context, stage stepStage) string { +func (sd *stepDocker) getIfExpression(_ context.Context, _ stepStage) string { return sd.Step.If.Value } diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index 4d855fd..9d887e7 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -59,7 +59,7 @@ func (sr *stepRun) getEnv() *map[string]string { return &sr.env } -func (sr *stepRun) getIfExpression(context context.Context, stage stepStage) string { +func (sr *stepRun) getIfExpression(_ context.Context, _ stepStage) string { return sr.Step.If.Value }