Merge pull request #355 from dundee/dundee/refactor/lint

refactor: lint fixes

.github/workflows/test.yml

@@ -18,9 +18,9 @@ jobs:
     - name: Checkout code
       uses: actions/checkout@v4
     - name: Run linters
-      uses: golangci/golangci-lint-action@v4
+      uses: golangci/golangci-lint-action@v5
       with:
-        version: v1.45
+        version: v1.57.2
 
   test:
     strategy:

.golangci.yml

@@ -0,0 +1,122 @@
+linters-settings:
+  errcheck:
+    check-blank: true
+  revive:
+    rules:
+      - 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: 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: unreachable-code
+      - name: redefines-builtin-id
+      # While we agree with this rule, right now it would break too many
+      # projects. So, we disable it by default.
+      - name: unused-parameter
+        disabled: true
+  gocyclo:
+    min-complexity: 25
+  dupl:
+    threshold: 100
+  goconst:
+    min-len: 3
+    min-occurrences: 3
+  lll:
+    line-length: 160
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      - whyNoLint
+  funlen:
+    lines: 500
+    statements: 50
+  govet:
+    enable:
+      - shadow
+
+linters:
+  disable-all: true
+  enable:
+    - bodyclose
+    - dogsled
+    - errcheck
+    - errorlint
+    - exhaustive
+    - exportloopref
+    - funlen
+    - goconst
+    - gocritic
+    - gocyclo
+    - gofmt
+    - goimports
+    - revive
+    - gosimple
+    - govet
+    - ineffassign
+    - lll
+    - nakedret
+    - staticcheck
+    - typecheck
+    - unparam
+    - unused
+    - whitespace
+
+issues:
+  exclude:
+    # We allow error shadowing
+    - 'declaration of "err" shadows declaration at'
+
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - gocyclo
+        - errcheck
+        - gosec
+        - funlen
+        - gocritic
+        - gochecknoglobals # Globals in test files are tolerated.
+        - goconst # Repeated consts in test files are tolerated.
+    # This rule is buggy and breaks on our `///Block` lines.  Disable for now.
+    - linters:
+        - gocritic
+      text: "commentFormatting: put a space"
+    # This rule incorrectly flags nil references after assert.Assert(t, x != nil)
+    - path: _test\.go
+      text: "SA5011"
+      linters:
+        - staticcheck
+    - linters:
+        - lll
+      source: "^//go:generate "
+    - linters:
+        - lll
+        - gocritic
+      path: \.resolvers\.go
+      source: '^func \(r \*[a-zA-Z]+Resolvers\) '
+
+output:
+  formats:
+    - format: colored-line-number
+  sort-results: true

Makefile

@@ -130,6 +130,9 @@ benchmark:
 		'gdu -npc ~' 'gdu -gnpc ~' 'gdu -npc --use-storage ~'
 	sudo cpupower frequency-set -g schedutil
 
+lint:
+	golangci-lint run -c .golangci.yml
+
 clean:
 	go mod tidy
 	-rm coverage.txt
@@ -151,5 +154,6 @@ install-dev-dependencies:
 	go install gotest.tools/gotestsum@latest
 	go install github.com/mitchellh/gox@latest
 	go install honnef.co/go/gotraceui/cmd/gotraceui@master
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
 
 .PHONY: run build build-static build-all test gobench benchmark coverage coverage-html clean clean-uncompressed-dist man show-man release

cmd/gdu/app/app_test.go

@@ -500,6 +500,7 @@ func TestMaxCoresLowEdge(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+// nolint: unparam // Why: it's used in linux tests
 func runApp(flags *Flags, args []string, istty bool, getter device.DevicesInfoGetter) (string, error) {
 	buff := bytes.NewBufferString("")
 

internal/common/ignore.go

@@ -100,6 +100,7 @@ func (ui *UI) IsHiddenDir(name, path string) bool {
 }
 
 // CreateIgnoreFunc returns function for detecting if dir should be ignored
+// nolint: gocyclo // Why: This function is a switch statement that is not too complex
 func (ui *UI) CreateIgnoreFunc() ShouldDirBeIgnored {
 	switch {
 	case len(ui.IgnoreDirPaths) > 0 && ui.IgnoreDirPathPatterns == nil && !ui.IgnoreHidden:

internal/testanalyze/analyze.go

@@ -84,19 +84,19 @@ func (a *MockedAnalyzer) ResetProgress() {}
 // SetFollowSymlinks does nothing
 func (a *MockedAnalyzer) SetFollowSymlinks(v bool) {}
 
-// RemoveItemFromDirWithErr returns error
-func RemoveItemFromDirWithErr(dir fs.Item, file fs.Item) error {
+// ItemFromDirWithErr returns error
+func ItemFromDirWithErr(dir, file fs.Item) error {
 	return errors.New("Failed")
 }
 
-// RemoveItemFromDirWithSleep returns error
-func RemoveItemFromDirWithSleep(dir fs.Item, file fs.Item) error {
+// ItemFromDirWithSleep returns error
+func ItemFromDirWithSleep(dir, file fs.Item) error {
 	time.Sleep(time.Millisecond * 600)
-	return remove.RemoveItemFromDir(dir, file)
+	return remove.ItemFromDir(dir, file)
 }
 
-// RemoveItemFromDirWithSleepAndErr returns error
-func RemoveItemFromDirWithSleepAndErr(dir fs.Item, file fs.Item) error {
+// ItemFromDirWithSleepAndErr returns error
+func ItemFromDirWithSleepAndErr(dir, file fs.Item) error {
 	time.Sleep(time.Millisecond * 600)
 	return errors.New("Failed")
 }

pkg/analyze/dir_linux-openbsd.go

@@ -12,8 +12,7 @@ import (
 const devBSize = 512
 
 func setPlatformSpecificAttrs(file *File, f os.FileInfo) {
-	switch stat := f.Sys().(type) {
-	case *syscall.Stat_t:
+	if stat, ok := f.Sys().(*syscall.Stat_t); ok {
 		file.Usage = stat.Blocks * devBSize
 		file.Mtime = time.Unix(int64(stat.Mtim.Sec), int64(stat.Mtim.Nsec))
 

pkg/analyze/dir_unix.go

@@ -12,8 +12,7 @@ import (
 const devBSize = 512
 
 func setPlatformSpecificAttrs(file *File, f os.FileInfo) {
-	switch stat := f.Sys().(type) {
-	case *syscall.Stat_t:
+	if stat, ok := f.Sys().(*syscall.Stat_t); ok {
 		file.Usage = stat.Blocks * devBSize
 		file.Mtime = time.Unix(int64(stat.Mtimespec.Sec), int64(stat.Mtimespec.Nsec))
 

pkg/analyze/file.go

@@ -66,8 +66,7 @@ func (f *File) GetMtime() time.Time {
 
 // GetType returns name type of item
 func (f *File) GetType() string {
-	switch f.Flag {
-	case '@':
+	if f.Flag == '@' {
 		return "Other"
 	}
 	return "File"
@@ -97,7 +96,7 @@ func (f *File) alreadyCounted(linkedItems fs.HardLinkedItems) bool {
 }
 
 // GetItemStats returns 1 as count of items, apparent usage and real usage of this file
-func (f *File) GetItemStats(linkedItems fs.HardLinkedItems) (int, int64, int64) {
+func (f *File) GetItemStats(linkedItems fs.HardLinkedItems) (itemCount int, size, usage int64) {
 	if f.alreadyCounted(linkedItems) {
 		return 1, 0, 0
 	}
@@ -198,7 +197,7 @@ func (f *Dir) GetPath() string {
 }
 
 // GetItemStats returns item count, apparent usage and real usage of this dir
-func (f *Dir) GetItemStats(linkedItems fs.HardLinkedItems) (int, int64, int64) {
+func (f *Dir) GetItemStats(linkedItems fs.HardLinkedItems) (itemCount int, size, usage int64) {
 	f.UpdateStats(linkedItems)
 	return f.ItemCount, f.GetSize(), f.GetUsage()
 }

pkg/analyze/parallel.go

@@ -140,12 +140,15 @@ func (a *ParallelAnalyzer) processDir(path string) *Dir {
 				continue
 			}
 			if a.followSymlinks && info.Mode()&os.ModeSymlink != 0 {
-				err = followSymlink(entryPath, &info)
+				infoF, err := followSymlink(entryPath)
 				if err != nil {
 					log.Print(err.Error())
 					dir.Flag = '!'
 					continue
 				}
+				if infoF != nil {
+					info = infoF
+				}
 			}
 
 			file = &File{
@@ -211,27 +214,23 @@ func getDirFlag(err error, items int) rune {
 }
 
 func getFlag(f os.FileInfo) rune {
-	switch {
-	case f.Mode()&os.ModeSymlink != 0:
-		fallthrough
-	case f.Mode()&os.ModeSocket != 0:
+	if f.Mode()&os.ModeSymlink != 0 || f.Mode()&os.ModeSocket != 0 {
 		return '@'
-	default:
-		return ' '
 	}
+	return ' '
 }
 
-func followSymlink(path string, f *os.FileInfo) error {
+func followSymlink(path string) (os.FileInfo, error) {
 	target, err := filepath.EvalSymlinks(path)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	tInfo, err := os.Lstat(target)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	if !tInfo.IsDir() {
-		*f = tInfo
+		return tInfo, nil
 	}
-	return nil
+	return nil, nil
 }

pkg/analyze/sequential.go

@@ -127,12 +127,15 @@ func (a *SequentialAnalyzer) processDir(path string) *Dir {
 				continue
 			}
 			if a.followSymlinks && info.Mode()&os.ModeSymlink != 0 {
-				err = followSymlink(entryPath, &info)
+				infoF, err := followSymlink(entryPath)
 				if err != nil {
 					log.Print(err.Error())
 					dir.Flag = '!'
 					continue
 				}
+				if infoF != nil {
+					info = infoF
+				}
 			}
 
 			file = &File{

pkg/analyze/stored.go

@@ -315,7 +315,7 @@ func (f *StoredDir) RemoveFile(item fs.Item) {
 }
 
 // GetItemStats returns item count, apparent usage and real usage of this dir
-func (f *StoredDir) GetItemStats(linkedItems fs.HardLinkedItems) (int, int64, int64) {
+func (f *StoredDir) GetItemStats(linkedItems fs.HardLinkedItems) (itemCount int, size, usage int64) {
 	f.UpdateStats(linkedItems)
 	return f.ItemCount, f.GetSize(), f.GetUsage()
 }
@@ -385,9 +385,9 @@ func (p *ParentDir) GetFiles() fs.Files                               { panic("m
 func (p *ParentDir) GetFilesLocked() fs.Files                         { panic("must not be called") }
 func (p *ParentDir) RLock() func()                                    { panic("must not be called") }
 func (p *ParentDir) SetFiles(fs.Files)                                { panic("must not be called") }
-func (f *ParentDir) RemoveFile(item fs.Item)                          { panic("must not be called") }
+func (p *ParentDir) RemoveFile(item fs.Item)                          { panic("must not be called") }
 func (p *ParentDir) GetItemStats(
 	linkedItems fs.HardLinkedItems,
-) (int, int64, int64) {
+) (itemCount int, size, usage int64) {
 	panic("must not be called")
 }

pkg/analyze/wait.go

@@ -19,7 +19,7 @@ func (s *WaitGroup) Init() *WaitGroup {
 // Add increments value
 func (s *WaitGroup) Add(value int) {
 	s.access.Lock()
-	s.value = s.value + value
+	s.value += value
 	s.access.Unlock()
 }
 

pkg/device/dev_freebsd_darwin_other.go

@@ -80,18 +80,20 @@ func processMounts(mounts Devices, ignoreErrors bool) (Devices, error) {
 	devices := Devices{}
 
 	for _, mount := range mounts {
-		if strings.HasPrefix(mount.Name, "/dev") || mount.Fstype == "zfs" {
-			info := &unix.Statfs_t{}
-			err := unix.Statfs(mount.MountPoint, info)
-			if err != nil && !ignoreErrors {
-				return nil, err
-			}
-
-			mount.Size = int64(info.Bsize) * int64(info.Blocks)
-			mount.Free = int64(info.Bsize) * int64(info.Bavail)
+		if !strings.HasPrefix(mount.Name, "/dev") && mount.Fstype != "zfs" {
+			continue
+		}
 
-			devices = append(devices, mount)
+		info := &unix.Statfs_t{}
+		err := unix.Statfs(mount.MountPoint, info)
+		if err != nil && !ignoreErrors {
+			return nil, err
 		}
+
+		mount.Size = int64(info.Bsize) * int64(info.Blocks)
+		mount.Free = int64(info.Bsize) * int64(info.Bavail)
+
+		devices = append(devices, mount)
 	}
 
 	return devices, nil

pkg/device/dev_freebsd_darwin_test.go

@@ -34,7 +34,9 @@ argon:/usr/obj on /usr/obj (nfs)`))
 }
 
 func TestMountsWithSpace(t *testing.T) {
-	mounts, err := readMountOutput(strings.NewReader(`//[email protected]/volatile on /Users/inglor/Mountpoints/volatile (vault.lan) (smbfs, nodev, nosuid, mounted by inglor)`))
+	mounts, err := readMountOutput(strings.NewReader(
+		`//[email protected]/volatile on /Users/inglor/Mountpoints/volatile (vault.lan) (smbfs, nodev, nosuid, mounted by inglor)`,
+	))
 	assert.Equal(t, "//[email protected]/volatile", mounts[0].Name)
 	assert.Equal(t, "/Users/inglor/Mountpoints/volatile (vault.lan)", mounts[0].MountPoint)
 	assert.Equal(t, "smbfs", mounts[0].Fstype)

pkg/device/dev_linux.go

@@ -28,7 +28,7 @@ func (t LinuxDevicesInfoGetter) GetMounts() (Devices, error) {
 	devices, err := readMountsFile(file)
 	if err != nil {
 		if cerr := file.Close(); cerr != nil {
-			return nil, fmt.Errorf("%w; %s", err, cerr)
+			return nil, fmt.Errorf("%w; %s", err, cerr.Error())
 		}
 		return nil, err
 	}