From c1bbcdbd47e0a9cb85d5da9b99a3d7204ed14a53 Mon Sep 17 00:00:00 2001 From: John Boyes Date: Sat, 10 Feb 2024 15:42:16 +0200 Subject: [PATCH] Enable golangci-lint checks which were not running (#431) * Bump reviewdog golangci-lint action major version This is a necessary precursor to bumping the Go version from 1.21 to 1.22, which will be done in separate PR. * Remove deprecated deadcode linter See https://github.com/golangci/golangci-lint/pull/3125 * Add missing period at end of comment As warned by golangci-lint. * Remove unnecessary trailing whitespace As per golangci-lint warning. * Change case of variable to mixed not snake As per golangci-lint warning. * Fix cuddled statements warned by golangci-lint * Fix return with no blank line before lint warnings * Fix unchecked error return value linting warnings * Exclude line from line length linter warning * Format files with gofumpt to fix linting warnings * Exclude file permissions line from gosec linting The [gosec linter][1] warns by default on [file permissions above 0600][2]. We need the permissions to be 0644 for this line (because it has to be written to), so we exclude it from linting. [1]: https://github.com/securego/gosec [2]: https://github.com/securego/gosec/issues/107 * Remove depguard from Go linting Created #430 to consider reinstating it in the future. * Fix magic number warning by extracting constant * Fix cuddling linter warnings * Add required comment to exported function * Do not use deprecated ioutil package * Remove unused function --- .github/workflows/reviewdog.yml | 2 +- .golangci.yml | 2 -- internal/github/action.go | 32 ++++++++++++++++++--------- internal/github/action_test.go | 12 +++++----- internal/github/pullrequest/labels.go | 3 ++- internal/slice/slice.go | 1 + 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index c179e92d..6313170c 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - - uses: reviewdog/action-golangci-lint@v1 + - uses: reviewdog/action-golangci-lint@v2 with: github_token: ${{ secrets.github_token }} golangci_lint_flags: "-c .golangci.yml" diff --git a/.golangci.yml b/.golangci.yml index 8bad45be..7e7c7cd8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,8 +14,6 @@ linters: # enable-all is deprecated, so enable linters individually enable: - bodyclose - - deadcode - - depguard - dogsled - dupl - errcheck diff --git a/internal/github/action.go b/internal/github/action.go index 5dec0c80..86ed54c6 100644 --- a/internal/github/action.go +++ b/internal/github/action.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "io" - "io/ioutil" "os" "path/filepath" "strings" @@ -17,7 +16,7 @@ import ( "github.com/agilepathway/label-checker/internal/github/pullrequest" ) -// Action encapsulates the Label Checker GitHub Action +// Action encapsulates the Label Checker GitHub Action. type Action struct { Stdout io.Writer // writer to write stdout messages to Stderr io.Writer // writer to write stderr messages to @@ -55,6 +54,7 @@ func (a *Action) CheckLabels(stdout, stderr io.Writer) int { } a.outputResult("success") + return 0 } @@ -64,9 +64,11 @@ func (a *Action) handleFailure() int { a.outputResult("failure") err := errors.New(a.trimTrailingNewLine(a.failMsg)) fmt.Fprintln(a.Stderr, "::error::", err) + if a.allowFailure() { return 0 } + return 1 } @@ -76,23 +78,24 @@ func (a *Action) trimTrailingNewLine(input string) string { type check func([]string, bool) (bool, string) -type specified func() []string - func (a *Action) runCheck(chk check, specified []string, prefixMode bool) { if len(specified) == 0 { return } + if prefixMode && len(specified) > 1 { a.failMsg += "Currently the label checker only supports checking with one prefix, not multiple." + return } + valid, message := chk(specified, prefixMode) + if valid { a.successMsg += message + "\n" } else { a.failMsg += message + "\n" } - } func (a *Action) repositoryOwner() string { @@ -112,23 +115,30 @@ func (a *Action) pullRequestNumber() int { githubEventJSONFile, err := os.Open(filepath.Clean(os.Getenv("GITHUB_EVENT_PATH"))) panic.IfError(err) defer githubEventJSONFile.Close() //nolint - byteValue, _ := ioutil.ReadAll(githubEventJSONFile) + byteValue, _ := io.ReadAll(githubEventJSONFile) panic.IfError(json.Unmarshal(byteValue, &event)) return event.PullRequest.Number } func (a *Action) outputResult(result string) { - label_check_output := fmt.Sprintf("label_check=%s", result) + const UserReadWriteFilePermission = 0o644 + + labelCheckOutput := fmt.Sprintf("label_check=%s", result) gitHubOutputFileName := filepath.Clean(os.Getenv("GITHUB_OUTPUT")) - githubOutputFile, err := os.OpenFile(gitHubOutputFileName, os.O_APPEND|os.O_WRONLY, 0644) + githubOutputFile, err := os.OpenFile(gitHubOutputFileName, os.O_APPEND|os.O_WRONLY, UserReadWriteFilePermission) //nolint:gosec,lll panic.IfError(err) - _, err = githubOutputFile.WriteString(label_check_output) + _, err = githubOutputFile.WriteString(labelCheckOutput) + if err != nil { - githubOutputFile.Close() + closingErr := githubOutputFile.Close() + panic.IfError(err) + panic.IfError(closingErr) } - githubOutputFile.Close() + + err = githubOutputFile.Close() + panic.IfError(err) } func (a *Action) token() string { diff --git a/internal/github/action_test.go b/internal/github/action_test.go index c4e661d7..7831a7d7 100644 --- a/internal/github/action_test.go +++ b/internal/github/action_test.go @@ -14,25 +14,25 @@ import ( "github.com/agilepathway/label-checker/internal/error/panic" ) -//nolint: gochecknoglobals +//nolint:gochecknoglobals var integration = flag.Bool( "integration", false, "Make calls to real external services. Requires INPUT_REPO_TOKEN environment variable.") -//nolint: gochecknoglobals +//nolint:gochecknoglobals var enterpriseCloud = flag.Bool( "enterprise-cloud", false, "Run the label checker against GitHub Enterprise Cloud instead of standard GitHub") -//nolint: gochecknoglobals +//nolint:gochecknoglobals var enterpriseServer = flag.Bool( "enterprise-server", false, "Run the label checker against GitHub Enterprise Server instead of standard GitHub") -// nolint: lll +//nolint:lll const ( EnvGitHubRepository = "GITHUB_REPOSITORY" EnvGitHubEventPath = "GITHUB_EVENT_PATH" @@ -91,7 +91,7 @@ const ( type specifyChecks func() -// nolint: lll, funlen, dupl +//nolint:lll,funlen,dupl func TestLabelChecks(t *testing.T) { tests := map[string]struct { prNumber int @@ -155,6 +155,7 @@ func TestLabelChecks(t *testing.T) { if len(tc.expectedStderr) > 0 { tc.expectedStderr = "::error:: " + tc.expectedStderr } + setPullRequestNumber(tc.prNumber) setPrefixMode(tc.prefixMode) tc.specifyChecks() @@ -269,6 +270,7 @@ func checkLabels() (int, *bytes.Buffer, *bytes.Buffer) { stdout := &bytes.Buffer{} stderr := &bytes.Buffer{} a := Action{} + return a.CheckLabels(stdout, stderr), stdout, stderr } diff --git a/internal/github/pullrequest/labels.go b/internal/github/pullrequest/labels.go index c6544afb..5ed45142 100644 --- a/internal/github/pullrequest/labels.go +++ b/internal/github/pullrequest/labels.go @@ -27,8 +27,9 @@ func (l Labels) HasNoneOf(specified []string, prefixMode bool) (bool, string) { // all of the specified labels, along with a report describing the result. func (l Labels) HasAllOf(specified []string, prefixMode bool) (bool, string) { if prefixMode { - return false, "The label checker does not support prefix checking with `all_of`, as that is not a logical combination." + return false, "The label checker does not support prefix checking with `all_of`, as that is not a logical combination." //nolint:lll } + return l.hasXof(specified, "all", prefixMode) } diff --git a/internal/slice/slice.go b/internal/slice/slice.go index f002404a..fb11fa7b 100644 --- a/internal/slice/slice.go +++ b/internal/slice/slice.go @@ -18,6 +18,7 @@ func Contains(slice []string, val string) bool { return false } +// StartsWithAnyOf returns true if the given slice starts with any of the given prefixes func StartsWithAnyOf(prefixes []string, candidate string) bool { for _, prefix := range prefixes { if strings.HasPrefix(candidate, prefix) {