Enabled golangci-lint (dapr#650)

* Enable golangci-lint

* Corrected typo

* Update azure pipeline to run `make lint` in CI

* Hope that wget works for windows

* Another try with windows

* Last try with .exe, pray it work

* Another try

* Another try :)

* Last for the day :)

* One more try :)

* Update .gitignore

* Give up on windows :(

* Disabled fail fast to get all error at once

* :)

* Clean up

* Update misspell locale

.gitignore

@@ -1 +1,2 @@
-/dist
+/dist
+.idea

.golangci.yml

@@ -0,0 +1,244 @@
+# options for analysis running
+run:
+  # default concurrency is a available CPU number
+  concurrency: 4
+
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  deadline: 5m
+
+  # exit code when at least one issue was found, default is 1
+  issues-exit-code: 1
+
+  # include test files or not, default is true
+  tests: false
+
+  # list of build tags, all linters use it. Default is empty list.
+  #build-tags:
+  #  - mytag
+
+  # 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:
+    - ^vendor$
+    - ^pkg.*client.*clientset.*versioned.*
+    - ^pkg.*client.*informers.*externalversions.*
+
+  # 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:
+  #  - ".*\\.my\\.go$"
+  #  - lib/bad.go
+    - pkg.*version.*version.go
+    - pkg.*models.*models.go
+    - pkg.*config.*modes.*kubernetes_config.go
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
+  format: tab
+
+  # print lines of code with issue, default is true
+  print-issued-lines: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+
+# all available settings of specific linters
+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
+
+    # [deprecated] comma-separated list of pairs of the form pkg:regex
+    # the regex is used to ignore names within pkg. (default "fmt:.*").
+    # see https://github.com/kisielk/errcheck#the-deprecated-method for details
+    ignore: fmt:.*,io/ioutil:^Read.*
+
+    # path to a file containing a list of functions to exclude from checking
+    # see https://github.com/kisielk/errcheck#excluding-functions for details
+    exclude:
+
+  funlen:
+    lines: 60
+    statements: 40
+
+  govet:
+    # report about shadowed variables
+    check-shadowing: true
+
+    # settings per analyzer
+    settings:
+      printf: # analyzer name, run `go tool vet help` to see all analyzers
+        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+
+    # enable or disable analyzers by name
+    enable:
+      - atomicalign
+    enable-all: false
+    disable:
+      - shadow
+    disable-all: false
+  golint:
+    # minimal confidence for issues, default is 0.8
+    min-confidence: 0.8
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: true
+  goimports:
+    # put imports beginning with prefix after 3rd-party packages;
+    # it's a comma-separated list of prefixes
+    local-prefixes: github.com/dapr/dapr
+  gocyclo:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 10
+  gocognit:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 10
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  dupl:
+    # tokens count to trigger issue, 150 by default
+    threshold: 100
+  goconst:
+    # minimal length of string constant, 3 by default
+    min-len: 3
+    # minimal occurrences count to trigger, 3 by default
+    min-occurrences: 3
+  depguard:
+    list-type: blacklist
+    include-go-root: false
+    packages:
+      - github.com/sirupsen/logrus
+    packages-with-error-messages:
+      # specify an error message to output when a blacklisted package is used
+      github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
+  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: default
+    ignore-words:
+      - someword
+  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: 120
+    # tab width in spaces. Default to 1.
+    tab-width: 1
+  unused:
+    # treat code as a program (not a library) and report unused exported identifiers; default is false.
+    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    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
+  nakedret:
+    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
+    max-func-lines: 30
+  prealloc:
+    # XXX: we don't recommend using this linter before doing performance profiling.
+    # For most programs usage of prealloc will be a premature optimization.
+
+    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
+    # True by default.
+    simple: true
+    range-loops: true # Report preallocation suggestions on range loops, true by default
+    for-loops: false # Report preallocation suggestions on for loops, false by default
+  gocritic:
+    # Which checks should be enabled; can't be combined with 'disabled-checks';
+    # See https://go-critic.github.io/overview#checks-overview
+    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
+    # By default list of stable checks is used.
+    enabled-checks:
+
+    # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
+    disabled-checks:
+      - regexpMust
+      - rangeValCopy
+      - hugeParam
+      - ifElseChain
+      - singleCaseSwitch
+
+    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
+    # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
+    enabled-tags:
+      - performance
+
+    settings: # settings passed to gocritic
+      captLocal: # must be valid enabled check name
+        paramsOnly: true
+  godox:
+    # report any comments starting with keywords, this is useful for TODO or FIXME comments that
+    # might be left in the code accidentally and should be resolved before merging
+    keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting
+      - NOTE
+      - OPTIMIZE # marks code that should be optimized before merging
+      - HACK # marks hack-arounds that should be removed before merging
+  dogsled:
+    # checks assignments with too many blank identifiers; default is 2
+    max-blank-identifiers: 2
+
+  whitespace:
+    multi-if: false   # Enforces newlines (or comments) after every multi-line if statement
+    multi-func: false # Enforces newlines (or comments) after every multi-line function signature
+
+  wsl:
+    # If true append is only allowed to be cuddled if appending value is
+    # matching variables, fields or types on line above. Default is true.
+    strict-append: true
+    # Allow calls and assignments to be cuddled as long as the lines have any
+    # matching variables, fields or types. Default is true.
+    allow-assign-and-call: true
+    # Allow multiline assignments to be cuddled. Default is true.
+    allow-multiline-assign: true
+    # Allow case blocks to end with a whitespace.
+    allow-case-traling-whitespace: true
+    # Allow declarations (var) to be cuddled.
+    allow-cuddle-declarations: false
+
+linters:
+  fast: false
+  enable-all: true
+  disable:
+    # TODO Enforce the below linters later
+    - deadcode
+    - dupl
+    - errcheck
+    - funlen
+    - gochecknoglobals
+    - gochecknoinits
+    - gocyclo
+    - gocognit
+    - godox
+    - golint
+    # TODO enable gofmt and goimports again once github action is done. Now both failed in windows only.
+    - gofmt
+    - goimports
+    - interfacer
+    - lll
+    - maligned
+    - scopelint
+    - unparam
+    - unused
+    - wsl

Makefile

@@ -56,9 +56,11 @@ export GOOS ?= $(TARGET_OS_LOCAL)
 
 ifeq ($(GOOS),windows)
 BINARY_EXT_LOCAL:=.exe
+GOLANGCI_LINT:=golangci-lint.exe
 export ARCHIVE_EXT = .zip
 else
 BINARY_EXT_LOCAL:=
+GOLANGCI_LINT:=golangci-lint
 export ARCHIVE_EXT = .tar.gz
 endif
 
@@ -239,3 +241,10 @@ release: build archive
 .PHONY: test
 test:
 	go test ./pkg/... -mod=vendor
+
+################################################################################
+# Target: lint                                                                 #
+################################################################################
+.PHONY: lint
+lint:
+	$(GOLANGCI_LINT) run

build-binary-template.yml

@@ -31,6 +31,14 @@ jobs:
           shopt -s extglob
           mv !(gopath) '$(modulePath)'
           echo '##vso[task.prependpath]$(GOBIN)'
+          curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b '$(GOPATH)/bin' v1.21.0
+    - task: CmdLine@2
+      displayName: 'Run make lint'
+      condition: ne('${{ parameters.targetArch }}', 'arm')
+      inputs:
+        script: 'make lint'
+        workingDirectory: '$(modulePath)'
+        failOnStderr: false
     - task: CmdLine@2
       displayName: 'Run make test'
       condition: ne('${{ parameters.targetArch }}', 'arm')

pkg/channel/http/http_channel.go

@@ -117,6 +117,7 @@ func (h *Channel) InvokeMethod(invokeRequest *channel.InvokeRequest) (*channel.I
 }
 
 // CreateLocalChannel creates an HTTP AppChannel
+// nolint:gosec
 func CreateLocalChannel(port, maxConcurrency int) (channel.AppChannel, error) {
 	c := &Channel{
 		client:      &fasthttp.Client{MaxConnsPerHost: 1000000, TLSConfig: &tls.Config{InsecureSkipVerify: true}, ReadTimeout: time.Second * 60, MaxIdemponentCallAttempts: 0},

pkg/components/bindings/loader.go

@@ -93,7 +93,5 @@ func Load() {
 	RegisterOutputBinding("gcp.pubsub", func() bindings.OutputBinding {
 		return pubsub.NewGCPPubSub()
 	})
-	RegisterInputBinding("kubernetes", func() bindings.InputBinding {
-		return kubernetes.NewKubernetes()
-	})
+	RegisterInputBinding("kubernetes", kubernetes.NewKubernetes)
 }

pkg/components/pubsub/loader.go

@@ -6,13 +6,10 @@
 package pubsub
 
 import (
-	"github.com/dapr/components-contrib/pubsub"
 	"github.com/dapr/components-contrib/pubsub/redis"
 )
 
 // Load message buses
 func Load() {
-	RegisterMessageBus("redis", func() pubsub.PubSub {
-		return redis.NewRedisStreams()
-	})
+	RegisterMessageBus("redis", redis.NewRedisStreams)
 }

pkg/components/secretstores/loader.go

@@ -6,17 +6,12 @@
 package secretstores
 
 import (
-	"github.com/dapr/components-contrib/secretstores"
 	"github.com/dapr/components-contrib/secretstores/keyvault"
 	"github.com/dapr/components-contrib/secretstores/kubernetes"
 )
 
 // Load secret stores
 func Load() {
-	RegisterSecretStore("kubernetes", func() secretstores.SecretStore {
-		return kubernetes.NewKubernetesSecretStore()
-	})
-	RegisterSecretStore("azure.keyvault", func() secretstores.SecretStore {
-		return keyvault.NewAzureKeyvaultSecretStore()
-	})
+	RegisterSecretStore("kubernetes", kubernetes.NewKubernetesSecretStore)
+	RegisterSecretStore("azure.keyvault", keyvault.NewAzureKeyvaultSecretStore)
 }

pkg/diagnostics/tracing.go

@@ -49,8 +49,8 @@ func DeserializeSpanContext(ctx string) trace.SpanContext {
 	traceID, _ := hex.DecodeString(parts[1])
 	traceOptions, _ := strconv.ParseUint(parts[2], 10, 32)
 	ret := trace.SpanContext{}
-	copy(ret.SpanID[:], spanID[:])
-	copy(ret.TraceID[:], traceID[:])
+	copy(ret.SpanID[:], spanID)
+	copy(ret.TraceID[:], traceID)
 	ret.TraceOptions = trace.TraceOptions(traceOptions)
 	return ret
 }
@@ -199,13 +199,13 @@ func addAnnotationsFromMD(md metautils.NiceMD, span *trace.Span, expandParams bo
 	if expandParams {
 		for k, vv := range md {
 			for _, v := range vv {
-				span.AddAttributes(trace.StringAttribute(string(k), v))
+				span.AddAttributes(trace.StringAttribute(k, v))
 			}
 		}
 	}
-	if includeBody {
-		//TODO: get request body?
-	}
+	//TODO: get request body?
+	//if includeBody {
+	//}
 }
 
 func projectStatusCode(code int) int32 {

pkg/discovery/mdns.go

@@ -63,7 +63,7 @@ func LookupPortMDNS(id string) (int, error) {
 
 	err = resolver.Browse(ctx, id, "local.", entries)
 	if err != nil {
-		return -1, fmt.Errorf("Failed to browse: %s", err.Error())
+		return -1, fmt.Errorf("failed to browse: %s", err.Error())
 	}
 
 	<-ctx.Done()