chore: enable several rules from go-critic

.golangci.yml

@@ -2,6 +2,7 @@ version: "2"
 linters:
   enable:
     - errorlint
+    - gocritic
     - misspell
     - revive
     - sloglint
@@ -11,6 +12,11 @@ linters:
     errcheck:
       exclude-functions:
         - (net/http.ResponseWriter).Write
+    gocritic:
+      enable-all: true
+      disabled-checks:
+        - hugeParam
+        - unnamedResult
     revive:
       rules:
         - name: unused-parameter

collector/collector.go

@@ -305,7 +305,7 @@ type Collector struct {
 	debugSNMP    bool
 }
 
-func New(ctx context.Context, target, authName, snmpContext string, snmpEngineID string, auth *config.Auth, modules []*NamedModule, logger *slog.Logger, metrics Metrics, conc int, debugSNMP bool) *Collector {
+func New(ctx context.Context, target, authName, snmpContext, snmpEngineID string, auth *config.Auth, modules []*NamedModule, logger *slog.Logger, metrics Metrics, conc int, debugSNMP bool) *Collector {
 	return &Collector{
 		ctx:          ctx,
 		target:       target,
@@ -809,36 +809,36 @@ func splitOid(oid []int, count int) ([]int, []int) {
 
 // This mirrors decodeValue in gosnmp's helper.go.
 func pduValueAsString(pdu *gosnmp.SnmpPDU, typ string, metrics Metrics) string {
-	switch pdu.Value.(type) {
+	switch v := pdu.Value.(type) {
 	case int:
-		return strconv.Itoa(pdu.Value.(int))
+		return strconv.Itoa(v)
 	case uint:
-		return strconv.FormatUint(uint64(pdu.Value.(uint)), 10)
+		return strconv.FormatUint(uint64(v), 10)
 	case uint64:
-		return strconv.FormatUint(pdu.Value.(uint64), 10)
+		return strconv.FormatUint(v, 10)
 	case float32:
-		return strconv.FormatFloat(float64(pdu.Value.(float32)), 'f', -1, 32)
+		return strconv.FormatFloat(float64(v), 'f', -1, 32)
 	case float64:
-		return strconv.FormatFloat(pdu.Value.(float64), 'f', -1, 64)
+		return strconv.FormatFloat(v, 'f', -1, 64)
 	case string:
 		if pdu.Type == gosnmp.ObjectIdentifier {
 			// Trim leading period.
-			return pdu.Value.(string)[1:]
+			return v[1:]
 		}
 		// DisplayString.
-		return strings.ToValidUTF8(pdu.Value.(string), "�")
+		return strings.ToValidUTF8(v, "�")
 	case []byte:
 		if typ == "" || typ == "Bits" {
 			typ = "OctetString"
 		}
 		// Reuse the OID index parsing code.
-		parts := make([]int, len(pdu.Value.([]byte)))
-		for i, o := range pdu.Value.([]byte) {
+		parts := make([]int, len(v))
+		for i, o := range v {
 			parts[i] = int(o)
 		}
 		if typ == "OctetString" || typ == "DisplayString" {
 			// Prepend the length, as it is explicit in an index.
-			parts = append([]int{len(pdu.Value.([]byte))}, parts...)
+			parts = append([]int{len(v)}, parts...)
 		}
 		str, _, _ := indexOidsAsString(parts, typ, 0, false, nil)
 		return strings.ToValidUTF8(str, "�")

collector/collector_test.go

@@ -186,7 +186,7 @@ func TestPduToSample(t *testing.T) {
 					"Template": []config.RegexpExtract{
 						{
 							Regex: config.Regexp{
-								regexp.MustCompile("([0-9].[0-9]+)"),
+								regexp.MustCompile(`(\d.\d+)`),
 							},
 							Value: "$1",
 						},

config/config.go

@@ -26,6 +26,18 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
+type Auth struct {
+	Community     Secret `yaml:"community,omitempty"`
+	SecurityLevel string `yaml:"security_level,omitempty"`
+	Username      string `yaml:"username,omitempty"`
+	Password      Secret `yaml:"password,omitempty"`
+	AuthProtocol  string `yaml:"auth_protocol,omitempty"`
+	PrivProtocol  string `yaml:"priv_protocol,omitempty"`
+	PrivPassword  Secret `yaml:"priv_password,omitempty"`
+	ContextName   string `yaml:"context_name,omitempty"`
+	Version       int    `yaml:"version,omitempty"`
+}
+
 func LoadFile(logger *slog.Logger, paths []string, expandEnvVars bool) (*Config, error) {
 	cfg := &Config{}
 	for _, p := range paths {
@@ -271,18 +283,6 @@ func (s Secret) MarshalYAML() (interface{}, error) {
 	return nil, nil
 }
 
-type Auth struct {
-	Community     Secret `yaml:"community,omitempty"`
-	SecurityLevel string `yaml:"security_level,omitempty"`
-	Username      string `yaml:"username,omitempty"`
-	Password      Secret `yaml:"password,omitempty"`
-	AuthProtocol  string `yaml:"auth_protocol,omitempty"`
-	PrivProtocol  string `yaml:"priv_protocol,omitempty"`
-	PrivPassword  Secret `yaml:"priv_password,omitempty"`
-	ContextName   string `yaml:"context_name,omitempty"`
-	Version       int    `yaml:"version,omitempty"`
-}
-
 func (c *Auth) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	*c = DefaultAuth
 	type plain Auth
@@ -361,9 +361,7 @@ func (re *Regexp) UnmarshalYAML(unmarshal func(interface{}) error) error {
 }
 
 func substituteEnvVariables(value string) (string, error) {
-	result := os.Expand(value, func(s string) string {
-		return os.Getenv(s)
-	})
+	result := os.Expand(value, os.Getenv)
 	if result == "" {
 		return "", errors.New(value + " environment variable not found")
 	}

config_test.go

@@ -31,9 +31,9 @@ func TestHideConfigSecrets(t *testing.T) {
 	}
 
 	// String method must not reveal authentication credentials.
-	sc.RLock()
+	sc.mu.RLock()
 	c, err := yaml.Marshal(sc.C)
-	sc.RUnlock()
+	sc.mu.RUnlock()
 	if err != nil {
 		t.Errorf("Error marshaling config: %v", err)
 	}
@@ -48,9 +48,9 @@ func TestLoadConfigWithOverrides(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error loading config %v: %v", "testdata/snmp-with-overrides.yml", err)
 	}
-	sc.RLock()
+	sc.mu.RLock()
 	_, err = yaml.Marshal(sc.C)
-	sc.RUnlock()
+	sc.mu.RUnlock()
 	if err != nil {
 		t.Errorf("Error marshaling config: %v", err)
 	}
@@ -63,9 +63,9 @@ func TestLoadMultipleConfigs(t *testing.T) {
 	if err != nil {
 		t.Errorf("Error loading configs %v: %v", configs, err)
 	}
-	sc.RLock()
+	sc.mu.RLock()
 	_, err = yaml.Marshal(sc.C)
-	sc.RUnlock()
+	sc.mu.RUnlock()
 	if err != nil {
 		t.Errorf("Error marshaling config: %v", err)
 	}
@@ -84,9 +84,9 @@ func TestEnvSecrets(t *testing.T) {
 	}
 
 	// String method must not reveal authentication credentials.
-	sc.RLock()
+	sc.mu.RLock()
 	c, err := yaml.Marshal(sc.C)
-	sc.RUnlock()
+	sc.mu.RUnlock()
 	if err != nil {
 		t.Errorf("Error marshaling config: %v", err)
 	}

generator/main.go

@@ -170,7 +170,7 @@ func main() {
 func scanParseOutput(logger *slog.Logger, output string) []string {
 	var parseOutput []string
 	output = strings.TrimSpace(strings.ToValidUTF8(output, "�"))
-	if len(output) > 0 {
+	if output != "" {
 		parseOutput = strings.Split(output, "\n")
 	}
 	parseErrors := len(parseOutput)

generator/tree.go

@@ -107,8 +107,7 @@ func prepareTree(nodes *Node, logger *slog.Logger) map[string]*Node {
 	walkNode(nodes, func(n *Node) {
 		// Set type on MAC addresses and strings.
 		// RFC 2579
-		switch n.Hint {
-		case "1x:":
+		if n.Hint == "1x:" {
 			n.Type = "PhysAddress48"
 		}
 		if displayStringRe.MatchString(n.Hint) {
@@ -395,11 +394,12 @@ func generateConfigModule(cfg *ModuleConfig, node *Node, nameToNode map[string]*
 
 				// Convert (InetAddressType,InetAddress) to (InetAddress)
 				if subtype, ok := combinedTypes[index.Type]; ok {
-					if prevType == subtype {
+					switch subtype {
+					case prevType:
 						metric.Indexes = metric.Indexes[:len(metric.Indexes)-1]
-					} else if prev2Type == subtype {
+					case prev2Type:
 						metric.Indexes = metric.Indexes[:len(metric.Indexes)-2]
-					} else {
+					default:
 						logger.Warn("Can't handle index type on node, missing preceding", "node", n.Label, "type", index.Type, "missing", subtype)
 						return
 					}
@@ -513,41 +513,43 @@ func generateConfigModule(cfg *ModuleConfig, node *Node, nameToNode map[string]*
 	// Check that the object before an InetAddress is an InetAddressType.
 	// If not, change it to an OctetString.
 	for _, metric := range out.Metrics {
-		if metric.Type == "InetAddress" || metric.Type == "InetAddressMissingSize" {
-			// Get previous oid.
-			oids := strings.Split(metric.Oid, ".")
-			i, _ := strconv.Atoi(oids[len(oids)-1])
-			oids[len(oids)-1] = strconv.Itoa(i - 1)
-			prevOid := strings.Join(oids, ".")
-			if prevObj, ok := nameToNode[prevOid]; !ok || prevObj.TextualConvention != "InetAddressType" {
-				metric.Type = "OctetString"
-			} else {
-				// Make sure the InetAddressType is included.
-				if len(tableInstances[metric.Oid]) > 0 {
-					for _, index := range tableInstances[metric.Oid] {
-						needToWalk[prevOid+index+"."] = struct{}{}
-					}
-				} else {
-					needToWalk[prevOid] = struct{}{}
+		if metric.Type != "InetAddress" && metric.Type != "InetAddressMissingSize" {
+			continue
+		}
+		// Get previous oid.
+		oids := strings.Split(metric.Oid, ".")
+		i, _ := strconv.Atoi(oids[len(oids)-1])
+		oids[len(oids)-1] = strconv.Itoa(i - 1)
+		prevOid := strings.Join(oids, ".")
+		if prevObj, ok := nameToNode[prevOid]; !ok || prevObj.TextualConvention != "InetAddressType" {
+			metric.Type = "OctetString"
+		} else {
+			// Make sure the InetAddressType is included.
+			if len(tableInstances[metric.Oid]) > 0 {
+				for _, index := range tableInstances[metric.Oid] {
+					needToWalk[prevOid+index+"."] = struct{}{}
 				}
+			} else {
+				needToWalk[prevOid] = struct{}{}
 			}
 		}
 	}
 
 	// Apply module config overrides to their corresponding metrics.
 	for name, params := range cfg.Overrides {
 		for _, metric := range out.Metrics {
-			if name == metric.Name || name == metric.Oid {
-				metric.RegexpExtracts = params.RegexpExtracts
-				metric.DateTimePattern = params.DateTimePattern
-				metric.Offset = params.Offset
-				metric.Scale = params.Scale
-				if params.Help != "" {
-					metric.Help = params.Help
-				}
-				if params.Name != "" {
-					metric.Name = params.Name
-				}
+			if name != metric.Name && name != metric.Oid {
+				continue
+			}
+			metric.RegexpExtracts = params.RegexpExtracts
+			metric.DateTimePattern = params.DateTimePattern
+			metric.Offset = params.Offset
+			metric.Scale = params.Scale
+			if params.Help != "" {
+				metric.Help = params.Help
+			}
+			if params.Name != "" {
+				metric.Name = params.Name
 			}
 		}
 	}

generator/tree_test.go

@@ -179,7 +179,7 @@ func TestTreePrepare(t *testing.T) {
 
 func TestGenerateConfigModule(t *testing.T) {
 	var regexpFooBar config.Regexp
-	regexpFooBar.Regexp, _ = regexp.Compile(".*")
+	regexpFooBar.Regexp = regexp.MustCompile(".*")
 
 	strMetrics := make(map[string][]config.RegexpExtract)
 	strMetrics["Status"] = []config.RegexpExtract{

main.go

@@ -142,10 +142,10 @@ func handler(w http.ResponseWriter, r *http.Request, logger *slog.Logger, export
 			}
 		}
 	}
-	sc.RLock()
+	sc.mu.RLock()
 	auth, authOk := sc.C.Auths[authName]
 	if !authOk {
-		sc.RUnlock()
+		sc.mu.RUnlock()
 		http.Error(w, fmt.Sprintf("Unknown auth '%s'", authName), http.StatusBadRequest)
 		snmpRequestErrors.Inc()
 		return
@@ -154,14 +154,14 @@ func handler(w http.ResponseWriter, r *http.Request, logger *slog.Logger, export
 	for _, m := range modules {
 		module, moduleOk := sc.C.Modules[m]
 		if !moduleOk {
-			sc.RUnlock()
+			sc.mu.RUnlock()
 			http.Error(w, fmt.Sprintf("Unknown module '%s'", m), http.StatusBadRequest)
 			snmpRequestErrors.Inc()
 			return
 		}
 		nmodules = append(nmodules, collector.NewNamedModule(m, module))
 	}
-	sc.RUnlock()
+	sc.mu.RUnlock()
 	logger = logger.With("auth", authName, "target", target)
 	registry := prometheus.NewRegistry()
 	c := collector.New(r.Context(), target, authName, snmpContext, snmpEngineID, auth, nmodules, logger, exporterMetrics, *concurrency, debug)
@@ -185,22 +185,22 @@ func updateConfiguration(w http.ResponseWriter, r *http.Request) {
 }
 
 type SafeConfig struct {
-	sync.RWMutex
-	C *config.Config
+	mu sync.RWMutex
+	C  *config.Config
 }
 
 func (sc *SafeConfig) ReloadConfig(logger *slog.Logger, configFile []string, expandEnvVars bool) (err error) {
 	conf, err := config.LoadFile(logger, configFile, expandEnvVars)
 	if err != nil {
 		return err
 	}
-	sc.Lock()
+	sc.mu.Lock()
 	sc.C = conf
 	// Initialize metrics.
 	for module := range sc.C.Modules {
 		snmpCollectionDuration.WithLabelValues(module)
 	}
-	sc.Unlock()
+	sc.mu.Unlock()
 	return nil
 }
 
@@ -373,9 +373,9 @@ func main() {
 	}
 
 	http.HandleFunc(configPath, func(w http.ResponseWriter, r *http.Request) {
-		sc.RLock()
+		sc.mu.RLock()
 		c, err := yaml.Marshal(sc.C)
-		sc.RUnlock()
+		sc.mu.RUnlock()
 		if err != nil {
 			logger.Error("Error marshaling configuration", "err", err)
 			http.Error(w, err.Error(), http.StatusInternalServerError)