chore(lint): Increasing lint difficulty level (#370)

Adding lots of linters

Author: fclairamb

.github/workflows/build.yml

@@ -6,7 +6,7 @@ on:
   push:
     branches:
       - main
-      - chore/lint-*
+      - chore/lint*
   pull_request:
 
 permissions:
@@ -26,7 +26,7 @@ jobs:
 
     strategy:
       matrix:
-        go: [ '1.22', '1.21', '1.20' ]
+        go: [ '1.22', '1.21' ]
         include:
           - go: '1.22'
             lint: true
@@ -54,11 +54,7 @@ jobs:
         run: go build -v ./...
 
       - name: Test
-        # We need GCC because of the "go test -race"
-        # env:
-        #   CGO_ENABLED: 0
         run: |
-          apt-get update && apt-get install gcc -y
           go test -parallel 20 -v -race -coverprofile=coverage.txt -covermode=atomic ./...
           bash <(curl -s https://codecov.io/bash)
 

.gitignore

@@ -3,5 +3,5 @@ ftpserver
 .idea
 .vscode
 debug
-__debug_bin
+__debug_bin*
 *.toml

.golangci.yml

@@ -160,61 +160,128 @@ linters-settings:
 linters:
     disable-all: true
     enable:
+        # https://golangci-lint.run/usage/linters/
         - asciicheck
+        - asasalint
+        - bidichk
         - bodyclose
         # - deadcode -> unused
+        - containedctx
+        # - contextcheck -> creates an odd error here: https://github.com/fclairamb/ftpserverlib/blob/4d7c663e9e0b2650673fc2e0fcdb443895f2a1b9/server.go#L234
+        # - copyloopvar -> unknown in v1.56.2 ???
+        # - cyclop -> Delaying it for now (too much work)
+        - decorder
         - depguard
         - dogsled
         - dupl
+        - dupword
+        - durationcheck
         - errcheck
         - exhaustive
+        - errchkjson
+        - errname
+        - errorlint
+        - execinquery
+        - exhaustive
+        # - exhaustruct --> Not convinced it's useful
         # - exhaustivestruct
         - exportloopref
         - funlen
-        # - gci
+        - forbidigo
+        - forcetypeassert
+        - gci
+        - ginkgolinter
         - gochecknoinits
-        # - gochecknoglobals
+        - gochecksumtype
+        - gochecknoglobals
         - gocognit
         - goconst
         - gocritic
         - gocyclo
+        # - godot --> lots of not so useful changes
         - godox
         - goerr113
         - gofmt
+        # - gofumpt -> conflicts with wsl
         - goimports
+        - gosimple
         # - golint --> revive
         - revive
-        # - gomnd
+        # - gomnd --> too much work
+        # - gomoddirectives
+        # - gomodguard
         - goprintffuncname
         - gosec
+        - gosmopolitan
         - gosimple
         - govet
+        - grouper
         - ineffassign
+        - importas
+        - inamedparam
+        # - intrange --> upcoming
+        # - interfacebloat
         # - interfacer --> (deprecated)
+        # - ireturn --> I can't even see how to fix those like ClientHandler::getFileHandle
         - lll
+        - loggercheck
+        - maintidx
+        - makezero
+        - mirror
         # - maligned --> govet:fieldalignment
         - megacheck
         - misspell
+        - musttag
         - nakedret
         # - nestif
         - nlreturn
         - prealloc
+        - nestif
         - nilerr
+        - nilnil
         - nolintlint
+        - nlreturn
         - rowserrcheck
+        - noctx
+        - nonamedreturns
         # - scopelint --> exportloopref
+        - nosprintfhostport
         - exportloopref 
         - staticcheck
         # - structcheck -> unused
         - stylecheck
+        # - paralleltest -> buggy, doesn't work with subtests
         - typecheck
+        - perfsprint
+        - prealloc
+        - predeclared
+        - reassign
+        - promlinter
+        - protogetter
+        - rowserrcheck
+        - sloglint
+        - spancheck
+        - sqlclosecheck
+        - stylecheck
+        - tagalign
+        - tagliatelle
+        - tenv
+        - testableexamples
+        - testifylint
+        # - testpackage -> too late for that
+        - thelper
+        - tparallel
         - unconvert
         - unparam
         - unused
+        - usestdlibvars
+        - varnamelen
+        - wastedassign
         # - varcheck -> unused
         - whitespace
-        # - wrapcheck
+        # - wrapcheck -> too much effort for now
         - wsl
+        # - zerologlint -> Will most probably never use it
     fast: false
 
 issues:
@@ -224,3 +291,7 @@ issues:
   # Default value for this option is true.
   exclude-use-default: false
 
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        - gochecknoglobals

README.md

@@ -47,7 +47,7 @@ If you're interested in a fully featured FTP server, you should use [sftpgo](htt
 The easiest way to test this library is to use [ftpserver](https://github.com/fclairamb/ftpserver).
 
 ## The driver
-The simplest way to get a good understanding of how the driver shall be implemented, you can have a look at the [tests driver](https://github.com/fclairamb/ftpserverlib/blob/master/driver_test.go).
+The simplest way to get a good understanding of how the driver shall be implemented is to look at the [tests driver](https://github.com/fclairamb/ftpserverlib/blob/master/driver_test.go).
 
 ### The base API
 

asciiconverter.go

@@ -10,6 +10,8 @@ type convertMode int8
 const (
 	convertModeToCRLF convertMode = iota
 	convertModeToLF
+
+	bufferSize = 4096
 )
 
 type asciiConverter struct {
@@ -19,7 +21,7 @@ type asciiConverter struct {
 }
 
 func newASCIIConverter(r io.Reader, mode convertMode) *asciiConverter {
-	reader := bufio.NewReaderSize(r, 4096)
+	reader := bufio.NewReaderSize(r, bufferSize)
 
 	return &asciiConverter{
 		reader:    reader,
@@ -28,30 +30,32 @@ func newASCIIConverter(r io.Reader, mode convertMode) *asciiConverter {
 	}
 }
 
-func (c *asciiConverter) Read(p []byte) (n int, err error) {
+func (c *asciiConverter) Read(bytes []byte) (int, error) {
 	var data []byte
+	var readBytes int
+	var err error
 
 	if len(c.remaining) > 0 {
 		data = c.remaining
 		c.remaining = nil
 	} else {
 		data, _, err = c.reader.ReadLine()
 		if err != nil {
-			return n, err
+			return readBytes, err
 		}
 	}
 
-	n = len(data)
-	if n > 0 {
-		maxSize := len(p) - 2
-		if n > maxSize {
-			copy(p, data[:maxSize])
+	readBytes = len(data)
+	if readBytes > 0 {
+		maxSize := len(bytes) - 2
+		if readBytes > maxSize {
+			copy(bytes, data[:maxSize])
 			c.remaining = data[maxSize:]
 
 			return maxSize, nil
 		}
 
-		copy(p[:n], data[:n])
+		copy(bytes[:readBytes], data[:readBytes])
 	}
 
 	// we can have a partial read if the line is too long
@@ -64,22 +68,22 @@ func (c *asciiConverter) Read(p []byte) (n int, err error) {
 	// client transfers it in ASCII mode
 	err = c.reader.UnreadByte()
 	if err != nil {
-		return n, err
+		return readBytes, err
 	}
 
 	lastByte, err := c.reader.ReadByte()
 
 	if err == nil && lastByte == '\n' {
 		switch c.mode {
 		case convertModeToCRLF:
-			p[n] = '\r'
-			p[n+1] = '\n'
-			n += 2
+			bytes[readBytes] = '\r'
+			bytes[readBytes+1] = '\n'
+			readBytes += 2
 		case convertModeToLF:
-			p[n] = '\n'
-			n++
+			bytes[readBytes] = '\n'
+			readBytes++
 		}
 	}
 
-	return n, err
+	return readBytes, err //nolint:wrapcheck // here wrapping errors brings nothing
 }

asciiconverter_test.go

@@ -12,15 +12,15 @@ func TestASCIIConvert(t *testing.T) {
 	lines := []byte("line1\r\nline2\r\n\r\nline4")
 	src := bytes.NewBuffer(lines)
 	dst := bytes.NewBuffer(nil)
-	c := newASCIIConverter(src, convertModeToLF)
-	_, err := io.Copy(dst, c)
+	converter := newASCIIConverter(src, convertModeToLF)
+	_, err := io.Copy(dst, converter)
 	require.NoError(t, err)
 	require.Equal(t, []byte("line1\nline2\n\nline4"), dst.Bytes())
 
 	lines = []byte("line1\nline2\n\nline4")
 	dst = bytes.NewBuffer(nil)
-	c = newASCIIConverter(bytes.NewBuffer(lines), convertModeToCRLF)
-	_, err = io.Copy(dst, c)
+	converter = newASCIIConverter(bytes.NewBuffer(lines), convertModeToCRLF)
+	_, err = io.Copy(dst, converter)
 	require.NoError(t, err)
 	require.Equal(t, []byte("line1\r\nline2\r\n\r\nline4"), dst.Bytes())
 
@@ -31,8 +31,8 @@ func TestASCIIConvert(t *testing.T) {
 	}
 
 	dst = bytes.NewBuffer(nil)
-	c = newASCIIConverter(bytes.NewBuffer(buf), convertModeToCRLF)
-	_, err = io.Copy(dst, c)
+	converter = newASCIIConverter(bytes.NewBuffer(buf), convertModeToCRLF)
+	_, err = io.Copy(dst, converter)
 	require.NoError(t, err)
 	require.Equal(t, buf, dst.Bytes())
 }