From 859d0d06aff1ed0a921549c6420478913f7665b3 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Mon, 10 Jul 2023 12:57:31 -0400 Subject: [PATCH] linting & minor refactoring Signed-off-by: Dave Henderson --- .golangci.yml | 71 ++++++++++++++++++++ codeowners.go | 6 +- codeowners_test.go | 161 ++++++++++++++++++++------------------------- 3 files changed, 146 insertions(+), 92 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..d6305b3 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,71 @@ +linters-settings: + govet: + check-shadowing: true + enable: + - fieldalignment + golint: + min-confidence: 0 + gocyclo: + min-complexity: 10 + dupl: + threshold: 100 + goconst: + min-len: 2 + min-occurrences: 4 + lll: + line-length: 140 + nolintlint: + allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) + allow-unused: false # report any unused nolint directives + require-explanation: false # don't require an explanation for nolint directives + require-specific: false # don't require nolint directives to be specific about which linter is being skipped + +linters: + disable-all: true + enable: + - asciicheck + - bodyclose + # - dogsled + - dupl + - errcheck + - exhaustive + - exportloopref + - funlen + - gci + - gochecknoglobals + - gochecknoinits + - gocognit + - goconst + - gocritic + - gocyclo + - godox + - gofmt + # - gofumpt + - goheader + - goimports + # - gomnd + - gomodguard + - goprintffuncname + - gosec + - gosimple + - govet + - ineffassign + - lll + - misspell + - nakedret + - nestif + # - nlreturn + - noctx + - nolintlint + - prealloc + - revive + - rowserrcheck + - sqlclosecheck + - staticcheck + - stylecheck + - typecheck + - unconvert + - unparam + - unused + - whitespace + # - wsl diff --git a/codeowners.go b/codeowners.go index a6ce481..f9daca0 100644 --- a/codeowners.go +++ b/codeowners.go @@ -129,7 +129,7 @@ func parseCodeowners(r io.Reader) []Codeowner { if len(fields) > 0 && strings.HasPrefix(fields[0], "#") { continue } - if len(fields) > 0 && strings.HasPrefix(fields[0], "[") && strings.HasSuffix(fields[len(fields) -1], "]") { + if len(fields) > 0 && strings.HasPrefix(fields[0], "[") && strings.HasSuffix(fields[len(fields)-1], "]") { continue } if len(fields) > 1 { @@ -219,9 +219,9 @@ func getPattern(line string) *regexp.Regexp { line = regexp.MustCompile(`\*`).ReplaceAllString(line, `([^/]*)`) // Handle escaping the "?" char - line = strings.Replace(line, "?", `\?`, -1) + line = strings.ReplaceAll(line, "?", `\?`) - line = strings.Replace(line, magicStar, "*", -1) + line = strings.ReplaceAll(line, magicStar, "*") // Temporary regex var expr = "" diff --git a/codeowners_test.go b/codeowners_test.go index 4c8ca23..51744ae 100644 --- a/codeowners_test.go +++ b/codeowners_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" ) +//nolint:gochecknoglobals var ( sample = `# comment * @everyone @@ -29,6 +30,56 @@ docs/** @org/docteam @joe` [test2][2] */foo @everyoneelse` + // based on https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners#codeowners-syntax + // with a few unimportant modifications + fullSample = `# This is a comment. +# Each line is a file pattern followed by one or more owners. + +# These owners will be the default owners for everything in +# the repo. Unless a later match takes precedence, +# @global-owner1 and @global-owner2 will be requested for +# review when someone opens a pull request. +* @global-owner1 @global-owner2 + +# Order is important; the last matching pattern takes the most +# precedence. When someone opens a pull request that only +# modifies JS files, only @js-owner and not the global +# owner(s) will be requested for a review. +*.js @js-owner + +# You can also use email addresses if you prefer. They'll be +# used to look up users just like we do for commit author +# emails. +*.go docs@example.com + +# In this example, @doctocat owns any files in the build/logs +# directory at the root of the repository and any of its +# subdirectories. +/build/logs/ @doctocat + +# The 'docs/*' pattern will match files like +# 'docs/getting-started.md' but not further nested files like +# 'docs/build-app/troubleshooting.md'. +docs/* docs@example.com + +# In this example, @octocat owns any file in an apps directory +# anywhere in your repository. +apps/ @octocat + +# In this example, @doctocat owns any file in the '/docs' +# directory in the root of your repository. +/docs/ @doctocat + + foobar/ @fooowner + +\#foo/ @hashowner + +docs/*.md @mdowner + +# this example tests an escaped space in the path +space/test\ space/ @spaceowner +` + codeowners []Codeowner ) @@ -112,56 +163,8 @@ func co(pattern string, owners []string) Codeowner { func TestFullParseCodeowners(t *testing.T) { t.Parallel() - // based on https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners#codeowners-syntax - // with a few unimportant modifications - example := `# This is a comment. -# Each line is a file pattern followed by one or more owners. - -# These owners will be the default owners for everything in -# the repo. Unless a later match takes precedence, -# @global-owner1 and @global-owner2 will be requested for -# review when someone opens a pull request. -* @global-owner1 @global-owner2 - -# Order is important; the last matching pattern takes the most -# precedence. When someone opens a pull request that only -# modifies JS files, only @js-owner and not the global -# owner(s) will be requested for a review. -*.js @js-owner - -# You can also use email addresses if you prefer. They'll be -# used to look up users just like we do for commit author -# emails. -*.go docs@example.com - -# In this example, @doctocat owns any files in the build/logs -# directory at the root of the repository and any of its -# subdirectories. -/build/logs/ @doctocat - -# The 'docs/*' pattern will match files like -# 'docs/getting-started.md' but not further nested files like -# 'docs/build-app/troubleshooting.md'. -docs/* docs@example.com - -# In this example, @octocat owns any file in an apps directory -# anywhere in your repository. -apps/ @octocat - -# In this example, @doctocat owns any file in the '/docs' -# directory in the root of your repository. -/docs/ @doctocat - - foobar/ @fooowner - -\#foo/ @hashowner - -docs/*.md @mdowner -# this example tests an escaped space in the path -space/test\ space/ @spaceowner -` - c := parseCodeowners(strings.NewReader(example)) + c := parseCodeowners(strings.NewReader(fullSample)) codeowners := &Codeowners{ repoRoot: "/build", Patterns: c, @@ -172,51 +175,31 @@ space/test\ space/ @spaceowner path string owners []string }{ - {"#foo/bar.go", - []string{"@hashowner"}}, - {"foobar/baz.go", - []string{"@fooowner"}}, - {"/docs/README.md", - []string{"@mdowner"}}, + {"#foo/bar.go", []string{"@hashowner"}}, + {"foobar/baz.go", []string{"@fooowner"}}, + {"/docs/README.md", []string{"@mdowner"}}, // XXX: uncertain about this one - {"blah/docs/README.md", - []string{"docs@example.com"}}, - {"foo.txt", - []string{"@global-owner1", "@global-owner2"}}, - {"foo/bar.txt", - []string{"@global-owner1", "@global-owner2"}}, - {"foo.js", - []string{"@js-owner"}}, - {"foo/bar.js", - []string{"@js-owner"}}, - {"foo.go", - []string{"docs@example.com"}}, - {"foo/bar.go", - []string{"docs@example.com"}}, + {"blah/docs/README.md", []string{"docs@example.com"}}, + {"foo.txt", []string{"@global-owner1", "@global-owner2"}}, + {"foo/bar.txt", []string{"@global-owner1", "@global-owner2"}}, + {"foo.js", []string{"@js-owner"}}, + {"foo/bar.js", []string{"@js-owner"}}, + {"foo.go", []string{"docs@example.com"}}, + {"foo/bar.go", []string{"docs@example.com"}}, // relative to root - {"build/logs/foo.go", - []string{"@doctocat"}}, - {"build/logs/foo/bar.go", - []string{"@doctocat"}}, + {"build/logs/foo.go", []string{"@doctocat"}}, + {"build/logs/foo/bar.go", []string{"@doctocat"}}, // not relative to root - {"foo/build/logs/foo.go", - []string{"docs@example.com"}}, + {"foo/build/logs/foo.go", []string{"docs@example.com"}}, // docs anywhere - {"foo/docs/foo.js", - []string{"docs@example.com"}}, - {"foo/bar/docs/foo.js", - []string{"docs@example.com"}}, + {"foo/docs/foo.js", []string{"docs@example.com"}}, + {"foo/bar/docs/foo.js", []string{"docs@example.com"}}, // but not nested - {"foo/bar/docs/foo/foo.js", - []string{"@js-owner"}}, - {"foo/apps/foo.js", - []string{"@octocat"}}, - {"docs/foo.js", - []string{"@doctocat"}}, - {"/docs/foo.js", - []string{"@doctocat"}}, - {"/space/test space/doc1.txt", - []string{"@spaceowner"}}, + {"foo/bar/docs/foo/foo.js", []string{"@js-owner"}}, + {"foo/apps/foo.js", []string{"@octocat"}}, + {"docs/foo.js", []string{"@doctocat"}}, + {"/docs/foo.js", []string{"@doctocat"}}, + {"/space/test space/doc1.txt", []string{"@spaceowner"}}, } for _, d := range data {