Skip to content

Commit

Permalink
Enable golangci-lint checks which were not running (#431)
Browse files Browse the repository at this point in the history
* 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 golangci/golangci-lint#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]: securego/gosec#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
  • Loading branch information
johnboyes committed Feb 10, 2024
1 parent f889af1 commit c1bbcdb
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/reviewdog.yml
Expand Up @@ -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"
Expand Down
2 changes: 0 additions & 2 deletions .golangci.yml
Expand Up @@ -14,8 +14,6 @@ linters:
# enable-all is deprecated, so enable linters individually
enable:
- bodyclose
- deadcode
- depguard
- dogsled
- dupl
- errcheck
Expand Down
32 changes: 21 additions & 11 deletions internal/github/action.go
Expand Up @@ -8,7 +8,6 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"strings"
Expand All @@ -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
Expand Down Expand Up @@ -55,6 +54,7 @@ func (a *Action) CheckLabels(stdout, stderr io.Writer) int {
}

a.outputResult("success")

return 0
}

Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
12 changes: 7 additions & 5 deletions internal/github/action_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion internal/github/pullrequest/labels.go
Expand Up @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions internal/slice/slice.go
Expand Up @@ -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) {
Expand Down

0 comments on commit c1bbcdb

Please sign in to comment.