lint(all): fix issues of errorlint etc (hybridgroup#1037)

.golangci.yml

@@ -34,10 +34,11 @@ run:
 linters:
   # currently active linters:
   #
-  #INFO [lintersdb] Active 49 linters: [asasalint asciicheck bidichk contextcheck decorder depguard dupword durationcheck errcheck exportloopref
-  # gocheckcompilerdirectives gochecknoinits gochecksumtype gofmt gofumpt goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper
-  # inamedparam ineffassign makezero mirror misspell musttag nilerr nilnil nolintlint nosprintfhostport perfsprint prealloc protogetter reassign revive
-  # sloglint staticcheck tagalign tenv testableexamples testifylint tparallel unconvert unparam unused wastedassign]
+  # INFO [lintersdb] Active 64 linters: [asasalint asciicheck bidichk bodyclose containedctx contextcheck decorder depguard dogsled dupword durationcheck
+  # errcheck errchkjson errorlint exportloopref forcetypeassert gci gocheckcompilerdirectives gochecknoinits gochecksumtype gocritic gofmt gofumpt goimports
+  # gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper inamedparam ineffassign lll makezero mirror misspell musttag nakedret nilerr nilnil
+  # noctx nolintlint nonamedreturns nosprintfhostport perfsprint prealloc predeclared protogetter reassign revive sloglint staticcheck tagalign tenv
+  # testableexamples testifylint thelper tparallel unconvert unparam unused usestdlibvars wastedassign]
 
   enable-all: true
 
@@ -75,39 +76,23 @@ linters:
     - testpackage # not needed, we use the same name for test package to have access to unexposed items
     - wrapcheck # not needed (we allow errors from interface methods)
     - zerologlint # not needed (related to zerolog package)
-    # important to have
-    - errorlint # useful (reduce bugs), but suppress the "Use `%w` to format errors" check
-    - forcetypeassert # useful (reduce bugs)
-    - nakedret # very useful together with "nonamedreturns" (reduce bugs)
-    - nonamedreturns # very useful (reduce bugs)
     # useful for the future
-    - bodyclose # maybe useful (reduce bugs), exclusions for tests needed
-    - containedctx # useful (structs are not for context wrapping)
     - cyclop # useful with some tweeks (better understandable code), see also gocyclo
-    - dogsled # useful with some tweeks (e.g. exclude tests)
     - dupl # useful with some tweeks (reduce bugs and lines of code)
-    - errchkjson # useful (reduce bugs)
     - errname # useful for common style
     - exhaustruct # useful with some exclusions for existing code (e.g. mavlink/common/common.go)
     - funlen # useful with some tweeks (reduce bugs, simplify code, better understandable code)
-    - gci # useful (improve readability)
     - gocognit # useful with some tweeks (better understandable code)
     - goconst # useful (reduce bugs)
-    - gocritic # useful with some exclusions for existing code (e.g. mavlink/common/common.go)
     - gocyclo # useful with some tweeks (better understandable code)
     - goheader # useful, if we introduce a common header (e.g. for copyright)
     - gomnd # useful with some exclusions for existing code (e.g. mavlink.go)
     - interfacebloat # useful with some exclusions at usage of external packages
-    - lll # useful with some exclusions for existing code (e.g. mavlink/common/common.go)
     - maintidx # useful with some tweeks (better understandable code), maybe use instead "gocyclo", "gocognit" , "cyclop"
     - nestif # useful (reduce bugs, simplify code, better understandable code)
     - nlreturn # more common style, but could become annoying
-    - noctx # maybe good (show used context explicit)
-    - predeclared # useful (reduce bugs)
     - stylecheck # useful with some tweaking (e.g. underscores in names should be allowed - we use it for constants retrieved from C/C++)
     - tagliatelle # maybe useful with some tweaking or excluding
-    - thelper # useful
-    - usestdlibvars # useful
     - varnamelen # maybe useful with some tweaking, but many findings
     - whitespace # more common style, but could become annoying
     - wsl # more common style, but could become annoying
@@ -145,6 +130,36 @@ linters-settings:
       - "and"
       - "a"
 
+  errorlint:
+    # Default: true
+    # %v should be used by default over %w, see https://github.com/uber-go/guide/blob/master/style.md#error-wrapping
+    errorf: false
+    # Permit more than 1 %w verb, valid per Go 1.20 (Requires errorf:true)
+    # Default: true
+    errorf-multi: false
+
+  gci:
+    # Section configuration to compare against.
+    # Section names are case-insensitive and may contain parameters in ().
+    # The default order of sections is `standard > default > custom > blank > dot`,
+    # If `custom-order` is `true`, it follows the order of `sections` option.
+    # Default: ["standard", "default"]
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(gobot.io/x/gobot/) # Custom section: groups all imports with the specified Prefix.
+      #- blank # Blank section: contains all blank imports. This section is not present unless explicitly enabled.
+      #- dot # Dot section: contains all dot imports. This section is not present unless explicitly enabled.
+    # Enable custom order of sections.
+    # If `true`, make the section order the same as the order of `sections`.
+    # Default: false
+    custom-order: true
+
+  gocritic:
+    disabled-checks:
+      - assignOp # very opinionated
+      - appendAssign # mostly used by intention
+
   nolintlint:
     # Enable to require an explanation of nonzero length after each nolint directive.
     # Default: false

api/api.go

@@ -2,7 +2,6 @@ package api
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"log"
 	"net/http"
@@ -11,6 +10,7 @@ import (
 	"time"
 
 	"github.com/bmizerany/pat"
+
 	"gobot.io/x/gobot/v2"
 	"gobot.io/x/gobot/v2/api/robeaux"
 )
@@ -178,12 +178,14 @@ func (a *API) robeaux(res http.ResponseWriter, req *http.Request) {
 		http.Error(res, err.Error(), http.StatusNotFound)
 		return
 	}
-	t := strings.Split(path, ".")
-	if t[len(t)-1] == "js" {
+	split := strings.Split(path, ".")
+	ext := split[len(split)-1]
+	switch ext {
+	case "js":
 		res.Header().Set("Content-Type", "text/javascript; charset=utf-8")
-	} else if t[len(t)-1] == "css" {
+	case "css":
 		res.Header().Set("Content-Type", "text/css; charset=utf-8")
-	} else if t[len(t)-1] == "html" {
+	case "html":
 		res.Header().Set("Content-Type", "text/html; charset=utf-8")
 	}
 	if _, err := res.Write(buf); err != nil {
@@ -269,9 +271,11 @@ func (a *API) robotDeviceEvent(res http.ResponseWriter, req *http.Request) {
 	device := a.master.Robot(req.URL.Query().Get(":robot")).
 		Device(req.URL.Query().Get(":device"))
 
+	//nolint:forcetypeassert // no error return value, so there is no better way
 	if event := a.master.Robot(req.URL.Query().Get(":robot")).
 		Device(req.URL.Query().Get(":device")).(gobot.Eventer).
 		Event(req.URL.Query().Get(":event")); len(event) > 0 {
+		//nolint:forcetypeassert // no error return value, so there is no better way
 		if err := device.(gobot.Eventer).On(event, func(data interface{}) {
 			d, _ := json.Marshal(data)
 			dataChan <- string(d)
@@ -345,6 +349,7 @@ func (a *API) executeRobotDeviceCommand(res http.ResponseWriter, req *http.Reque
 		a.writeJSON(map[string]interface{}{"error": err.Error()}, res)
 	} else {
 		a.executeCommand(
+			//nolint:forcetypeassert // no error return value, so there is no better way
 			a.master.Robot(req.URL.Query().Get(":robot")).
 				Device(req.URL.Query().Get(":device")).(gobot.Commander).
 				Command(req.URL.Query().Get(":command")),
@@ -387,7 +392,10 @@ func (a *API) executeCommand(f func(map[string]interface{}) interface{},
 
 // writeJSON writes `j` as JSON in response
 func (a *API) writeJSON(j interface{}, res http.ResponseWriter) {
-	data, _ := json.Marshal(j)
+	data, err := json.Marshal(j)
+	if err != nil {
+		panic(err)
+	}
 	res.Header().Set("Content-Type", "application/json; charset=utf-8")
 	if _, err := res.Write(data); err != nil {
 		panic(err)
@@ -401,29 +409,25 @@ func (a *API) Debug() {
 	})
 }
 
-func (a *API) jsonRobotFor(name string) (jrobot *gobot.JSONRobot, err error) {
+func (a *API) jsonRobotFor(name string) (*gobot.JSONRobot, error) {
 	if robot := a.master.Robot(name); robot != nil {
-		jrobot = gobot.NewJSONRobot(robot)
-	} else {
-		err = errors.New("No Robot found with the name " + name)
+		return gobot.NewJSONRobot(robot), nil
 	}
-	return
+	return nil, fmt.Errorf("No Robot found with the name %s", name)
 }
 
-func (a *API) jsonDeviceFor(robot string, name string) (jdevice *gobot.JSONDevice, err error) {
+func (a *API) jsonDeviceFor(robot string, name string) (*gobot.JSONDevice, error) {
 	if device := a.master.Robot(robot).Device(name); device != nil {
-		jdevice = gobot.NewJSONDevice(device)
-	} else {
-		err = errors.New("No Device found with the name " + name)
+		return gobot.NewJSONDevice(device), nil
 	}
-	return
+
+	return nil, fmt.Errorf("No Device found with the name %s", name)
 }
 
-func (a *API) jsonConnectionFor(robot string, name string) (jconnection *gobot.JSONConnection, err error) {
+func (a *API) jsonConnectionFor(robot string, name string) (*gobot.JSONConnection, error) {
 	if connection := a.master.Robot(robot).Connection(name); connection != nil {
-		jconnection = gobot.NewJSONConnection(connection)
-	} else {
-		err = errors.New("No Connection found with the name " + name)
+		return gobot.NewJSONConnection(connection), nil
 	}
-	return
+
+	return nil, fmt.Errorf("No Connection found with the name %s", name)
 }

api/api_test.go

@@ -1,3 +1,4 @@
+//nolint:forcetypeassert,usestdlibvars,bodyclose,noctx // ok here
 package api
 
 import (
@@ -12,6 +13,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+
 	"gobot.io/x/gobot/v2"
 )
 
@@ -413,7 +415,7 @@ func TestRobotDeviceEvent(t *testing.T) {
 			data, _ := reader.ReadString('\n')
 			assert.Equal(t, "data: \"event-data\"\n", data)
 			done = true
-		case <-time.After(100 * time.Millisecond):
+		case <-time.After(200 * time.Millisecond):
 			t.Error("Not receiving data")
 			done = true
 		}

api/basic_auth_test.go

@@ -1,3 +1,4 @@
+//nolint:usestdlibvars,noctx // ok here
 package api
 
 import (

api/cors.go

@@ -38,23 +38,22 @@ func AllowRequestsFrom(allowedOrigins ...string) http.HandlerFunc {
 }
 
 // isOriginAllowed returns true if origin matches an allowed origin pattern.
-func (c *CORS) isOriginAllowed(origin string) (allowed bool) {
+func (c *CORS) isOriginAllowed(origin string) bool {
 	for _, allowedOriginPattern := range c.allowOriginPatterns {
-		allowed, _ = regexp.MatchString(allowedOriginPattern, origin)
-		if allowed {
-			return
+		if allowed, _ := regexp.MatchString(allowedOriginPattern, origin); allowed {
+			return true
 		}
 	}
-	return
+	return false
 }
 
 // generatePatterns generates regex expression for AllowOrigins
 func (c *CORS) generatePatterns() {
 	if c.AllowOrigins != nil {
 		for _, origin := range c.AllowOrigins {
 			pattern := regexp.QuoteMeta(origin)
-			pattern = strings.Replace(pattern, "\\*", ".*", -1)
-			pattern = strings.Replace(pattern, "\\?", ".", -1)
+			pattern = strings.ReplaceAll(pattern, "\\*", ".*")
+			pattern = strings.ReplaceAll(pattern, "\\?", ".")
 			c.allowOriginPatterns = append(c.allowOriginPatterns, "^"+pattern+"$")
 		}
 	}

api/cors_test.go

@@ -1,3 +1,4 @@
+//nolint:usestdlibvars,noctx // ok here
 package api
 
 import (

api/helpers_test.go

@@ -1,3 +1,4 @@
+//nolint:forcetypeassert // ok here
 package api
 
 import (
@@ -28,8 +29,8 @@ type testDriver struct {
 	gobot.Eventer
 }
 
-func (t *testDriver) Start() (err error)           { return }
-func (t *testDriver) Halt() (err error)            { return }
+func (t *testDriver) Start() error                 { return nil }
+func (t *testDriver) Halt() error                  { return nil }
 func (t *testDriver) Name() string                 { return t.name }
 func (t *testDriver) SetName(n string)             { t.name = n }
 func (t *testDriver) Pin() string                  { return t.pin }
@@ -65,15 +66,15 @@ type testAdaptor struct {
 }
 
 var (
-	testAdaptorConnect  = func() (err error) { return }
-	testAdaptorFinalize = func() (err error) { return }
+	testAdaptorConnect  = func() error { return nil }
+	testAdaptorFinalize = func() error { return nil }
 )
 
-func (t *testAdaptor) Finalize() (err error) { return testAdaptorFinalize() }
-func (t *testAdaptor) Connect() (err error)  { return testAdaptorConnect() }
-func (t *testAdaptor) Name() string          { return t.name }
-func (t *testAdaptor) SetName(n string)      { t.name = n }
-func (t *testAdaptor) Port() string          { return t.port }
+func (t *testAdaptor) Finalize() error  { return testAdaptorFinalize() }
+func (t *testAdaptor) Connect() error   { return testAdaptorConnect() }
+func (t *testAdaptor) Name() string     { return t.name }
+func (t *testAdaptor) SetName(n string) { t.name = n }
+func (t *testAdaptor) Port() string     { return t.port }
 
 func newTestAdaptor(name string, port string) *testAdaptor {
 	return &testAdaptor{

connection.go

@@ -40,8 +40,9 @@ func (c *Connections) Each(f func(Connection)) {
 }
 
 // Start calls Connect on each Connection in c
-func (c *Connections) Start() (err error) {
+func (c *Connections) Start() error {
 	log.Println("Starting connections...")
+	var err error
 	for _, connection := range *c {
 		info := "Starting connection " + connection.Name()
 
@@ -59,7 +60,8 @@ func (c *Connections) Start() (err error) {
 }
 
 // Finalize calls Finalize on each Connection in c
-func (c *Connections) Finalize() (err error) {
+func (c *Connections) Finalize() error {
+	var err error
 	for _, connection := range *c {
 		if cerr := connection.Finalize(); cerr != nil {
 			err = multierror.Append(err, cerr)

device.go

@@ -53,8 +53,9 @@ func (d *Devices) Each(f func(Device)) {
 }
 
 // Start calls Start on each Device in d
-func (d *Devices) Start() (err error) {
+func (d *Devices) Start() error {
 	log.Println("Starting devices...")
+	var err error
 	for _, device := range *d {
 		info := "Starting device " + device.Name()
 
@@ -71,7 +72,8 @@ func (d *Devices) Start() (err error) {
 }
 
 // Halt calls Halt on each Device in d
-func (d *Devices) Halt() (err error) {
+func (d *Devices) Halt() error {
+	var err error
 	for _, device := range *d {
 		if derr := device.Halt(); derr != nil {
 			err = multierror.Append(err, derr)

doc.go

@@ -1,9 +1,11 @@
 // Copyright 2014-2018 The Hybrid Group. All rights reserved.
 
 /*
-Package gobot is the primary entrypoint for Gobot (http://gobot.io), a framework for robotics, physical computing, and the Internet of Things written using the Go programming language .
+Package gobot is the primary entrypoint for Gobot (http://gobot.io), a framework for robotics, physical computing, and
+the Internet of Things written using the Go programming language .
 
-It provides a simple, yet powerful way to create solutions that incorporate multiple, different hardware devices at the same time.
+It provides a simple, yet powerful way to create solutions that incorporate multiple, different hardware devices at the
+same time.
 
 # Classic Gobot
 
@@ -40,7 +42,8 @@ Here is a "Classic Gobot" program that blinks an LED using an Arduino:
 
 # Metal Gobot
 
-You can also use Metal Gobot and pick and choose from the various Gobot packages to control hardware with nothing but pure idiomatic Golang code. For example:
+You can also use Metal Gobot and pick and choose from the various Gobot packages to control hardware with nothing but
+pure idiomatic Golang code. For example:
 
 	package main
 

drivers/aio/aio.go

@@ -28,5 +28,5 @@ type AnalogReader interface {
 // AnalogWriter interface represents an Adaptor which has AnalogWrite capabilities
 type AnalogWriter interface {
 	// gobot.Adaptor
-	AnalogWrite(pin string, val int) (err error)
+	AnalogWrite(pin string, val int) error
 }