lint by golangci-lint (#3080)

Author: fatedier

.circleci/config.yml

@@ -2,14 +2,16 @@ version: 2
 jobs:
   go-version-latest:
     docker:
-      - image: cimg/go:1.18-node
+      - image: cimg/go:1.19-node
+    resource_class: large
     steps:
       - checkout
       - run: make
       - run: make alltest
   go-version-last:
     docker:
-      - image: cimg/go:1.17-node
+      - image: cimg/go:1.18-node
+    resource_class: large
     steps:
       - checkout
       - run: make

.github/workflows/golangci-lint.yml

@@ -0,0 +1,41 @@
+name: golangci-lint
+on:
+  push:
+    branches:
+    - master
+    - dev
+  pull_request:
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  pull-requests: read
+jobs:
+  golangci:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.19
+      - uses: actions/checkout@v3
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
+          version: v1.49.0
+
+          # Optional: golangci-lint command line arguments.
+          # args: --issues-exit-code=0
+
+          # Optional: show only new issues if it's a pull request. The default value is `false`.
+          # only-new-issues: true
+
+          # Optional: if set to true then the all caching functionality will be complete disabled,
+          #           takes precedence over all other caching options.
+          # skip-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/go/pkg.
+          # skip-pkg-cache: true
+
+          # Optional: if set to true then the action don't cache or restore ~/.cache/go-build.
+          # skip-build-cache: true

.github/workflows/goreleaser.yml

@@ -15,7 +15,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: 1.18
+          go-version: 1.19
 
       - run: |
           # https://github.com/actions/setup-go/issues/107

.golangci.yml

@@ -0,0 +1,141 @@
+service:
+  # When updating this, also update the version stored in docker/build-tools/Dockerfile in the istio/tools repo.
+  golangci-lint-version: 1.49.x # use the fixed version to not introduce new linters unexpectedly
+  
+run:
+  concurrency: 4
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 20m
+  build-tags:
+  - integ
+  - integfuzz
+  # which dirs to skip: they won't be analyzed;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but next dirs are always skipped independently
+  # from this option's value:
+  #       vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs:
+    - genfiles$
+    - vendor$
+    - bin$
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  skip-files:
+    - ".*\\.pb\\.go"
+    - ".*\\.gen\\.go"
+
+linters:
+  disable-all: true
+  enable:
+  - unused
+  - errcheck
+  - exportloopref
+  - gocritic
+  - gofumpt
+  - goimports
+  - revive
+  - gosimple
+  - govet
+  - ineffassign
+  - lll
+  - misspell
+  - staticcheck
+  - stylecheck
+  - typecheck
+  - unconvert
+  - unparam
+  - gci
+  - bodyclose
+  - gosec
+  - asciicheck
+  - prealloc
+  - predeclared
+  - makezero
+  fast: false
+
+linters-settings:
+  errcheck:
+    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
+    # default is false: such cases aren't reported by default.
+    check-type-assertions: false
+
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+    # default is false: such cases aren't reported by default.
+    check-blank: false
+  govet:
+    # report about shadowed variables
+    check-shadowing: false
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Default is to use a neutral variety of English.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    locale: US
+    ignore-words:
+    - cancelled
+    - marshalled
+  lll:
+    # max line length, lines longer will be reported. Default is 120.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
+    line-length: 160
+    # tab width in spaces. Default to 1.
+    tab-width: 1
+  gocritic:
+    disabled-checks:
+    - exitAfterDefer
+  unused:
+    check-exported: false
+  unparam:
+    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
+    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  gci:
+    sections:
+    - standard
+    - default
+    - prefix(github.com/fatedier/frp/)
+  gosec:
+    severity: "low"
+    confidence: "low"
+    excludes:
+    - G102
+    - G112
+    - G306
+    - G401
+    - G402
+    - G404
+    - G501
+
+issues:
+  # List of regexps of issue texts to exclude, empty list by default.
+  # But independently from this option we use default exclude patterns,
+  # it can be disabled by `exclude-use-default: false`. To list all
+  # excluded by default patterns execute `golangci-lint run --help`
+  # exclude:
+  #  - composite literal uses unkeyed fields
+
+  exclude-rules:
+    # Exclude some linters from running on test files.
+    - path: _test\.go$|^tests/|^samples/
+      linters:
+        - errcheck
+        - maligned
+
+  # Independently from option `exclude` we use default exclude patterns,
+  # it can be disabled by this option. To list all
+  # excluded by default patterns execute `golangci-lint run --help`.
+  # Default value for this option is true.
+  exclude-use-default: true
+
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0

client/admin.go

@@ -20,10 +20,10 @@ import (
 	"net/http/pprof"
 	"time"
 
+	"github.com/gorilla/mux"
+
 	"github.com/fatedier/frp/assets"
 	frpNet "github.com/fatedier/frp/pkg/util/net"
-
-	"github.com/gorilla/mux"
 )
 
 var (
@@ -77,6 +77,8 @@ func (svr *Service) RunAdminServer(address string) (err error) {
 		return err
 	}
 
-	go server.Serve(ln)
+	go func() {
+		_ = server.Serve(ln)
+	}()
 	return
 }

client/admin_api.go

@@ -49,7 +49,7 @@ func (svr *Service) apiReload(w http.ResponseWriter, r *http.Request) {
 		log.Info("api response [/api/reload], code [%d]", res.Code)
 		w.WriteHeader(res.Code)
 		if len(res.Msg) > 0 {
-			w.Write([]byte(res.Msg))
+			_, _ = w.Write([]byte(res.Msg))
 		}
 	}()
 
@@ -68,7 +68,6 @@ func (svr *Service) apiReload(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	log.Info("success reload conf")
-	return
 }
 
 type StatusResp struct {
@@ -173,7 +172,7 @@ func (svr *Service) apiStatus(w http.ResponseWriter, r *http.Request) {
 	defer func() {
 		log.Info("Http response [/api/status]")
 		buf, _ = json.Marshal(&res)
-		w.Write(buf)
+		_, _ = w.Write(buf)
 	}()
 
 	ps := svr.ctl.pm.GetAllProxyStatus()
@@ -202,7 +201,6 @@ func (svr *Service) apiStatus(w http.ResponseWriter, r *http.Request) {
 	sort.Sort(ByProxyStatusResp(res.STCP))
 	sort.Sort(ByProxyStatusResp(res.XTCP))
 	sort.Sort(ByProxyStatusResp(res.SUDP))
-	return
 }
 
 // GET api/config
@@ -214,7 +212,7 @@ func (svr *Service) apiGetConfig(w http.ResponseWriter, r *http.Request) {
 		log.Info("Http get response [/api/config], code [%d]", res.Code)
 		w.WriteHeader(res.Code)
 		if len(res.Msg) > 0 {
-			w.Write([]byte(res.Msg))
+			_, _ = w.Write([]byte(res.Msg))
 		}
 	}()
 
@@ -254,7 +252,7 @@ func (svr *Service) apiPutConfig(w http.ResponseWriter, r *http.Request) {
 		log.Info("Http put response [/api/config], code [%d]", res.Code)
 		w.WriteHeader(res.Code)
 		if len(res.Msg) > 0 {
-			w.Write([]byte(res.Msg))
+			_, _ = w.Write([]byte(res.Msg))
 		}
 	}()
 
@@ -315,7 +313,7 @@ func (svr *Service) apiPutConfig(w http.ResponseWriter, r *http.Request) {
 	}
 	content = strings.Join(newRows, "\n")
 
-	err = os.WriteFile(svr.cfgFile, []byte(content), 0644)
+	err = os.WriteFile(svr.cfgFile, []byte(content), 0o644)
 	if err != nil {
 		res.Code = 500
 		res.Msg = fmt.Sprintf("write content to frpc config file error: %v", err)

client/control.go

@@ -21,21 +21,20 @@ import (
 	"net"
 	"runtime/debug"
 	"strconv"
-	"sync"
 	"time"
 
+	"github.com/fatedier/golib/control/shutdown"
+	"github.com/fatedier/golib/crypto"
+	libdial "github.com/fatedier/golib/net/dial"
+	fmux "github.com/hashicorp/yamux"
+
 	"github.com/fatedier/frp/client/proxy"
 	"github.com/fatedier/frp/pkg/auth"
 	"github.com/fatedier/frp/pkg/config"
 	"github.com/fatedier/frp/pkg/msg"
 	"github.com/fatedier/frp/pkg/transport"
 	frpNet "github.com/fatedier/frp/pkg/util/net"
 	"github.com/fatedier/frp/pkg/util/xlog"
-
-	"github.com/fatedier/golib/control/shutdown"
-	"github.com/fatedier/golib/crypto"
-	libdial "github.com/fatedier/golib/net/dial"
-	fmux "github.com/hashicorp/yamux"
 )
 
 type Control struct {
@@ -79,8 +78,6 @@ type Control struct {
 	// The UDP port that the server is listening on
 	serverUDPPort int
 
-	mu sync.RWMutex
-
 	xl *xlog.Logger
 
 	// service context
@@ -95,8 +92,8 @@ func NewControl(ctx context.Context, runID string, conn net.Conn, session *fmux.
 	pxyCfgs map[string]config.ProxyConf,
 	visitorCfgs map[string]config.VisitorConf,
 	serverUDPPort int,
-	authSetter auth.Setter) *Control {
-
+	authSetter auth.Setter,
+) *Control {
 	// new xlog instance
 	ctl := &Control{
 		runID:              runID,
@@ -131,7 +128,6 @@ func (ctl *Control) Run() {
 
 	// start all visitors
 	go ctl.vm.Run()
-	return
 }
 
 func (ctl *Control) HandleReqWorkConn(inMsg *msg.ReqWorkConn) {
@@ -400,24 +396,21 @@ func (ctl *Control) worker() {
 	go ctl.reader()
 	go ctl.writer()
 
-	select {
-	case <-ctl.closedCh:
-		// close related channels and wait until other goroutines done
-		close(ctl.readCh)
-		ctl.readerShutdown.WaitDone()
-		ctl.msgHandlerShutdown.WaitDone()
+	<-ctl.closedCh
+	// close related channels and wait until other goroutines done
+	close(ctl.readCh)
+	ctl.readerShutdown.WaitDone()
+	ctl.msgHandlerShutdown.WaitDone()
 
-		close(ctl.sendCh)
-		ctl.writerShutdown.WaitDone()
+	close(ctl.sendCh)
+	ctl.writerShutdown.WaitDone()
 
-		ctl.pm.Close()
-		ctl.vm.Close()
+	ctl.pm.Close()
+	ctl.vm.Close()
 
-		close(ctl.closedDoneCh)
-		if ctl.session != nil {
-			ctl.session.Close()
-		}
-		return
+	close(ctl.closedDoneCh)
+	if ctl.session != nil {
+		ctl.session.Close()
 	}
 }
 

client/event/event.go

@@ -6,9 +6,7 @@ import (
 	"github.com/fatedier/frp/pkg/msg"
 )
 
-var (
-	ErrPayloadType = errors.New("error payload type")
-)
+var ErrPayloadType = errors.New("error payload type")
 
 type Handler func(payload interface{}) error