Merge pull request #158 from numtide/feat/more-linters

feat/more linters
numtide · Nov 19, 2024 · 48b88e6 · 48b88e6
2 parents 42e00a6 + 24c08ed
commit 48b88e6
Show file tree

Hide file tree

Showing 40 changed files with 275 additions and 166 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,34 +1,24 @@
 linters:
-    enable:
-        - errname
-        - exhaustive
-        - gci
-        - goconst
-        - godot
-        - gofumpt
-        - goheader
-        - goimports
-        - gosec
-        - importas
-        - ireturn
-        - lll
-        - makezero
-        - misspell
-        - nakedret
-        - nestif
-        - nilerr
-        - nilnil
-        - noctx
-        - nolintlint
-        - prealloc
-        - predeclared
-        - revive
-        - rowserrcheck
-        - stylecheck
-        - tenv
-        - testpackage
-        - unconvert
-        - unparam
-        - wastedassign
-        - whitespace
-        - wsl
+    enable-all: true
+    disable:
+        - depguard
+        - execinquery
+        - exhaustruct
+        - exportloopref
+        - funlen
+        - godox
+        - gomnd
+        - mnd
+        - varnamelen
+        - forbidigo
+        - gocognit
+        - gocyclo
+        - cyclop
+        - err113
+        - maintidx
+        - tagliatelle # prefer snake_case in json fields
+        - gochecknoglobals
+        - gochecknoinits
+        # would be nice to have but too many tests depend on environment variables, which is not allowed for t.Parallel()
+        - paralleltest
+        - nlreturn # find this annoying more than useful
diff --git a/nix/packages/nixos-facter/tests/default.nix b/nix/packages/nixos-facter/tests/default.nix
@@ -34,28 +34,6 @@ in
         environment.systemPackages = [
           perSystem.self.nixos-facter
         ];
-
-        systemd.services = {
-          create-swap-files = {
-            serviceConfig.Type = "oneshot";
-            wantedBy = ["multi-user.target"];
-            path = with pkgs; [
-              coreutils
-              util-linux
-            ];
-            script = ''
-              # create some swap files
-              mkdir -p /swap
-              for (( i=1; i<=3; i++ )); do
-                out="/swap/swapfile-$i"
-                dd if=/dev/zero of="$out" bs=1MB count=10
-                chmod 600 "$out"
-                mkswap "$out"
-                swapon "$out"
-              done
-            '';
-          };
-        };
       };
 
       extraTestScript = ''
@@ -74,14 +52,23 @@ in
 
         with subtest("Capture swap entries"):
             assert 'swap' in report, "'swap' not found in the report"
-            assert report['swap'] == [
-                { 'path': '/dev/vda4', 'type': 'partition', 'size': 1048572, 'used': 0, 'priority': -2 },
-                { 'path': '/dev/dm-0', 'type': 'partition', 'size': 10236, 'used': 0, 'priority': 100 },
-                { 'path': '/swap/swapfile-1', 'type': 'file', 'size': 9760, 'used': 0, 'priority': -3 },
-                { 'path': '/swap/swapfile-2', 'type': 'file', 'size': 9760, 'used': 0, 'priority': -4 },
-                { 'path': '/swap/swapfile-3', 'type': 'file', 'size': 9760, 'used': 0, 'priority': -5 }
-            ], "swap entries did not match what we expected"
-            # todo is there a nice way of showing a diff?
+
+            swap = report['swap']
+
+            expected = [
+                { 'type': 'partition', 'size': 1048572, 'used': 0, 'priority': -2 },
+                { 'type': 'partition', 'size': 10236, 'used': 0, 'priority': 100 }
+            ]
+
+            assert len(swap) == len(expected), f"expected {len(expected)} swap entries, found {len(swap)}"
+
+            for i in range(2):
+                assert swap[i]['path'].startswith("/dev/disk/by-uuid/"), f"expected a stable device path: {swap[i]['path']}"
+
+                # delete for easier comparison
+                del swap[i]['path']
+
+                assert swap[i] == expected[i], "swap[{i}] mismatch"
       '';
     };
   }
diff --git a/pkg/ephem/ephem.go b/pkg/ephem/ephem.go
@@ -5,6 +5,7 @@
 package ephem
 
 import (
+	"fmt"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -24,13 +25,13 @@ var deviceGlobs = []string{
 func StableDevicePath(device string) (string, error) {
 	l := slog.With("prefix", "stableDevicePath")
 
-	if !strings.HasPrefix("/", device) {
+	if !strings.HasPrefix(device, "/") {
 		return device, nil
 	}
 
 	stat, err := os.Stat(device)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("failed to stat device %s: %w", device, err)
 	}
 
 	for idx := range deviceGlobs {
@@ -40,7 +41,7 @@ func StableDevicePath(device string) (string, error) {
 		matches, err := filepath.Glob(glob)
 		if err != nil {
 			// the only possible error is ErrBadPattern
-			return "", err
+			return "", fmt.Errorf("failed to glob %s: %w", glob, err)
 		}
 
 		for _, match := range matches {

diff --git a/pkg/ephem/swap.go b/pkg/ephem/swap.go
@@ -2,6 +2,7 @@ package ephem
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -43,7 +44,7 @@ type SwapEntry struct {
 func SwapEntries() ([]*SwapEntry, error) {
 	f, err := os.Open("/proc/swaps")
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to open /proc/swaps: %w", err)
 	}
 
 	devices, err := ReadSwapFile(f)
@@ -68,7 +69,7 @@ func SwapEntries() ([]*SwapEntry, error) {
 func ReadSwapFile(reader io.Reader) ([]*SwapEntry, error) {
 	scanner := bufio.NewScanner(reader)
 	if !scanner.Scan() {
-		return nil, fmt.Errorf("swaps file is empty")
+		return nil, errors.New("swaps file is empty")
 	} else if b := scanner.Bytes(); !swapHeaderRegex.Match(b) {
 		return nil, fmt.Errorf("header in swaps file is malformed: '%s'", string(b))
 	}

diff --git a/pkg/ephem/swap_test.go b/pkg/ephem/swap_test.go
@@ -46,21 +46,21 @@ func TestReadSwapFile(t *testing.T) {
 	as.NoError(err)
 	as.Len(swaps, 3)
 
-	as.Equal(swaps[0].Filename, "/dev/sda6")
-	as.Equal(swaps[0].Type, ephem.SwapTypePartition)
-	as.Equal(swaps[0].Size, uint64(4194300))
-	as.Equal(swaps[0].Used, uint64(0))
-	as.Equal(swaps[0].Priority, int32(-1))
+	as.Equal("/dev/sda6", swaps[0].Filename)
+	as.Equal(ephem.SwapTypePartition, swaps[0].Type)
+	as.Equal(uint64(4194300), swaps[0].Size)
+	as.Equal(uint64(0), swaps[0].Used)
+	as.Equal(int32(-1), swaps[0].Priority)
 
-	as.Equal(swaps[1].Filename, "/var/lib/swap-1")
-	as.Equal(swaps[1].Type, ephem.SwapTypeFile)
-	as.Equal(swaps[1].Size, uint64(1048576))
-	as.Equal(swaps[1].Used, uint64(123))
-	as.Equal(swaps[1].Priority, int32(-3))
+	as.Equal("/var/lib/swap-1", swaps[1].Filename)
+	as.Equal(ephem.SwapTypeFile, swaps[1].Type)
+	as.Equal(uint64(1048576), swaps[1].Size)
+	as.Equal(uint64(123), swaps[1].Used)
+	as.Equal(int32(-3), swaps[1].Priority)
 
-	as.Equal(swaps[2].Filename, "/var/lib/swap-2")
-	as.Equal(swaps[2].Type, ephem.SwapTypeFile)
-	as.Equal(swaps[2].Size, uint64(2097152))
-	as.Equal(swaps[2].Used, uint64(4567))
-	as.Equal(swaps[2].Priority, int32(-2))
+	as.Equal("/var/lib/swap-2", swaps[2].Filename)
+	as.Equal(ephem.SwapTypeFile, swaps[2].Type)
+	as.Equal(uint64(2097152), swaps[2].Size)
+	as.Equal(uint64(4567), swaps[2].Used)
+	as.Equal(int32(-2), swaps[2].Priority)
 }
diff --git a/pkg/facter/facter.go b/pkg/facter/facter.go
@@ -3,6 +3,7 @@
 package facter
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/numtide/nixos-facter/pkg/build"
@@ -57,7 +58,7 @@ func (s *Scanner) Scan() (*Report, error) {
 	}
 
 	if build.System == "" {
-		return nil, fmt.Errorf("system is not set")
+		return nil, errors.New("system is not set")
 	}
 
 	report.System = build.System

diff --git a/pkg/facter/hardware.go b/pkg/facter/hardware.go
@@ -1,6 +1,7 @@
 package facter
 
 import (
+	"errors"
 	"fmt"
 	"slices"
 
@@ -196,7 +197,7 @@ func (h *Hardware) add(device hwinfo.HardwareDevice) error {
 	switch class {
 	case hwinfo.HardwareClassBios:
 		if h.Bios != nil {
-			return fmt.Errorf("bios field is already set")
+			return errors.New("bios field is already set")
 		} else if bios, ok := device.Detail.(*hwinfo.DetailBios); !ok {
 			return fmt.Errorf("expected hwinfo.DetailBios, found %T", device.Detail)
 		} else {
@@ -359,7 +360,7 @@ func (h *Hardware) add(device hwinfo.HardwareDevice) error {
 		slices.SortFunc(h.StorageController, compareDevice)
 	case hwinfo.HardwareClassSystem:
 		if h.System != nil {
-			return fmt.Errorf("system field is already set")
+			return errors.New("system field is already set")
 		} else if system, ok := device.Detail.(*hwinfo.DetailSys); !ok {
 			return fmt.Errorf("expected hwinfo.DetailSys, found %T", device.Detail)
 		} else {

diff --git a/pkg/facter/smbios.go b/pkg/facter/smbios.go
@@ -1,6 +1,7 @@
 package facter
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/numtide/nixos-facter/pkg/hwinfo"
@@ -77,15 +78,15 @@ func (s *Smbios) add(item hwinfo.Smbios) error {
 	switch item.SmbiosType() {
 	case hwinfo.SmbiosTypeBios:
 		if s.Bios != nil {
-			return fmt.Errorf("bios field is already set")
+			return errors.New("bios field is already set")
 		} else if bios, ok := item.(*hwinfo.SmbiosBios); !ok {
 			return fmt.Errorf("expected hwinfo.SmbiosBios, found %T", item)
 		} else {
 			s.Bios = bios
 		}
 	case hwinfo.SmbiosTypeBoard:
 		if s.Board != nil {
-			return fmt.Errorf("board field is already set")
+			return errors.New("board field is already set")
 		} else if board, ok := item.(*hwinfo.SmbiosBoard); !ok {
 			return fmt.Errorf("expected hwinfo.SmbiosBoard, found %T", item)
 		} else {
@@ -95,15 +96,15 @@ func (s *Smbios) add(item hwinfo.Smbios) error {
 		s.Cache = append(s.Cache, item)
 	case hwinfo.SmbiosTypeChassis:
 		if s.Chassis != nil {
-			return fmt.Errorf("chassis field is already set")
+			return errors.New("chassis field is already set")
 		} else if chassis, ok := item.(*hwinfo.SmbiosChassis); !ok {
 			return fmt.Errorf("expected hwinfo.SmbiosChassis, found %T", item)
 		} else {
 			s.Chassis = chassis
 		}
 	case hwinfo.SmbiosTypeConfig:
 		if s.Config != nil {
-			return fmt.Errorf("config field is already set")
+			return errors.New("config field is already set")
 		} else if config, ok := item.(*hwinfo.SmbiosConfig); !ok {
 			return fmt.Errorf("expected hwinfo.SmbiosConfig, found %T", item)
 		} else {
@@ -141,7 +142,7 @@ func (s *Smbios) add(item hwinfo.Smbios) error {
 		s.Slot = append(s.Slot, item)
 	case hwinfo.SmbiosTypeSystem:
 		if s.System != nil {
-			return fmt.Errorf("system field is already set")
+			return errors.New("system field is already set")
 		} else if system, ok := item.(*hwinfo.SmbiosSystem); !ok {
 			return fmt.Errorf("expected hwinfo.SmbiosSystem, found %T", item)
 		} else {

diff --git a/pkg/hwinfo/detail.go b/pkg/hwinfo/detail.go
@@ -19,6 +19,7 @@ import "C"
 
 import (
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"unsafe"
 )
@@ -52,11 +53,16 @@ type Detail interface {
 }
 
 //nolint:ireturn
-func NewDetail(detail *C.hd_detail_t) (result Detail, err error) {
+func NewDetail(detail *C.hd_detail_t) (Detail, error) {
 	if detail == nil {
-		return result, err
+		return nil, errors.New("detail is nil")
 	}
 
+	var (
+		err    error
+		result Detail
+	)
+
 	switch DetailType(C.hd_detail_get_type(detail)) {
 	case DetailTypePci:
 		result, err = NewDetailPci(C.hd_detail_get_pci(detail))
@@ -93,6 +99,6 @@ func NewMemoryRange(mem C.memory_range_t) MemoryRange {
 	return MemoryRange{
 		Start: uint(mem.start),
 		Size:  uint(mem.size),
-		Data:  hex.EncodeToString(C.GoBytes(unsafe.Pointer(&mem.data), C.int(mem.size))),
+		Data:  hex.EncodeToString(C.GoBytes(unsafe.Pointer(&mem.data), C.int(mem.size))), //nolint:gocritic
 	}
 }
diff --git a/pkg/hwinfo/detail_isa_pnp_dev.go b/pkg/hwinfo/detail_isa_pnp_dev.go
@@ -12,7 +12,7 @@ import "C"
 
 import (
 	"encoding/hex"
-	"fmt"
+	"errors"
 	"unsafe"
 )
 
@@ -29,7 +29,7 @@ func NewIsaPnpResource(res *C.isapnp_res_t) *IsaPnpResource {
 	return &IsaPnpResource{
 		Length: int(res.len),
 		Type:   int(res._type),
-		Data:   hex.EncodeToString(C.GoBytes(unsafe.Pointer(&res.data), res.len)),
+		Data:   hex.EncodeToString(C.GoBytes(unsafe.Pointer(&res.data), res.len)), //nolint:gocritic
 	}
 }
 
@@ -46,15 +46,15 @@ type IsaPnpCard struct {
 
 func NewIsaPnpCard(card *C.isapnp_card_t) (*IsaPnpCard, error) {
 	if card == nil {
-		return nil, fmt.Errorf("card is nil")
+		return nil, errors.New("card is nil")
 	}
 
 	return &IsaPnpCard{
 		Csn:     int(card.csn),
 		LogDevs: int(card.log_devs),
 		// Serial:   C.GoString(card.serial),	todo
 		// CardRegs: C.GoString(card.card_regs), todo
-		LdevRegs: hex.EncodeToString(C.GoBytes(unsafe.Pointer(&card.ldev_regs), C.int(0xd0))),
+		LdevRegs: hex.EncodeToString(C.GoBytes(unsafe.Pointer(&card.ldev_regs), C.int(0xd0))), //nolint:gocritic
 		ResLen:   int(card.res_len),
 		Broken:   bool(C.hd_isapnp_card_get_broken(card)),
 		Resource: NewIsaPnpResource(card.res),