From 562ae9055ce337597461560a568e4dcc6d0d0afa Mon Sep 17 00:00:00 2001
From: Ben Kochie <superq@gmail.com>
Date: Fri, 23 Jun 2023 15:00:38 +0200
Subject: [PATCH] Improve linting (#897)

* Enable more golangci-lint checks.
* Cleanup various linter issues.

Signed-off-by: SuperQ <superq@gmail.com>
---
 .golangci.yml          | 29 ++++++++++++++++++++++++++---
 collector/collector.go | 22 +++++++++-------------
 config/config.go       |  6 +++---
 generator/tree.go      |  3 +--
 4 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/.golangci.yml b/.golangci.yml
index d9efa75c..fa108af5 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -1,5 +1,28 @@
-# Run only staticcheck for now. Additional linters will be enabled one-by-one.
 linters:
   enable:
-  - staticcheck
-  disable-all: true
+    - misspell
+    - revive
+  disable:
+    # Disable soon to deprecated[1] linters that lead to false
+    # positives when build tags disable certain files[2]
+    # 1: https://github.com/golangci/golangci-lint/issues/1841
+    # 2: https://github.com/prometheus/node_exporter/issues/1545
+    - deadcode
+    - unused
+    - structcheck
+    - varcheck
+
+issues:
+  exclude-rules:
+    - path: _test.go
+      linters:
+        - errcheck
+        - govet
+
+linters-settings:
+  errcheck:
+    exclude-functions:
+      # Used in HTTP handlers, any error is handled by the server itself.
+      - (net/http.ResponseWriter).Write
+      # Never check for logger errors.
+      - (github.com/go-kit/log.Logger).Log
diff --git a/collector/collector.go b/collector/collector.go
index ef41c89a..f31e1460 100644
--- a/collector/collector.go
+++ b/collector/collector.go
@@ -343,7 +343,7 @@ type internalMetrics struct {
 	snmpRetries           prometheus.Counter
 }
 
-type collector struct {
+type Collector struct {
 	ctx     context.Context
 	target  string
 	auth    *config.Auth
@@ -391,18 +391,18 @@ func newInternalMetrics(reg prometheus.Registerer) internalMetrics {
 	}
 }
 
-func New(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger, reg prometheus.Registerer) *collector {
+func New(ctx context.Context, target string, auth *config.Auth, module *config.Module, logger log.Logger, reg prometheus.Registerer) *Collector {
 	internalMetrics := newInternalMetrics(reg)
-	return &collector{ctx: ctx, target: target, auth: auth, module: module, logger: logger, metrics: internalMetrics}
+	return &Collector{ctx: ctx, target: target, auth: auth, module: module, logger: logger, metrics: internalMetrics}
 }
 
 // Describe implements Prometheus.Collector.
-func (c collector) Describe(ch chan<- *prometheus.Desc) {
+func (c Collector) Describe(ch chan<- *prometheus.Desc) {
 	ch <- prometheus.NewDesc("dummy", "dummy", nil, nil)
 }
 
 // Collect implements Prometheus.Collector.
-func (c collector) Collect(ch chan<- prometheus.Metric) {
+func (c Collector) Collect(ch chan<- prometheus.Metric) {
 	start := time.Now()
 	results, err := ScrapeTarget(c.ctx, c.target, c.auth, c.module, c.logger, c.metrics)
 	if err != nil {
@@ -465,9 +465,8 @@ func getPduValue(pdu *gosnmp.SnmpPDU) float64 {
 		if *wrapCounters {
 			// Wrap by 2^53.
 			return float64(gosnmp.ToBigInt(pdu.Value).Uint64() % float64Mantissa)
-		} else {
-			return float64(gosnmp.ToBigInt(pdu.Value).Uint64())
 		}
+		return float64(gosnmp.ToBigInt(pdu.Value).Uint64())
 	case gosnmp.OpaqueFloat:
 		return float64(pdu.Value.(float32))
 	case gosnmp.OpaqueDouble:
@@ -530,7 +529,6 @@ func pduToSamples(indexOids []int, pdu *gosnmp.SnmpPDU, metric *config.Metric, o
 	labels := indexesToLabels(indexOids, metric, oidToPdu, metrics)
 
 	value := getPduValue(pdu)
-	t := prometheus.UntypedValue
 
 	labelnames := make([]string, 0, len(labels)+1)
 	labelvalues := make([]string, 0, len(labels)+1)
@@ -539,6 +537,7 @@ func pduToSamples(indexOids []int, pdu *gosnmp.SnmpPDU, metric *config.Metric, o
 		labelvalues = append(labelvalues, v)
 	}
 
+	var t prometheus.ValueType
 	switch metric.Type {
 	case "counter":
 		t = prometheus.CounterValue
@@ -827,9 +826,8 @@ func indexOidsAsString(indexOids []int, typ string, fixedSize int, implied bool,
 		}
 		if len(parts) == 0 {
 			return "", subOid, indexOids
-		} else {
-			return fmt.Sprintf("0x%X", string(parts)), subOid, indexOids
 		}
+		return fmt.Sprintf("0x%X", string(parts)), subOid, indexOids
 	case "DisplayString":
 		var subOid []int
 		length := fixedSize
@@ -867,12 +865,10 @@ func indexOidsAsString(indexOids []int, typ string, fixedSize int, implied bool,
 		value, ok := enumValues[subOid[0]]
 		if ok {
 			return value, subOid, indexOids
-		} else {
-			return fmt.Sprintf("%d", subOid[0]), subOid, indexOids
 		}
+		return fmt.Sprintf("%d", subOid[0]), subOid, indexOids
 	default:
 		panic(fmt.Sprintf("Unknown index type %s", typ))
-		return "", nil, nil
 	}
 }
 
diff --git a/config/config.go b/config/config.go
index 044dc77c..e7004d88 100644
--- a/config/config.go
+++ b/config/config.go
@@ -41,7 +41,7 @@ func LoadFile(filename string) (*Config, error) {
 }
 
 var (
-	defaultRetries int = 3
+	defaultRetries = 3
 
 	DefaultAuth = Auth{
 		Community:     "public",
@@ -69,8 +69,8 @@ var (
 
 // Config for the snmp_exporter.
 type Config struct {
-	Auths   map[string]*Auth   `yaml:"auths",omitempty"`
-	Modules map[string]*Module `yaml:"modules",omitempty"`
+	Auths   map[string]*Auth   `yaml:"auths,omitempty"`
+	Modules map[string]*Module `yaml:"modules,omitempty"`
 	Version int                `yaml:"version"`
 }
 
diff --git a/generator/tree.go b/generator/tree.go
index 5198659d..a4380628 100644
--- a/generator/tree.go
+++ b/generator/tree.go
@@ -232,9 +232,8 @@ func getMetricNode(oid string, node *Node, nameToNode map[string]*Node) (*Node,
 		_, ok = metricType(n.Type)
 		if ok && metricAccess(n.Access) && len(n.Indexes) == 0 {
 			return n, oidScalar
-		} else {
-			return n, oidSubtree
 		}
+		return n, oidSubtree
 	}
 
 	// Unknown OID/name, search Node tree for longest match.