From 6650fb3b19dac19fb514ef845f6d897ea808d8f9 Mon Sep 17 00:00:00 2001 From: Daniel <dhaavi@users.noreply.github.com> Date: Wed, 2 Feb 2022 14:58:27 +0100 Subject: [PATCH] Update linter settings and fix warnings --- .github/workflows/go.yml | 9 ++++++++- .golangci.yml | 20 ++++++++++++++++++++ api/client/websocket.go | 2 +- api/database.go | 2 +- api/main_test.go | 2 +- config/set_test.go | 10 +++++----- database/database_test.go | 2 +- database/query/parser.go | 2 +- database/record/meta-gencode.go | 9 --------- database/storage/badger/badger_test.go | 4 +++- database/storage/bbolt/bbolt_test.go | 4 +++- formats/dsd/gencode_test.go | 12 ------------ metrics/api.go | 4 +++- modules/subsystems/subsystems_test.go | 2 +- rng/test/main.go | 10 +++++----- template/module_test.go | 2 +- test | 9 --------- updater/fetch.go | 12 ++++++++---- updater/file.go | 4 +++- updater/registry_test.go | 2 +- updater/unpacking.go | 12 +++++++++--- utils/atomic.go | 4 +++- utils/debug/debug.go | 16 ++++++++-------- utils/renameio/tempfile.go | 12 +++++++----- utils/renameio/tempfile_linux_test.go | 12 ++++++------ utils/renameio/writefile_test.go | 4 +++- 26 files changed, 102 insertions(+), 81 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index b7b83ab..35826da 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -54,7 +54,14 @@ jobs: - name: Run golint run: ./golint -set_exit_status -min_confidence 1.0 ./... - # TODO: Enable gofmt again when all environments are up to date. + # golint is run (sufficiently; with excludes) as a part of golangci-lint. + # - name: Install golint + # run: bash -c "GOBIN=$(pwd) go get -u golang.org/x/lint/golint" + # + # - name: Run golint + # run: ./golint -set_exit_status -min_confidence 1.0 ./... + + # gofmt is run (sufficiently; with excludes) as a part of golangci-lint. # - name: Run gofmt # run: bash -c 'test -z "$(gofmt -s -l .)"' diff --git a/.golangci.yml b/.golangci.yml index 6cdc8ff..b4c851b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,6 +33,9 @@ linters: - wsl linters-settings: + revive: + # See https://github.com/mgechev/revive#available-rules for details. + enable-all-rules: true gci: # put imports beginning with prefix after 3rd-party packages; # only support one prefix @@ -43,3 +46,20 @@ linters-settings: # might be left in the code accidentally and should be resolved before merging keywords: - FIXME + gosec: + # To specify a set of rules to explicitly exclude. + # Available rules: https://github.com/securego/gosec#available-rules + excludes: + - G204 # Variables in commands. + - G304 # Variables in file paths. + - G505 # We need crypto/sha1 for non-security stuff. Using `nolint:` triggers another linter. + +issues: + exclude-use-default: false + exclude-rules: + - text: "a blank import .*" + linters: + - golint + - text: "ST1000: at least one file in a package should have a package comment.*" + linters: + - stylecheck diff --git a/api/client/websocket.go b/api/client/websocket.go index f8e2b2a..622cadf 100644 --- a/api/client/websocket.go +++ b/api/client/websocket.go @@ -41,7 +41,7 @@ func (c *Client) wsConnect() error { case <-c.shutdownSignal: state.Error("") } - state.wsConn.Close() + _ = state.wsConn.Close() state.wg.Wait() return nil diff --git a/api/database.go b/api/database.go index 5087dda..970dc38 100644 --- a/api/database.go +++ b/api/database.go @@ -251,7 +251,7 @@ func (api *DatabaseAPI) shutdown(err error) error { // Trigger shutdown. close(api.shutdownSignal) - api.conn.Close() + _ = api.conn.Close() return nil } diff --git a/api/main_test.go b/api/main_test.go index 8ca9089..078f4b6 100644 --- a/api/main_test.go +++ b/api/main_test.go @@ -52,6 +52,6 @@ func TestMain(m *testing.M) { fmt.Fprintf(os.Stderr, "failed to cleanly shutdown test: %s\n", err) } // clean up and exit - os.RemoveAll(tmpDir) + _ = os.RemoveAll(tmpDir) os.Exit(exitCode) } diff --git a/config/set_test.go b/config/set_test.go index 3c9037e..7d74ee2 100644 --- a/config/set_test.go +++ b/config/set_test.go @@ -1,4 +1,4 @@ -//nolint:goconst,errcheck +//nolint:goconst package config import "testing" @@ -83,7 +83,7 @@ func TestLayersSetters(t *testing.T) { //nolint:paralleltest // reset options = make(map[string]*Option) - Register(&Option{ + _ = Register(&Option{ Name: "name", Key: "monkey", Description: "description", @@ -93,7 +93,7 @@ func TestLayersSetters(t *testing.T) { //nolint:paralleltest DefaultValue: "banana", ValidationRegex: "^(banana|water)$", }) - Register(&Option{ + _ = Register(&Option{ Name: "name", Key: "zebras/zebra", Description: "description", @@ -103,7 +103,7 @@ func TestLayersSetters(t *testing.T) { //nolint:paralleltest DefaultValue: []string{"black", "white"}, ValidationRegex: "^[a-z]+$", }) - Register(&Option{ + _ = Register(&Option{ Name: "name", Key: "elephant", Description: "description", @@ -113,7 +113,7 @@ func TestLayersSetters(t *testing.T) { //nolint:paralleltest DefaultValue: 2, ValidationRegex: "", }) - Register(&Option{ + _ = Register(&Option{ Name: "name", Key: "hot", Description: "description", diff --git a/database/database_test.go b/database/database_test.go index b2d0225..33cb2df 100644 --- a/database/database_test.go +++ b/database/database_test.go @@ -36,7 +36,7 @@ func TestMain(m *testing.M) { // Clean up the test directory. // Do not defer, as we end this function with a os.Exit call. - os.RemoveAll(testDir) + _ = os.RemoveAll(testDir) os.Exit(exitCode) } diff --git a/database/query/parser.go b/database/query/parser.go index ff28ccf..988ea59 100644 --- a/database/query/parser.go +++ b/database/query/parser.go @@ -207,7 +207,7 @@ func parseAndOr(getSnippet func() (*snippet, error), remainingSnippets func() in for { if !expectingMore && rootCondition && remainingSnippets() == 0 { // advance snippetsPos by one, as it will be set back by 1 - getSnippet() //nolint:errcheck + _, _ = getSnippet() if len(conditions) == 1 { return conditions[0], nil } diff --git a/database/record/meta-gencode.go b/database/record/meta-gencode.go index 98c2ce5..180e7fa 100644 --- a/database/record/meta-gencode.go +++ b/database/record/meta-gencode.go @@ -2,15 +2,6 @@ package record import ( "fmt" - "io" - "time" - "unsafe" -) - -var ( - _ = unsafe.Sizeof(0) - _ = io.ReadFull - _ = time.Now() ) // GenCodeSize returns the size of the gencode marshalled byte slice. diff --git a/database/storage/badger/badger_test.go b/database/storage/badger/badger_test.go index 898e4ac..9cff49f 100644 --- a/database/storage/badger/badger_test.go +++ b/database/storage/badger/badger_test.go @@ -45,7 +45,9 @@ func TestBadger(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(testDir) // clean up + defer func() { + _ = os.RemoveAll(testDir) // clean up + }() // start db, err := NewBadger("test", testDir) diff --git a/database/storage/bbolt/bbolt_test.go b/database/storage/bbolt/bbolt_test.go index f11b965..5e9b302 100644 --- a/database/storage/bbolt/bbolt_test.go +++ b/database/storage/bbolt/bbolt_test.go @@ -47,7 +47,9 @@ func TestBBolt(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(testDir) // clean up + defer func() { + _ = os.RemoveAll(testDir) // clean up + }() // start db, err := NewBBolt("test", testDir) diff --git a/formats/dsd/gencode_test.go b/formats/dsd/gencode_test.go index 1957961..2fbf18a 100644 --- a/formats/dsd/gencode_test.go +++ b/formats/dsd/gencode_test.go @@ -1,18 +1,6 @@ //nolint:nakedret,unconvert,gocognit,wastedassign,gofumpt package dsd -import ( - "io" - "time" - "unsafe" -) - -var ( - _ = unsafe.Sizeof(0) - _ = io.ReadFull - _ = time.Now() -) - func (d *SimpleTestStruct) Size() (s uint64) { { diff --git a/metrics/api.go b/metrics/api.go index a83e195..974140e 100644 --- a/metrics/api.go +++ b/metrics/api.go @@ -101,7 +101,9 @@ func writeMetricsTo(ctx context.Context, url string) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() // Check return status. if resp.StatusCode >= 200 && resp.StatusCode <= 299 { diff --git a/modules/subsystems/subsystems_test.go b/modules/subsystems/subsystems_test.go index 23949f2..ab632e5 100644 --- a/modules/subsystems/subsystems_test.go +++ b/modules/subsystems/subsystems_test.go @@ -120,5 +120,5 @@ func TestSubsystems(t *testing.T) { //nolint:paralleltest // Too much interferen } // clean up and exit - os.RemoveAll(tmpDir) + _ = os.RemoveAll(tmpDir) } diff --git a/rng/test/main.go b/rng/test/main.go index ad5d7be..acaabf6 100644 --- a/rng/test/main.go +++ b/rng/test/main.go @@ -58,7 +58,7 @@ func prep() error { } var err error - outputFile, err = os.OpenFile(os.Args[2], os.O_CREATE|os.O_WRONLY, 0o0644) + outputFile, err = os.OpenFile(os.Args[2], os.O_CREATE|os.O_WRONLY, 0o0644) //nolint:gosec if err != nil { return fmt.Errorf("failed to open output file: %w", err) } @@ -106,13 +106,13 @@ func fortuna(_ context.Context) error { bytesWritten += 64 if bytesWritten%1024 == 0 { - os.Stderr.WriteString(".") + _, _ = os.Stderr.WriteString(".") } if bytesWritten%65536 == 0 { fmt.Fprintf(os.Stderr, "\n%d bytes written\n", bytesWritten) } if bytesWritten >= outputSize { - os.Stderr.WriteString("\n") + _, _ = os.Stderr.WriteString("\n") break } } @@ -145,7 +145,7 @@ func tickfeeder(ctx context.Context) error { } bytesWritten += 8 if bytesWritten%1024 == 0 { - os.Stderr.WriteString(".") + _, _ = os.Stderr.WriteString(".") } if bytesWritten%65536 == 0 { fmt.Fprintf(os.Stderr, "\n%d bytes written\n", bytesWritten) @@ -154,7 +154,7 @@ func tickfeeder(ctx context.Context) error { } if bytesWritten >= outputSize { - os.Stderr.WriteString("\n") + _, _ = os.Stderr.WriteString("\n") break } } diff --git a/template/module_test.go b/template/module_test.go index 6e76eb9..30c9bb8 100644 --- a/template/module_test.go +++ b/template/module_test.go @@ -50,6 +50,6 @@ func TestMain(m *testing.M) { fmt.Fprintf(os.Stderr, "failed to cleanly shutdown test: %s\n", err) } // clean up and exit - os.RemoveAll(tmpDir) + _ = os.RemoveAll(tmpDir) os.Exit(exitCode) } diff --git a/test b/test index fd0f3ee..a9afaf0 100755 --- a/test +++ b/test @@ -104,8 +104,6 @@ fi # install if [[ $install -eq 1 ]]; then echo "installing dependencies..." - echo "$ go get -u golang.org/x/lint/golint" - go get -u golang.org/x/lint/golint # TODO: update golangci-lint version regularly echo "$ curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.44.0" curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.44.0 @@ -122,12 +120,6 @@ if [[ $testonly -eq 0 ]]; then echo "gofmt command not found" exit 1 fi - if [[ $(which golint) == "" ]]; then - echo "golint command not found" - echo "install with: go get -u golang.org/x/lint/golint" - echo "or run: ./test install" - exit 1 - fi if [[ $(which golangci-lint) == "" ]]; then echo "golangci-lint command not found" echo "install with: curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin vX.Y.Z" @@ -160,7 +152,6 @@ for package in $packages; do echo "" echo $package if [[ $testonly -eq 0 ]]; then - run golint -set_exit_status -min_confidence 1.0 $package run go vet $package run golangci-lint run $packagename fi diff --git a/updater/fetch.go b/updater/fetch.go index e6942a9..adad517 100644 --- a/updater/fetch.go +++ b/updater/fetch.go @@ -45,7 +45,9 @@ func (reg *ResourceRegistry) fetchFile(ctx context.Context, client *http.Client, if err != nil { return err } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() // download and write file n, err := io.Copy(atomicFile, resp.Body) @@ -64,7 +66,7 @@ func (reg *ResourceRegistry) fetchFile(ctx context.Context, client *http.Client, // set permissions if !onWindows { // TODO: only set executable files to 0755, set other to 0644 - err = os.Chmod(rv.storagePath(), 0o0755) + err = os.Chmod(rv.storagePath(), 0o0755) //nolint:gosec // See TODO above. if err != nil { log.Warningf("%s: failed to set permissions on downloaded file %s: %s", reg.Name, rv.storagePath(), err) } @@ -89,7 +91,9 @@ func (reg *ResourceRegistry) fetchData(ctx context.Context, client *http.Client, if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + _ = resp.Body.Close() + }() // download and write file buf := bytes.NewBuffer(make([]byte, 0, resp.ContentLength)) @@ -135,7 +139,7 @@ func (reg *ResourceRegistry) makeRequest(ctx context.Context, client *http.Clien // check return code if resp.StatusCode != http.StatusOK { - resp.Body.Close() + _ = resp.Body.Close() return nil, "", fmt.Errorf("failed to fetch %q: %d %s", downloadURL, resp.StatusCode, resp.Status) } diff --git a/updater/file.go b/updater/file.go index b879e3a..8c78978 100644 --- a/updater/file.go +++ b/updater/file.go @@ -92,7 +92,9 @@ func (file *File) Unpack(suffix string, unpacker Unpacker) (string, error) { if err != nil { return "", err } - defer f.Close() + defer func() { + _ = f.Close() + }() r, err := unpacker(f) if err != nil { diff --git a/updater/registry_test.go b/updater/registry_test.go index 2ef1b78..d25ef87 100644 --- a/updater/registry_test.go +++ b/updater/registry_test.go @@ -31,6 +31,6 @@ func TestMain(m *testing.M) { ret := m.Run() // teardown - os.RemoveAll(tmpDir) + _ = os.RemoveAll(tmpDir) os.Exit(ret) } diff --git a/updater/unpacking.go b/updater/unpacking.go index 5b19b86..00283bc 100644 --- a/updater/unpacking.go +++ b/updater/unpacking.go @@ -117,7 +117,9 @@ func (res *Resource) unpackZipArchive() (err error) { if err != nil { return } - defer archiveReader.Close() + defer func() { + _ = archiveReader.Close() + }() // Save all files to the tmp dir. for _, file := range archiveReader.File { @@ -161,14 +163,18 @@ func copyFromZipArchive(archiveFile *zip.File, dstPath string) error { if err != nil { return err } - defer fileReader.Close() + defer func() { + _ = fileReader.Close() + }() // Open destination file for writing. dstFile, err := os.OpenFile(dstPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, archiveFile.Mode()) if err != nil { return err } - defer dstFile.Close() + defer func() { + _ = dstFile.Close() + }() // Copy full file from archive to dst. if _, err := io.CopyN(dstFile, fileReader, MaxUnpackSize); err != nil { diff --git a/utils/atomic.go b/utils/atomic.go index cd1224d..cde4a1a 100644 --- a/utils/atomic.go +++ b/utils/atomic.go @@ -74,7 +74,9 @@ func CopyFileAtomic(dest string, src string, opts *AtomicFileOptions) error { if err != nil { return err } - defer f.Close() + defer func() { + _ = f.Close() + }() return CreateAtomic(dest, f, opts) } diff --git a/utils/debug/debug.go b/utils/debug/debug.go index a28146b..bd74e2b 100644 --- a/utils/debug/debug.go +++ b/utils/debug/debug.go @@ -51,37 +51,37 @@ func addContentLineBreaks(flags InfoFlag) bool { func (di *Info) AddSection(name string, flags InfoFlag, content ...string) { // Check if we need a spacer. if di.Len() > 0 { - di.WriteString("\n\n") + _, _ = di.WriteString("\n\n") } // Write section to buffer. // Write section header. if di.Style == "github" { - di.WriteString(fmt.Sprintf("<details>\n<summary>%s</summary>\n\n", name)) + _, _ = di.WriteString(fmt.Sprintf("<details>\n<summary>%s</summary>\n\n", name)) } else { - di.WriteString(fmt.Sprintf("**%s**:\n\n", name)) + _, _ = di.WriteString(fmt.Sprintf("**%s**:\n\n", name)) } // Write section content. if useCodeSection(flags) { // Write code header: Needs one empty line between previous data. - di.WriteString("```\n") + _, _ = di.WriteString("```\n") } for i, part := range content { - di.WriteString(part) + _, _ = di.WriteString(part) if addContentLineBreaks(flags) && i < len(content)-1 { - di.WriteString("\n") + _, _ = di.WriteString("\n") } } if useCodeSection(flags) { // Write code footer: Needs one empty line between next data. - di.WriteString("\n```\n") + _, _ = di.WriteString("\n```\n") } // Write section header. if di.Style == "github" { - di.WriteString("\n</details>") + _, _ = di.WriteString("\n</details>") } } diff --git a/utils/renameio/tempfile.go b/utils/renameio/tempfile.go index acfad61..8364d64 100644 --- a/utils/renameio/tempfile.go +++ b/utils/renameio/tempfile.go @@ -38,17 +38,19 @@ func tempDir(dir, dest string) string { cleanup := true defer func() { if cleanup { - os.Remove(testsrc.Name()) + _ = os.Remove(testsrc.Name()) } }() - testsrc.Close() + _ = testsrc.Close() testdest, err := ioutil.TempFile(filepath.Dir(dest), "."+filepath.Base(dest)) if err != nil { return fallback } - defer os.Remove(testdest.Name()) - testdest.Close() + defer func() { + _ = os.Remove(testdest.Name()) + }() + _ = testdest.Close() if err := os.Rename(testsrc.Name(), testdest.Name()); err != nil { return fallback @@ -149,7 +151,7 @@ func Symlink(oldname, newname string) error { cleanup := true defer func() { if cleanup { - os.RemoveAll(d) + _ = os.RemoveAll(d) } }() diff --git a/utils/renameio/tempfile_linux_test.go b/utils/renameio/tempfile_linux_test.go index bf569ea..2077709 100644 --- a/utils/renameio/tempfile_linux_test.go +++ b/utils/renameio/tempfile_linux_test.go @@ -15,11 +15,11 @@ func TestTempDir(t *testing.T) { if tmpdir, ok := os.LookupEnv("TMPDIR"); ok { t.Cleanup(func() { - os.Setenv("TMPDIR", tmpdir) // restore + _ = os.Setenv("TMPDIR", tmpdir) // restore }) } else { t.Cleanup(func() { - os.Unsetenv("TMPDIR") // restore + _ = os.Unsetenv("TMPDIR") // restore }) } @@ -28,7 +28,7 @@ func TestTempDir(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { - os.RemoveAll(mount1) + _ = os.RemoveAll(mount1) }) mount2, err := ioutil.TempDir("", "test-renameio-testtempdir2") @@ -36,7 +36,7 @@ func TestTempDir(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { - os.RemoveAll(mount2) + _ = os.RemoveAll(mount2) }) if err := syscall.Mount("tmpfs", mount1, "tmpfs", 0, ""); err != nil { @@ -103,9 +103,9 @@ func TestTempDir(t *testing.T) { t.Parallel() if testCase.TMPDIR == "" { - os.Unsetenv("TMPDIR") + _ = os.Unsetenv("TMPDIR") } else { - os.Setenv("TMPDIR", testCase.TMPDIR) + _ = os.Setenv("TMPDIR", testCase.TMPDIR) } if got := tempDir(testCase.dir, testCase.path); got != testCase.want { diff --git a/utils/renameio/writefile_test.go b/utils/renameio/writefile_test.go index 92872e7..5c04651 100644 --- a/utils/renameio/writefile_test.go +++ b/utils/renameio/writefile_test.go @@ -17,7 +17,9 @@ func TestWriteFile(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.RemoveAll(d) + defer func() { + _ = os.RemoveAll(d) + }() filename := filepath.Join(d, "hello.sh")