From 67d6d4c67bd5957879e1a8d5b04c06edce89ce08 Mon Sep 17 00:00:00 2001 From: Oleg Zaytsev Date: Fri, 10 Jan 2020 12:24:09 +0100 Subject: [PATCH] Use golangci-lint as linter (#29) * Use golangci-lint as linter This is faster than `goimports` and includes more linters. Also removed non user-runnable targets from Makefile. Finally, applied changes to the code to comply with the code. --- .golangci.yaml | 109 ++++++++++++++++++++++++++++++++++ .travis.yml | 9 +-- Makefile | 19 ++---- default.go | 4 +- init.go | 11 ++-- metrics_test.go | 6 +- prometheusvanilla/builders.go | 4 +- 7 files changed, 130 insertions(+), 32 deletions(-) create mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..f3e2dbd --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,109 @@ +run: + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 1m + + # include test files or not, default is true + tests: true + + # list of build tags, all linters use it. Default is empty list. + build-tags: [] + +# settings of specific linters +linters-settings: + govet: + check-shadowing: false + golint: + min-confidence: 0 + maligned: + suggest-new: true + dupl: + threshold: 100 + goconst: + min-len: 2 + min-occurrences: 2 + misspell: + locale: US + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + - octalLiteral + - wrapperFunc + +linters: + disable-all: true + enable: + - bodyclose + - deadcode + - depguard + - dogsled + - dupl + - errcheck + - goconst + - gocritic + - gofmt + - goimports + - golint + - gosec + - gosimple + - govet + - ineffassign + - interfacer + - misspell + - nakedret + - scopelint + - staticcheck + - structcheck + - stylecheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - whitespace + + # don't enable: + # - funlen + # - gochecknoglobals + # - gocognit + # - gocyclo + # - godox + # - lll + # - maligned + # - prealloc + # - gochecknoinits + + +issues: + # Excluding configuration per-path, per-linter, per-text and per-source + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: + - gocyclo + - errcheck + - dupl + - gosec + - scopelint + - goconst + - funlen + - gocritic + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter: 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues: 0 + + # Show only new issues: if there are unstaged changes or untracked files, + # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # It's a super-useful option for integration of golangci-lint into existing + # large codebase. It's not practical to fix all existing issues at the moment + # of integration: much better don't allow issues in new code. + # Default is false. + new: false diff --git a/.travis.yml b/.travis.yml index d60fea5..40384dc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,18 +1,19 @@ language: go go: - - "1.11.x" - "1.12.x" + - "1.13.x" env: - GO111MODULE=on install: - - make install + - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.20.1 script: - make test - - make check-fmt + - make lint after_success: - - make report-coveralls + - go get github.com/mattn/goveralls + - goveralls -coverprofile=coverage.out -service=travis-ci diff --git a/Makefile b/Makefile index 15a834f..4ac00c0 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: test benchmark help fmt install +.PHONY: test help fmt report-coveralls benchmark lint help: ## Show the help text @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[36m%-20s\033[93m %s\n", $$1, $$2}' @@ -6,20 +6,11 @@ help: ## Show the help text test: ## Run unit tests @go test -coverprofile=coverage.out -covermode=atomic -race ./... -benchmark: ## Run benchmarks - @go test -bench=. ./... - -check-fmt: ## Check file forma - @GOIMP=$$(for f in $$(find . -type f -name "*.go" ! -path "./.cache/*" ! -path "./vendor/*" ! -name "bindata.go") ; do \ - goimports -l $$f ; \ - done) && echo $$GOIMP && test -z "$$GOIMP" +lint: # Run linters using golangci-lint + @golangci-lint run fmt: ## Format files @goimports -w $$(find . -name "*.go" -not -path "./vendor/*") -install: ## Installs dependencies - GOPATH=$$GOPATH && go get -u -v \ - golang.org/x/tools/cmd/goimports - -report-coveralls: ## Reports generated coverage profile to coveralls.io. Intended to be used only from travis - go get github.com/mattn/goveralls && goveralls -coverprofile=coverage.out -service=travis-ci +benchmark: ## Run benchmarks + @go test -run=NONE -benchmem -benchtime=5s -bench=. ./... diff --git a/default.go b/default.go index c89470e..ce34c2b 100644 --- a/default.go +++ b/default.go @@ -27,12 +27,12 @@ func AddBuilder(typ reflect.Type, registerer Builder) error { return DefaultInitializer.AddBuilder(typ, registerer) } -// MustInit initialises the metrics or panics. +// MustInit initializes the metrics or panics. func MustInit(metrics interface{}, namespace string) { DefaultInitializer.MustInit(metrics, namespace) } -// Init initialises the metrics in the given namespace. +// Init initializes the metrics in the given namespace. func Init(metrics interface{}, namespace string) error { return DefaultInitializer.Init(metrics, namespace) } diff --git a/init.go b/init.go index 60142b5..40900d6 100644 --- a/init.go +++ b/init.go @@ -28,10 +28,10 @@ type Initializer interface { // func() interface{} implements AddBuilder(typ reflect.Type, registerer Builder) error - // MustInit initialises the metrics or panics. + // MustInit initializes the metrics or panics. MustInit(metrics interface{}, namespace string) - // Init initialises the metrics in the given namespace. + // Init initializes the metrics in the given namespace. Init(metrics interface{}, namespace string) error } @@ -68,14 +68,14 @@ func (in initializer) AddBuilder(typ reflect.Type, builder Builder) error { return nil } -// MustInit initialises the metrics or panics. +// MustInit initializes the metrics or panics. func (in initializer) MustInit(metrics interface{}, namespace string) { if err := in.Init(metrics, namespace); err != nil { panic(err) } } -// Init initialises the metrics in the given namespace. +// Init initializes the metrics in the given namespace. func (in initializer) Init(metrics interface{}, namespace string) error { metricsPtr := reflect.ValueOf(metrics) if metricsPtr.Kind() != reflect.Ptr { @@ -91,7 +91,6 @@ func (in initializer) initMetrics(group reflect.Value, namespaces ...string) err } for i := 0; i < group.Type().NumField(); i++ { - field := group.Field(i) fieldType := group.Type().Field(i) @@ -177,7 +176,6 @@ func (in initializer) initMetricFunc(field reflect.Value, structField reflect.St metricFunc := func(args []reflect.Value) []reflect.Value { labels := make(prometheus.Labels, len(labelIndexes)) for label, index := range labelIndexes { - value := args[0].FieldByIndex(index) if label.hasDefaultValue && value.Interface() == label.zeroTypeValueInterface { @@ -227,7 +225,6 @@ func findLabelIndexes(typ reflect.Type, indexes map[label][]int, current ...int) return err } } else { - labelTag, ok := f.Tag.Lookup("label") if !ok { return fmt.Errorf("field %s does not have the label tag", f.Name) diff --git a/metrics_test.go b/metrics_test.go index 779c3fd..3f8d1c1 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -82,9 +82,9 @@ func Test_NestedMetrics(t *testing.T) { } assert.Equal(t, map[string]struct{}{ - "testservice_server_hits_total": struct{}{}, - "testservice_client_requests_total": struct{}{}, - "testservice_memory_consumption_bytes": struct{}{}, + "testservice_server_hits_total": {}, + "testservice_client_requests_total": {}, + "testservice_memory_consumption_bytes": {}, }, reportedMetrics) } diff --git a/prometheusvanilla/builders.go b/prometheusvanilla/builders.go index 390e639..1c766d0 100644 --- a/prometheusvanilla/builders.go +++ b/prometheusvanilla/builders.go @@ -119,7 +119,7 @@ func bucketsFromTag(tag reflect.StructTag) ([]float64, error) { return nil, fmt.Errorf("buckets not specified") } - if len(bucketsString) == 0 { + if bucketsString == "" { return nil, nil } @@ -160,7 +160,7 @@ func objectivesFromTag(tag reflect.StructTag) (map[float64]float64, error) { return nil, fmt.Errorf("objectives not specified") } - if len(quantileString) == 0 { + if quantileString == "" { return nil, nil }