build(style): switch to gofumpt and add linters (#1009)

.golangci.yml

@@ -31,9 +31,10 @@ run:
 linters:
   # currently active linters:
   #
-  #INFO [lintersdb] Active 35 linters: [asasalint asciicheck bidichk contextcheck decorder depguard durationcheck errcheck exportloopref
-  # gocheckcompilerdirectives gomoddirectives gomodguard goprintffuncname gosimple govet grouper ineffassign makezero mirror musttag
-  # nilerr nilnil nolintlint nosprintfhostport prealloc reassign revive staticcheck tagalign tenv testableexamples tparallel typecheck unused wastedassign]
+  #INFO [lintersdb] Active 42 linters: [asasalint asciicheck bidichk contextcheck decorder depguard dupword durationcheck errcheck exportloopref
+  # gocheckcompilerdirectives gofmt gofumpt goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper ineffassign makezero mirror
+  # misspell musttag nilerr nilnil nolintlint nosprintfhostport prealloc reassign revive staticcheck tagalign tenv testableexamples tparallel unconvert unparam
+  # unused wastedassign]
 
   enable-all: true
 
@@ -60,7 +61,6 @@ linters:
     - godot # not used (seems to be counting peas)
     - godox # not used (we have many TODOs, so not useful)
     - goerr113 # not used (we allow error creation at return statement)
-    - gofumpt # not used (we use "go fmt" or "gofmt" not gofumpt"
     - gosmopolitan # not needed (report i18n/l10n anti-patterns)
     - importas # not needed (there is no alias rule at the moment)
     - ireturn # not used (we allow return interfaces)
@@ -73,23 +73,19 @@ linters:
     - wrapcheck # not needed (we allow errors from interface methods)
     - zerologlint # not needed (related to zerolog package)
     # important to have
-    - gofmt # important to prevent "format tsunami" ("gofmt -s -w" missed), disabled due to "https://github.com/golangci/golangci-lint-action/issues/535"
+    - 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)
-    - unconvert # very useful (reduce bugs, simplify code)
-    - unparam # very useful (reduce bugs, simplify code)
     # 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)
-    - dupword # useful with some tweeks, but not important
     - errchkjson # useful (reduce bugs)
     - errname # useful for common style
-    - errorlint # useful (reduce bugs), but suppress the "Use `%w` to format errors" check
     - exhaustruct # useful with some exclusions for existing code (e.g. mavlink/common/common.go)
-    - forcetypeassert # useful (reduce bugs)
     - funlen # useful with some tweeks (reduce bugs, simplify code, better understandable code)
     - gci # useful (improve readability)
     - gochecknoinits # useful (reduce bugs, simplify bug catching)
@@ -98,13 +94,10 @@ linters:
     - 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)
-    - goimports # useful (common style), but not important
     - gomnd # useful with some exclusions for existing code (e.g. mavlink.go)
-    - gosec # useful (security)
     - 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"
-    - misspell # useful (better understandable code), but not important
     - nestif # useful (reduce bugs, simplify code, better understandable code)
     - nlreturn # more common style, but could become annoying
     - noctx # maybe good (show used context explicit)
@@ -141,6 +134,15 @@ linters-settings:
           - pkg: "github.com/pkg/errors"
             desc: Should be replaced by standard lib errors package
 
+  dupword:
+    # Keywords for detecting duplicate words.
+    # If this list is not empty, only the words defined in this list will be detected.
+    # Default: []
+    keywords:
+      - "the"
+      - "and"
+      - "a"
+
   nolintlint:
     # Enable to require an explanation of nonzero length after each nolint directive.
     # Default: false

CONTRIBUTING.md

@@ -41,7 +41,7 @@ Descriptions for each of these will eventually be provided below.
   from time to time but they can complicate merges and should be done separately.
 * Take care to maintain the existing coding style.
 * `golangci-lint` your code, see [instruction for local installation](https://golangci-lint.run/usage/install/#local-installation)
-* `go fmt` your code (with the go version of go.mod)
+* `gofumpt` your code (the go version will be automatically obtained from go.mod), see [instructions](https://github.com/mvdan/gofumpt/blob/master/README.md)
 * Add unit tests for any new or changed functionality.
 * All pull requests should be "fast forward"
   * If there are commits after yours use “git rebase -i <new_head_branch>”

Makefile

@@ -50,14 +50,14 @@ version_check:
 	echo "go: $${gv}.*, go.mod: $${mv}" ; \
 	if [ "$${gv}" != "$${mv}" ]; then exit 50; fi ; \
 
-# Check for bad code style and other issues
+# Check for bad code style and other issues (gofumpt and gofmt check is activated for the linter)
 fmt_check:
-	gofmt -l -s .
 	golangci-lint run -v
 
-# Fix bad code style (will only be executed, on version match)
-fmt_fix: version_check
-	gofmt -l -s -w .
+# Fix bad code style (the go version will be automatically obtained from go.mod)
+fmt_fix: 
+	$(MAKE) version_check || true
+	gofumpt -l -w .
 
 examples: $(EXAMPLES)
 

adaptor.go

@@ -109,7 +109,6 @@ type PWMPinnerProvider interface {
 // Data (8 bits): A plain data byte. DataLow and DataHigh represent the low and high byte of a 16 bit word.
 // Count (8 bits): A data byte containing the length of a block operation.
 // [..]: Data sent by I2C device, as opposed to data sent by the host adapter.
-//
 type I2cSystemDevicer interface {
 	// ReadByte must be implemented as the sequence:
 	// "S Addr Rd [A] [Data] NA P"

api/api.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"time"
 
 	"github.com/bmizerany/pat"
 	"gobot.io/x/gobot/v2"
@@ -35,16 +36,20 @@ func NewAPI(m *gobot.Master) *API {
 		start: func(a *API) {
 			log.Println("Initializing API on " + a.Host + ":" + a.Port + "...")
 			http.Handle("/", a)
+			server := &http.Server{
+				Addr:              a.Host + ":" + a.Port,
+				ReadHeaderTimeout: 30 * time.Second,
+			}
 
 			go func() {
 				if a.Cert != "" && a.Key != "" {
-					if err := http.ListenAndServeTLS(a.Host+":"+a.Port, a.Cert, a.Key, nil); err != nil {
+					if err := server.ListenAndServeTLS(a.Cert, a.Key); err != nil {
 						panic(err)
 					}
 				} else {
 					log.Println("WARNING: API using insecure connection. " +
 						"We recommend using an SSL certificate with Gobot.")
-					if err := http.ListenAndServe(a.Host+":"+a.Port, nil); err != nil {
+					if err := server.ListenAndServe(); err != nil {
 						panic(err)
 					}
 				}
@@ -313,7 +318,6 @@ func (a *API) robotConnections(res http.ResponseWriter, req *http.Request) {
 	} else {
 		a.writeJSON(map[string]interface{}{"error": "No Robot found with the name " + req.URL.Query().Get(":robot")}, res)
 	}
-
 }
 
 // robotConnection returns connection route handler
@@ -369,7 +373,6 @@ func (a *API) executeCommand(f func(map[string]interface{}) interface{},
 	res http.ResponseWriter,
 	req *http.Request,
 ) {
-
 	body := make(map[string]interface{})
 	if err := json.NewDecoder(req.Body).Decode(&body); err != nil {
 		panic(err)

api/api_test.go

@@ -333,7 +333,6 @@ func TestExecuteRobotDeviceCommand(t *testing.T) {
 
 	_ = json.NewDecoder(response.Body).Decode(&body)
 	assert.Equal(t, "No Device found with the name UnknownDevice1", body.(map[string]interface{})["error"])
-
 }
 
 func TestRobotConnections(t *testing.T) {

api/doc.go

@@ -3,35 +3,35 @@ Package api provides a webserver to interact with your Gobot program over the ne
 
 Example:
 
-  package main
+	package main
 
-  import (
-    "fmt"
+	import (
+	  "fmt"
 
-    "gobot.io/x/gobot/v2"
-    "gobot.io/x/gobot/v2/api"
-  )
+	  "gobot.io/x/gobot/v2"
+	  "gobot.io/x/gobot/v2/api"
+	)
 
-  func main() {
-    gbot := gobot.NewMaster()
+	func main() {
+	  gbot := gobot.NewMaster()
 
-    // Starts the API server on default port 3000
-    api.NewAPI(gbot).Start()
+	  // Starts the API server on default port 3000
+	  api.NewAPI(gbot).Start()
 
-    // Accessible via http://localhost:3000/api/commands/say_hello
-    gbot.AddCommand("say_hello", func(params map[string]interface{}) interface{} {
-      return "Master says hello!"
-    })
+	  // Accessible via http://localhost:3000/api/commands/say_hello
+	  gbot.AddCommand("say_hello", func(params map[string]interface{}) interface{} {
+	    return "Master says hello!"
+	  })
 
-    hello := gbot.AddRobot(gobot.NewRobot("Eve"))
+	  hello := gbot.AddRobot(gobot.NewRobot("Eve"))
 
-    // Accessible via http://localhost:3000/api/robots/Eve/commands/say_hello
-    hello.AddCommand("say_hello", func(params map[string]interface{}) interface{} {
-      return fmt.Sprintf("%v says hello!", hello.Name)
-    })
+	  // Accessible via http://localhost:3000/api/robots/Eve/commands/say_hello
+	  hello.AddCommand("say_hello", func(params map[string]interface{}) interface{} {
+	    return fmt.Sprintf("%v says hello!", hello.Name)
+	  })
 
-    gbot.Start()
-  }
+	  gbot.Start()
+	}
 
 It follows Common Protocol for Programming Physical Input and Output (CPPP-IO) spec:
 https://gobot.io/x/cppp-io

api/helpers_test.go

@@ -64,8 +64,10 @@ type testAdaptor struct {
 	port string
 }
 
-var testAdaptorConnect = func() (err error) { return }
-var testAdaptorFinalize = func() (err error) { return }
+var (
+	testAdaptorConnect  = func() (err error) { return }
+	testAdaptorFinalize = func() (err error) { return }
+)
 
 func (t *testAdaptor) Finalize() (err error) { return testAdaptorFinalize() }
 func (t *testAdaptor) Connect() (err error)  { return testAdaptorConnect() }