From cc3dd491cd5a1b21c0d867a4fb96011759bb1b49 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 29 Jun 2024 03:33:12 +0200 Subject: [PATCH] chore: improve linter configuration --- .github/workflows/linting.yml | 4 ++ .golangci.yml | 83 ++++++++++++++++++++++- flock.go | 3 +- flock_aix.go | 1 - flock_example_test.go | 6 ++ flock_test.go | 123 ++++++++++++++++++---------------- flock_unix.go | 9 ++- flock_winapi.go | 9 +-- flock_windows.go | 8 ++- 9 files changed, 174 insertions(+), 72 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index f78aa98..f963f08 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -25,6 +25,10 @@ jobs: runs-on: ${{ matrix.os }} steps: + - if: ${{ matrix.os == 'windows-latest' }} # https://github.com/actions/checkout/issues/135 + run: | + git config --global core.eol lf + git config --global core.autocrlf input - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: diff --git a/.golangci.yml b/.golangci.yml index e597a34..3ad88a3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,9 +3,42 @@ run: linters: enable: - - misspell + - asasalint + - bidichk + - dogsled + - dupword + - durationcheck + - err113 + - errname + - errorlint + - fatcontext + - forbidigo + - gocheckcompilerdirectives + - gochecknoinits + - gocritic - godot + - godox + - gofumpt - goheader + - goimports + - gomoddirectives + - goprintffuncname + - gosec + - inamedparam + - interfacebloat + - ireturn + - mirror + - misspell + - nolintlint + - revive + - stylecheck + - tenv + - testifylint + - thelper + - unconvert + - unparam + - usestdlibvars + - whitespace linters-settings: misspell: @@ -19,6 +52,54 @@ linters-settings: Copyright 2018-{{ YEAR }} The Gofrs. All rights reserved. Use of this source code is governed by the BSD 3-Clause license that can be found in the LICENSE file. + gofumpt: + extra-rules: true + gocritic: + enabled-tags: + - diagnostic + - style + - performance + disabled-checks: + - paramTypeCombine # already handle by gofumpt.extra-rules + - whyNoLint # already handle by nonolint + - unnamedResult + - hugeParam + - sloppyReassign + - rangeValCopy + - octalLiteral + - ptrToRefParam + - appendAssign + - ruleguard + - httpNoBody + - exposedSyncMutex + + revive: + rules: + - name: struct-tag + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + - name: if-return + - name: increment-decrement + - name: var-naming + - name: var-declaration + - name: package-comments + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: empty-block + - name: superfluous-else + - name: unused-parameter + - name: unreachable-code + - name: redefines-builtin-id issues: exclude-use-default: true diff --git a/flock.go b/flock.go index 49eeca3..3c694f9 100644 --- a/flock.go +++ b/flock.go @@ -126,7 +126,8 @@ func (f *Flock) setFh() error { } else { flags |= os.O_RDONLY } - fh, err := os.OpenFile(f.path, flags, os.FileMode(0600)) + + fh, err := os.OpenFile(f.path, flags, os.FileMode(0o600)) if err != nil { return err } diff --git a/flock_aix.go b/flock_aix.go index fc57d66..134a4a0 100644 --- a/flock_aix.go +++ b/flock_aix.go @@ -153,7 +153,6 @@ func (f *Flock) doLock(cmd cmdType, lt lockType, blocking bool) (bool, error) { } err = setlkw(f.fh.Fd(), cmd, lt) - if err != nil { f.doUnlock() if cmd == tryLock && err == unix.EACCES { diff --git a/flock_example_test.go b/flock_example_test.go index 1e34b99..3a1ed47 100644 --- a/flock_example_test.go +++ b/flock_example_test.go @@ -20,6 +20,7 @@ func ExampleFlock_Locked() { _, err := f.TryLock() if err != nil { // handle locking error + panic(err) } fmt.Printf("locked: %v\n", f.Locked()) @@ -27,6 +28,7 @@ func ExampleFlock_Locked() { err = f.Unlock() if err != nil { // handle locking error + panic(err) } fmt.Printf("locked: %v\n", f.Locked()) @@ -41,6 +43,7 @@ func ExampleFlock_TryLock() { locked, err := fileLock.TryLock() if err != nil { // handle locking error + panic(err) } if locked { @@ -48,6 +51,7 @@ func ExampleFlock_TryLock() { if err := fileLock.Unlock(); err != nil { // handle unlock error + panic(err) } } @@ -64,6 +68,7 @@ func ExampleFlock_TryLockContext() { locked, err := fileLock.TryLockContext(lockCtx, 678*time.Millisecond) if err != nil { // handle locking error + panic(err) } if locked { @@ -71,6 +76,7 @@ func ExampleFlock_TryLockContext() { if err := fileLock.Unlock(); err != nil { // handle unlock error + panic(err) } } diff --git a/flock_test.go b/flock_test.go index bb3fb7a..82e0dd8 100644 --- a/flock_test.go +++ b/flock_test.go @@ -2,6 +2,7 @@ // Copyright 2018-2024 The Gofrs. All rights reserved. // Use of this source code is governed by the BSD 3-Clause // license that can be found in the LICENSE file. + package flock_test import ( @@ -47,71 +48,71 @@ func (s *TestSuite) TestNew() { f := flock.New(s.path) s.Require().NotNil(f) - s.Assert().Equal(s.path, f.Path()) - s.Assert().False(f.Locked()) - s.Assert().False(f.RLocked()) + s.Equal(s.path, f.Path()) + s.False(f.Locked()) + s.False(f.RLocked()) } func (s *TestSuite) TestFlock_Path() { path := s.flock.Path() - s.Assert().Equal(s.path, path) + s.Equal(s.path, path) } func (s *TestSuite) TestFlock_Locked() { locked := s.flock.Locked() - s.Assert().False(locked) + s.False(locked) } func (s *TestSuite) TestFlock_RLocked() { locked := s.flock.RLocked() - s.Assert().False(locked) + s.False(locked) } func (s *TestSuite) TestFlock_String() { str := s.flock.String() - s.Assert().Equal(s.path, str) + s.Equal(s.path, str) } func (s *TestSuite) TestFlock_TryLock() { - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) var locked bool var err error locked, err = s.flock.TryLock() s.Require().NoError(err) - s.Assert().True(locked) - s.Assert().True(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.True(locked) + s.True(s.flock.Locked()) + s.False(s.flock.RLocked()) locked, err = s.flock.TryLock() s.Require().NoError(err) - s.Assert().True(locked) + s.True(locked) // make sure we just return false with no error in cases // where we would have been blocked locked, err = flock.New(s.path).TryLock() s.Require().NoError(err) - s.Assert().False(locked) + s.False(locked) } func (s *TestSuite) TestFlock_TryRLock() { - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) var locked bool var err error locked, err = s.flock.TryRLock() s.Require().NoError(err) - s.Assert().True(locked) - s.Assert().False(s.flock.Locked()) - s.Assert().True(s.flock.RLocked()) + s.True(locked) + s.False(s.flock.Locked()) + s.True(s.flock.RLocked()) locked, err = s.flock.TryRLock() s.Require().NoError(err) - s.Assert().True(locked) + s.True(locked) // shared lock should not block. flock2 := flock.New(s.path) @@ -124,9 +125,9 @@ func (s *TestSuite) TestFlock_TryRLock() { // when the first descriptor is closed, the second descriptor // would still be open but silently unlocked. So a second // TryRLock must return false. - s.Assert().False(locked) + s.False(locked) } else { - s.Assert().True(locked) + s.True(locked) } // make sure we just return false with no error in cases @@ -136,7 +137,7 @@ func (s *TestSuite) TestFlock_TryRLock() { _ = s.flock.Lock() locked, err = flock.New(s.path).TryRLock() s.Require().NoError(err) - s.Assert().False(locked) + s.False(locked) } func (s *TestSuite) TestFlock_TryLockContext() { @@ -144,20 +145,22 @@ func (s *TestSuite) TestFlock_TryLockContext() { ctx, cancel := context.WithCancel(context.Background()) locked, err := s.flock.TryLockContext(ctx, time.Second) s.Require().NoError(err) - s.Assert().True(locked) + s.True(locked) // context already canceled cancel() + locked, err = flock.New(s.path).TryLockContext(ctx, time.Second) - s.Assert().ErrorIs(err, context.Canceled) - s.Assert().False(locked) + s.Require().ErrorIs(err, context.Canceled) + s.False(locked) // timeout ctx, cancel = context.WithTimeout(context.Background(), 10*time.Millisecond) defer cancel() + locked, err = flock.New(s.path).TryLockContext(ctx, time.Second) - s.Assert().ErrorIs(err, context.DeadlineExceeded) - s.Assert().False(locked) + s.Require().ErrorIs(err, context.DeadlineExceeded) + s.False(locked) } func (s *TestSuite) TestFlock_TryRLockContext() { @@ -165,22 +168,24 @@ func (s *TestSuite) TestFlock_TryRLockContext() { ctx, cancel := context.WithCancel(context.Background()) locked, err := s.flock.TryRLockContext(ctx, time.Second) s.Require().NoError(err) - s.Assert().True(locked) + s.True(locked) // context already canceled cancel() locked, err = flock.New(s.path).TryRLockContext(ctx, time.Second) - s.Assert().ErrorIs(err, context.Canceled) - s.Assert().False(locked) + s.Require().ErrorIs(err, context.Canceled) + s.False(locked) // timeout _ = s.flock.Unlock() _ = s.flock.Lock() + ctx, cancel = context.WithTimeout(context.Background(), 10*time.Millisecond) defer cancel() + locked, err = flock.New(s.path).TryRLockContext(ctx, time.Second) - s.Assert().ErrorIs(err, context.DeadlineExceeded) - s.Assert().False(locked) + s.Require().ErrorIs(err, context.DeadlineExceeded) + s.False(locked) } func (s *TestSuite) TestFlock_Unlock() { @@ -192,29 +197,29 @@ func (s *TestSuite) TestFlock_Unlock() { // get a lock for us to unlock locked, err := s.flock.TryLock() s.Require().NoError(err) - s.Assert().True(locked) - s.Assert().True(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.True(locked) + s.True(s.flock.Locked()) + s.False(s.flock.RLocked()) _, err = os.Stat(s.path) - s.Assert().False(os.IsNotExist(err)) + s.False(os.IsNotExist(err)) err = s.flock.Unlock() s.Require().NoError(err) - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) } func (s *TestSuite) TestFlock_Lock() { - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) var err error err = s.flock.Lock() s.Require().NoError(err) - s.Assert().True(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.True(s.flock.Locked()) + s.False(s.flock.RLocked()) // test that the short-circuit works err = s.flock.Lock() @@ -234,31 +239,31 @@ func (s *TestSuite) TestFlock_Lock() { }(ch) errCh, ok := <-ch - s.Assert().True(ok) + s.True(ok) s.Require().NoError(errCh) err = s.flock.Unlock() s.Require().NoError(err) errCh, ok = <-ch - s.Assert().True(ok) + s.True(ok) s.Require().NoError(errCh) - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) - s.Assert().True(gf.Locked()) - s.Assert().False(gf.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) + s.True(gf.Locked()) + s.False(gf.RLocked()) } func (s *TestSuite) TestFlock_RLock() { - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) var err error err = s.flock.RLock() s.Require().NoError(err) - s.Assert().False(s.flock.Locked()) - s.Assert().True(s.flock.RLocked()) + s.False(s.flock.Locked()) + s.True(s.flock.RLocked()) // test that the short-circuit works err = s.flock.RLock() @@ -278,17 +283,17 @@ func (s *TestSuite) TestFlock_RLock() { }(ch) errCh, ok := <-ch - s.Assert().True(ok) + s.True(ok) s.Require().NoError(errCh) err = s.flock.Unlock() s.Require().NoError(err) errCh, ok = <-ch - s.Assert().True(ok) + s.True(ok) s.Require().NoError(errCh) - s.Assert().False(s.flock.Locked()) - s.Assert().False(s.flock.RLocked()) - s.Assert().False(gf.Locked()) - s.Assert().True(gf.RLocked()) + s.False(s.flock.Locked()) + s.False(s.flock.RLocked()) + s.False(gf.Locked()) + s.True(gf.RLocked()) } diff --git a/flock_unix.go b/flock_unix.go index ae6a452..f5f17f4 100644 --- a/flock_unix.go +++ b/flock_unix.go @@ -8,6 +8,7 @@ package flock import ( + "errors" "os" "syscall" ) @@ -158,6 +159,7 @@ retry: *locked = true return true, nil } + if !retried { if shouldRetry, reopenErr := f.reopenFDOnError(err); reopenErr != nil { return false, reopenErr @@ -176,21 +178,22 @@ retry: // Since Linux 3.4 (commit 55725513) // Probably NFSv4 where flock() is emulated by fcntl(). func (f *Flock) reopenFDOnError(err error) (bool, error) { - if err != syscall.EIO && err != syscall.EBADF { + if !errors.Is(err, syscall.EIO) && !errors.Is(err, syscall.EBADF) { return false, nil } if st, err := f.fh.Stat(); err == nil { // if the file is able to be read and written - if st.Mode()&0600 == 0600 { + if st.Mode()&0o600 == 0o600 { f.fh.Close() f.fh = nil // reopen in read-write mode and set the filehandle - fh, err := os.OpenFile(f.path, os.O_CREATE|os.O_RDWR, os.FileMode(0600)) + fh, err := os.OpenFile(f.path, os.O_CREATE|os.O_RDWR, os.FileMode(0o600)) if err != nil { return false, err } f.fh = fh + return true, nil } } diff --git a/flock_winapi.go b/flock_winapi.go index ad9c1ba..9e4df34 100644 --- a/flock_winapi.go +++ b/flock_winapi.go @@ -32,9 +32,10 @@ const ( // // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365203(v=vs.85).aspx -func lockFileEx(handle syscall.Handle, flags uint32, reserved uint32, numberOfBytesToLockLow uint32, numberOfBytesToLockHigh uint32, offset *syscall.Overlapped) (bool, syscall.Errno) { +//nolint:unparam +func lockFileEx(handle syscall.Handle, flags, reserved, numberOfBytesToLockLow, numberOfBytesToLockHigh uint32, offset *syscall.Overlapped) (bool, syscall.Errno) { r1, _, errNo := syscall.SyscallN( - uintptr(procLockFileEx), + procLockFileEx, uintptr(handle), uintptr(flags), uintptr(reserved), @@ -53,9 +54,9 @@ func lockFileEx(handle syscall.Handle, flags uint32, reserved uint32, numberOfBy return true, 0 } -func unlockFileEx(handle syscall.Handle, reserved uint32, numberOfBytesToLockLow uint32, numberOfBytesToLockHigh uint32, offset *syscall.Overlapped) (bool, syscall.Errno) { +func unlockFileEx(handle syscall.Handle, reserved, numberOfBytesToLockLow, numberOfBytesToLockHigh uint32, offset *syscall.Overlapped) (bool, syscall.Errno) { r1, _, errNo := syscall.SyscallN( - uintptr(procUnlockFileEx), + procUnlockFileEx, uintptr(handle), uintptr(reserved), uintptr(numberOfBytesToLockLow), diff --git a/flock_windows.go b/flock_windows.go index a68287a..1f0c968 100644 --- a/flock_windows.go +++ b/flock_windows.go @@ -10,7 +10,7 @@ import ( ) // ErrorLockViolation is the error code returned from the Windows syscall when a -// lock would block and you ask to fail immediately. +// lock would block, and you ask to fail immediately. const ErrorLockViolation syscall.Errno = 0x21 // 33 // Lock is a blocking call to try and take an exclusive file lock. It will wait @@ -50,7 +50,8 @@ func (f *Flock) lock(locked *bool, flag uint32) error { defer f.ensureFhState() } - if _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), flag, 0, 1, 0, &syscall.Overlapped{}); errNo > 0 { + _, errNo := lockFileEx(syscall.Handle(f.fh.Fd()), flag, 0, 1, 0, &syscall.Overlapped{}) + if errNo > 0 { return errNo } @@ -75,7 +76,8 @@ func (f *Flock) Unlock() error { } // mark the file as unlocked - if _, errNo := unlockFileEx(syscall.Handle(f.fh.Fd()), 0, 1, 0, &syscall.Overlapped{}); errNo > 0 { + _, errNo := unlockFileEx(syscall.Handle(f.fh.Fd()), 0, 1, 0, &syscall.Overlapped{}) + if errNo > 0 { return errNo }