golangci-lint: Add more linters (#2877)

Adds new 1.63 linters: - https://github.com/alingse/nilnesserr: Provides a concise way to detect return an unrelated/nil-values error. - https://github.com/ldez/exptostd: Detects functions from golang.org/x/exp/ that can be replaced by std functions. - https://github.com/ldez/usetesting: Detects when some calls can be replaced by methods from the testing package. Additionally, add more older, useful linters.
elastic · Jan 3, 2025 · e4e4996 · e4e4996
1 parent b0f4341
commit e4e4996
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 152 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -4,26 +4,37 @@ run:
 
 linters:
   enable:
+    - bodyclose
     - canonicalheader
     - containedctx
     - contextcheck
-    - exhaustruct
     - copyloopvar
+    - errname
+    - exhaustruct
+    - exptostd
     - fatcontext
     - gci
     - gocritic
     - gocyclo
     - gofmt
     - gomodguard
+    - iface
+    - makezero
     - misspell
+    - nilerr
+    - nilnesserr
     - nolintlint
     - prealloc
+    - predeclared
     - reassign
+    - recvcheck
     - revive
     - testifylint
     - unconvert
     - unparam
     - unused
+    - usetesting
+    - wastedassign
     - whitespace
 
 issues:

diff --git a/bin/.golangci-lint-1.62.2.pkg → bin/.golangci-lint-1.63.3.pkg b/bin/.golangci-lint-1.62.2.pkg → bin/.golangci-lint-1.63.3.pkg
diff --git a/bin/golangci-lint b/bin/golangci-lint
@@ -1 +1 @@
-.golangci-lint-1.62.2.pkg
+.golangci-lint-1.63.3.pkg
diff --git a/internal/inventory/azurefetcher/fetcher_storage.go b/internal/inventory/azurefetcher/fetcher_storage.go
@@ -78,7 +78,7 @@ func (f *storageFetcher) listStorageAccounts(ctx context.Context) ([]azurelib.Az
 		return nil, fmt.Errorf("error listing subscriptions: %v", err)
 	}
 
-	subscriptionIds := make([]string, len(subscriptions))
+	subscriptionIds := make([]string, 0, len(subscriptions))
 	for _, subscription := range subscriptions {
 		subscriptionIds = append(subscriptionIds, subscription.Name)
 	}

diff --git a/internal/launcher/launcher.go b/internal/launcher/launcher.go
@@ -40,14 +40,14 @@ const (
 	shutdownGracePeriod = 20 * time.Second
 )
 
-// ErrorStopSignal is used to indicate we got a stop signal
-var ErrorStopSignal = errors.New("stop beater")
+// ErrStopSignal is used to indicate we got a stop signal
+var ErrStopSignal = errors.New("stop beater")
 
-// ErrorGracefulExit is used when the launcher stops before shutdownGracePeriod, after waiting for the beater to stop
-var ErrorGracefulExit = beat.GracefulExit
+// ErrGracefulExit is used when the launcher stops before shutdownGracePeriod, after waiting for the beater to stop
+var ErrGracefulExit = beat.GracefulExit
 
-// ErrorTimeoutExit is used when the launcher stops after shutdownGracePeriod, without waiting for the beater to stop
-var ErrorTimeoutExit = errors.New("exit after timeout")
+// ErrTimeoutExit is used when the launcher stops after shutdownGracePeriod, without waiting for the beater to stop
+var ErrTimeoutExit = errors.New("exit after timeout")
 
 type launcher struct {
 	wg        sync.WaitGroup // WaitGroup used to wait for active beaters
@@ -115,9 +115,9 @@ func (l *launcher) run() error {
 	err := l.runLoop()
 
 	switch {
-	case errors.Is(err, ErrorGracefulExit):
+	case errors.Is(err, ErrGracefulExit):
 		l.log.Info("Launcher stopped successfully")
-	case errors.Is(err, ErrorTimeoutExit):
+	case errors.Is(err, ErrTimeoutExit):
 		l.log.Info("Launcher stopped after timeout")
 	case err == nil: // unexpected
 	default:
@@ -140,7 +140,7 @@ func (l *launcher) runLoop() error {
 
 		// Wait for something to happen:
 		// config update		(val, nil)
-		// stop signal			(nil, ErrorStopSignal)
+		// stop signal			(nil, ErrStopSignal)
 		// beater error			(nil, err)
 		cfg, err := l.waitForUpdates()
 
@@ -224,24 +224,24 @@ func (l *launcher) stopBeaterWithTimeout(duration time.Duration) error {
 	select {
 	case <-time.After(duration):
 		l.log.Infof("Grace period for %s ended", l.name)
-		return ErrorTimeoutExit
+		return ErrTimeoutExit
 	case <-wgCh:
 		l.log.Infof("Launcher shut %s down gracefully", l.name)
-		return ErrorGracefulExit
+		return ErrGracefulExit
 	}
 }
 
 // waitForUpdates is the function that keeps Launcher runLoop busy.
 // It will finish for one of following reasons:
-//  1. The Stop function got called 	(nil, ErrorStopSignal)
+//  1. The Stop function got called 	(nil, ErrStopSignal)
 //  2. The beater run has returned 		(nil, err)
 //  3. A config update received 		(val, nil)
 func (l *launcher) waitForUpdates() (*config.C, error) {
 	select {
 	case err, ok := <-l.beaterErr:
 		if !ok {
 			l.log.Infof("Launcher received a stop signal")
-			return nil, ErrorStopSignal
+			return nil, ErrStopSignal
 		}
 		return nil, err
 
@@ -260,7 +260,7 @@ func isConfigUpdate(cfg *config.C, err error) bool {
 }
 
 func isStopSignal(cfg *config.C, err error) bool {
-	return cfg == nil && errors.Is(err, ErrorStopSignal)
+	return cfg == nil && errors.Is(err, ErrStopSignal)
 }
 
 func isBeaterError(cfg *config.C, err error) bool {

diff --git a/internal/launcher/launcher_test.go b/internal/launcher/launcher_test.go
@@ -304,7 +304,7 @@ func (s *LauncherTestSuite) TestWaitForUpdates() {
 			}(tt.configs)
 
 			err := sut.run()
-			s.Require().ErrorIs(err, ErrorGracefulExit)
+			s.Require().ErrorIs(err, ErrGracefulExit)
 			beater, ok := sut.beater.(*beaterMock)
 			s.Require().True(ok)
 			s.Equal(tt.expected, beater.cfg)
@@ -450,7 +450,7 @@ func (s *LauncherTestSuite) TestLauncherUpdateAndStop() {
 		sut.Stop()
 	}()
 	err := sut.run()
-	s.Require().ErrorIs(err, ErrorGracefulExit)
+	s.Require().ErrorIs(err, ErrGracefulExit)
 }
 
 func (s *LauncherTestSuite) TestLauncherStopTwicePanics() {
@@ -461,7 +461,7 @@ func (s *LauncherTestSuite) TestLauncherStopTwicePanics() {
 		sut.Stop()
 	}()
 	err := sut.run()
-	s.Require().ErrorIs(err, ErrorGracefulExit)
+	s.Require().ErrorIs(err, ErrGracefulExit)
 
 	s.Panics(func() {
 		sut.Stop()
@@ -481,7 +481,7 @@ func (s *LauncherTestSuite) TestLauncherStop() {
 	}()
 
 	err := sut.run()
-	s.Require().ErrorIs(err, ErrorGracefulExit)
+	s.Require().ErrorIs(err, ErrGracefulExit)
 }
 
 func (s *LauncherTestSuite) TestLauncherStopTimeout() {
@@ -496,7 +496,7 @@ func (s *LauncherTestSuite) TestLauncherStopTimeout() {
 	}()
 
 	err := sut.run()
-	s.Require().ErrorIs(err, ErrorTimeoutExit)
+	s.Require().ErrorIs(err, ErrTimeoutExit)
 }
 
 func (s *LauncherTestSuite) initMocks() *launcherMocks {

diff --git a/internal/processor/add_cluster_id/errors.go b/internal/processor/add_cluster_id/errors.go
@@ -22,26 +22,26 @@ import (
 )
 
 type (
-	errConfigUnpack struct{ cause error }
-	errComputeID    struct{ cause error }
+	configUnpackError struct{ cause error }
+	computeIDError    struct{ cause error }
 )
 
-func makeErrConfigUnpack(cause error) errConfigUnpack {
-	return errConfigUnpack{cause}
+func makeErrConfigUnpack(cause error) configUnpackError {
+	return configUnpackError{cause}
 }
-func (e errConfigUnpack) Error() string {
+func (e configUnpackError) Error() string {
 	return fmt.Sprintf("failed to unpack %v processor configuration: %v", processorName, e.cause)
 }
-func (e errConfigUnpack) Unwrap() error {
+func (e configUnpackError) Unwrap() error {
 	return e.cause
 }
 
-func makeErrComputeID(cause error) errComputeID {
-	return errComputeID{cause}
+func makeErrComputeID(cause error) computeIDError {
+	return computeIDError{cause}
 }
-func (e errComputeID) Error() string {
+func (e computeIDError) Error() string {
 	return fmt.Sprintf("failed to compute ID: %v", e.cause)
 }
-func (e errComputeID) Unwrap() error {
+func (e computeIDError) Unwrap() error {
 	return e.cause
 }
diff --git a/internal/resources/providers/awslib/current_region_selecot_test.go b/internal/resources/providers/awslib/current_region_selecot_test.go
@@ -35,7 +35,7 @@ var successfulCurrentCloudRegionOutput = &ec2imds.InstanceIdentityDocument{
 type CurrentRegionSelectorTestSuite struct {
 	suite.Suite
 	selector *currentRegionSelector
-	mock     *mockCurrentCloudRegion
+	mock     *MockMetadataProvider
 }
 
 func TestCurrentRegionSelectorTestSuite(t *testing.T) {
@@ -46,7 +46,7 @@ func TestCurrentRegionSelectorTestSuite(t *testing.T) {
 
 func (s *CurrentRegionSelectorTestSuite) SetupTest() {
 	s.selector = &currentRegionSelector{}
-	s.mock = &mockCurrentCloudRegion{}
+	s.mock = &MockMetadataProvider{}
 	s.selector.client = s.mock
 }
 

diff --git a/internal/resources/providers/awslib/current_region_selector.go b/internal/resources/providers/awslib/current_region_selector.go
@@ -21,16 +21,11 @@ import (
 	"context"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
-	ec2imds "github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
 	"github.com/elastic/elastic-agent-libs/logp"
 )
 
 type currentRegionSelector struct {
-	client currentCloudRegion
-}
-
-type currentCloudRegion interface {
-	GetMetadata(ctx context.Context, cfg aws.Config) (*ec2imds.InstanceIdentityDocument, error)
+	client MetadataProvider
 }
 
 func (s *currentRegionSelector) Regions(ctx context.Context, cfg aws.Config) ([]string, error) {

diff --git a/internal/resources/providers/awslib/mock_current_cloud_region.go b/internal/resources/providers/awslib/mock_current_cloud_region.go
diff --git a/internal/resources/utils/user/user.go b/internal/resources/utils/user/user.go
@@ -206,6 +206,7 @@ func matchGroupIndexValue(value string, idx int) lineFunc {
 			return
 		}
 		if _, err := strconv.Atoi(parts[2]); err != nil {
+			//nolint:nilerr
 			return nil, nil
 		}
 		return &Group{Name: parts[0], Gid: parts[2]}, nil
@@ -240,9 +241,11 @@ func matchUserIndexValue(value string, idx int) lineFunc {
 			return
 		}
 		if _, err := strconv.Atoi(parts[2]); err != nil {
+			//nolint:nilerr
 			return nil, nil
 		}
 		if _, err := strconv.Atoi(parts[3]); err != nil {
+			//nolint:nilerr
 			return nil, nil
 		}
 		u := &User{
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		.golangci-lint-1.62.2.pkg
		.golangci-lint-1.63.3.pkg