From e85c31782b88b38f5713a823971dc9c191183edd Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Sat, 27 Aug 2022 23:16:01 +0100 Subject: [PATCH] Fix misc. linter issues (comments, variable naming, etc.) We've accumulated a few linter issues around comments and a couple of variable names. While none of them is major, this patch cleans them up so it's easier to go through the linter output, and we can start being more strict about it. --- .cirrus.yml | 3 +-- internal/aliases/aliases_test.go | 10 +++++----- internal/courier/mda_test.go | 2 ++ internal/expvarom/expvarom_test.go | 3 ++- internal/smtpsrv/server.go | 2 ++ internal/testlib/testlib.go | 2 +- test/t-03-queue_persistency/addtoqueue.go | 1 + test/util/conngen/conngen.go | 6 +++++- test/util/coverhtml/coverhtml.go | 4 ++++ test/util/loadgen/loadgen.go | 2 +- test/util/minidns/minidns.go | 8 ++++++-- 11 files changed, 30 insertions(+), 13 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 4c1bf2e..e2854cd 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -37,8 +37,7 @@ lint_task: - linters: - staticcheck # SA1015: time.Tick leaks but it's ok on endless functions. - # SA1019: BuildNameToCertificate deprecation, remove on Go >= 1.14. - text: "SA1015:|SA1019:" + text: "SA1015:" exclude-use-default: true lint_config_file: path: /tmp/lint.yml diff --git a/internal/aliases/aliases_test.go b/internal/aliases/aliases_test.go index a0e091b..9df92aa 100644 --- a/internal/aliases/aliases_test.go +++ b/internal/aliases/aliases_test.go @@ -60,11 +60,11 @@ func usersWithXDontExist(user, domain string) (bool, error) { return true, nil } -var userLookupError = errors.New("test error userLookupError") +var errUserLookup = errors.New("test error errUserLookup") func usersWithXErrorYDontExist(user, domain string) (bool, error) { if strings.HasPrefix(user, "x") { - return false, userLookupError + return false, errUserLookup } if strings.HasPrefix(user, "y") { return false, nil @@ -134,12 +134,12 @@ func TestUserLookupErrors(t *testing.T) { cases := Cases{ {"a@dom", []Recipient{{"a@remote", EMAIL}}, nil}, - {"b@dom", nil, userLookupError}, + {"b@dom", nil, errUserLookup}, {"c@dom", []Recipient{{"c@dom", EMAIL}}, nil}, - {"x@dom", nil, userLookupError}, + {"x@dom", nil, errUserLookup}, // This one goes through the catch-all. - {"y@dom", nil, userLookupError}, + {"y@dom", nil, errUserLookup}, } cases.check(t, resolver) } diff --git a/internal/courier/mda_test.go b/internal/courier/mda_test.go index 30738db..5239384 100644 --- a/internal/courier/mda_test.go +++ b/internal/courier/mda_test.go @@ -113,6 +113,8 @@ func TestSanitize(t *testing.T) { // http://www.user.uni-hannover.de/nhtcapri/bidirectional-text.html // We allow them, they're the same on both sides. {"١٩٩٩–١٢–٣١", "١٩٩٩–١٢–٣١"}, + + //lint:ignore ST1018 The use of a literal U+200C is intentional. {"موزه‌ها", "موزه\u200cها"}, } for _, c := range cases { diff --git a/internal/expvarom/expvarom_test.go b/internal/expvarom/expvarom_test.go index 511ce97..fa43c39 100644 --- a/internal/expvarom/expvarom_test.go +++ b/internal/expvarom/expvarom_test.go @@ -20,7 +20,8 @@ var ( testMXI = expvar.NewMap("testMXI") testMXF = expvar.NewMap("testMXF") - testMEmpty = expvar.NewMap("testMEmpty") //nolint // Unused. + //lint:ignore U1000 Intentionally unused, should not be exported. + testMEmpty = expvar.NewMap("testMEmpty") //nolint testMOther = expvar.NewMap("testMOther") diff --git a/internal/smtpsrv/server.go b/internal/smtpsrv/server.go index 80484a4..f07c7b2 100644 --- a/internal/smtpsrv/server.go +++ b/internal/smtpsrv/server.go @@ -174,6 +174,8 @@ func (s *Server) periodicallyReload() { if reloadEvery == nil { return } + + //lint:ignore SA1015 This lasts the program's lifetime. for range time.Tick(*reloadEvery) { err := s.aliasesR.Reload() if err != nil { diff --git a/internal/testlib/testlib.go b/internal/testlib/testlib.go index 461764d..66541d8 100644 --- a/internal/testlib/testlib.go +++ b/internal/testlib/testlib.go @@ -137,7 +137,7 @@ func (c dumbCourier) Deliver(from string, to string, data []byte) (error, bool) // DumbCourier always succeeds delivery, and ignores everything. var DumbCourier = dumbCourier{} -// generateCert generates a new, INSECURE self-signed certificate and writes +// GenerateCert generates a new, INSECURE self-signed certificate and writes // it to a pair of (cert.pem, key.pem) files to the given path. // Note the certificate is only useful for testing purposes. func GenerateCert(path string) (*tls.Config, error) { diff --git a/test/t-03-queue_persistency/addtoqueue.go b/test/t-03-queue_persistency/addtoqueue.go index 55bb8e0..9e71ae3 100644 --- a/test/t-03-queue_persistency/addtoqueue.go +++ b/test/t-03-queue_persistency/addtoqueue.go @@ -4,6 +4,7 @@ // Note that chasquid does NOT support this, we do it before starting up the // daemon for testing purposes only. // +//go:build ignore // +build ignore package main diff --git a/test/util/conngen/conngen.go b/test/util/conngen/conngen.go index e7f1764..8e7dd82 100644 --- a/test/util/conngen/conngen.go +++ b/test/util/conngen/conngen.go @@ -31,7 +31,6 @@ var ( var ( host string - exit bool ) func main() { @@ -73,8 +72,13 @@ func main() { time.Sleep(24 * time.Hour) } } + + for _, c := range conns { + c.close() + } } +// C represents a single connection. type C struct { tr trace.Trace n net.Conn diff --git a/test/util/coverhtml/coverhtml.go b/test/util/coverhtml/coverhtml.go index f9340ea..ff57af1 100644 --- a/test/util/coverhtml/coverhtml.go +++ b/test/util/coverhtml/coverhtml.go @@ -90,6 +90,7 @@ func main() { fmt.Printf("Total: %.1f\n", totals.TotalPercent()) } +// Totals is used to keep track of total counters. type Totals struct { // Total statements. total int @@ -104,6 +105,7 @@ type Totals struct { coveredF map[string]int } +// Add the given profile to the total counters. func (t *Totals) Add(p *cover.Profile) { for _, b := range p.Blocks { t.total += b.NumStmt @@ -115,10 +117,12 @@ func (t *Totals) Add(p *cover.Profile) { } } +// Percent covered for the given file. func (t *Totals) Percent(f string) float32 { return float32(t.coveredF[f]) / float32(t.totalF[f]) * 100 } +// TotalPercent covered, across all files. func (t *Totals) TotalPercent() float32 { return float32(t.covered) / float32(t.total) * 100 } diff --git a/test/util/loadgen/loadgen.go b/test/util/loadgen/loadgen.go index 6ff98ec..00897d6 100644 --- a/test/util/loadgen/loadgen.go +++ b/test/util/loadgen/loadgen.go @@ -126,7 +126,7 @@ func serial(id int) { var count int64 start := time.Now() for { - count += 1 + count++ err := one() if err != nil { log.Fatalf("%v", err) diff --git a/test/util/minidns/minidns.go b/test/util/minidns/minidns.go index 4e1c818..7fc7911 100644 --- a/test/util/minidns/minidns.go +++ b/test/util/minidns/minidns.go @@ -119,7 +119,11 @@ func (m *miniDNS) listenAndServeUDP(addr string) { log.Fatalf("error packing reply: %v", err) } - conn.WriteTo(rbuf, addr) + _, err = conn.WriteTo(rbuf, addr) + if err != nil { + log.Infof("%v/%-5d error writing: %v", + addr, msg.ID, err) + } } } @@ -245,7 +249,7 @@ func (m *miniDNS) loadZones(f *os.File) { continue } - vs := regexp.MustCompile("\\s+").Split(line, 3) + vs := regexp.MustCompile(`\s+`).Split(line, 3) if len(vs) != 3 { log.Fatalf("line %d: invalid format", lineno) }