From 7542542d1c9bca21eba16b86c12e2fc10ffa3c3f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Sat, 10 Feb 2024 16:51:53 -0800 Subject: [PATCH] ci: Use golangci-lint This PR switches to using golangci-lint to run linters. staticcheck is included and enabled by default. We add revive and a few others. All issues reported by the linters have been fixed. This also allows the linter to run in parallel with the tests. License header check has also been moved to golangci-lint, similarly to uber-go/fx#1157. Refs uber-go/fx#1150 Refs uber-go/zap#1323 --- .github/workflows/go.yml | 46 +++++++++++--------- .golangci.yml | 89 ++++++++++++++++++++++++++++++++++++++ Makefile | 49 +++++++-------------- callback.go | 1 - check_license.sh | 17 -------- decorate_test.go | 2 +- dig_test.go | 3 +- error.go | 2 - error_test.go | 1 - glide.yaml | 7 --- internal/dot/graph_test.go | 8 ++-- invoke.go | 3 +- param.go | 3 +- param_test.go | 2 - scope.go | 2 +- tools/doc.go | 24 ---------- tools/go.mod | 16 ------- tools/go.sum | 26 ----------- tools/tools.go | 30 ------------- visualize_golden_test.go | 2 +- 20 files changed, 143 insertions(+), 190 deletions(-) create mode 100644 .golangci.yml delete mode 100755 check_license.sh delete mode 100644 glide.yaml delete mode 100644 tools/doc.go delete mode 100644 tools/go.mod delete mode 100644 tools/go.sum delete mode 100644 tools/tools.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index dd8328ea..6778b23e 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -14,36 +14,22 @@ jobs: build: runs-on: ubuntu-latest + name: Test (Go ${{ matrix.go }}) strategy: matrix: go: ["1.20.x", "1.21.x"] - include: - - go: 1.21.x - latest: true steps: - name: Setup Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} - name: Checkout code - uses: actions/checkout@v2 - - - name: Load cached dependencies - uses: actions/cache@v1 - with: - path: ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- + uses: actions/checkout@v4 - name: Download Dependencies - run: make install - - - name: Lint - if: matrix.latest - run: make lint + run: go mod download - name: Test run: make cover @@ -51,5 +37,25 @@ jobs: - name: Upload coverage to codecov.io uses: codecov/codecov-action@v1 - - name: Benchmark - run: make bench + lint: + name: Lint + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: 1.21.x + cache: false # managed by golangci-lint + + - uses: golangci/golangci-lint-action@v3 + name: Install golangci-lint + with: + version: latest + args: --version # make lint will run the linter + + - run: make lint + name: Lint diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..0ea3ebb6 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,89 @@ +output: + # Make output more digestible with quickfix in vim/emacs/etc. + sort-results: true + print-issued-lines: false + +linters: + # We'll track the golangci-lint default linters manually + # instead of letting them change without our control. + disable-all: true + enable: + # golangci-lint defaults: + - gosimple + - govet + - ineffassign + - staticcheck + - unused + + # Our own extras: + - gofumpt + - nolintlint # lints nolint directives + - revive + - errorlint + + # License header check + - goheader + +linters-settings: + govet: + # These govet checks are disabled by default, but they're useful. + enable: + - niliness + - reflectvaluecompare + - sortslice + - unusedwrite + + goheader: + values: + const: + COMPANY: 'Uber Technologies, Inc.' + regexp: + YEAR_RANGE: '\d{4}(-\d{4})?' + template: |- + Copyright (c) {{ YEAR_RANGE }} {{ COMPANY }} + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. + +issues: + # Print all issues reported by all linters. + max-issues-per-linter: 0 + max-same-issues: 0 + + # Don't ignore some of the issues that golangci-lint considers okay. + # This includes documenting all exported entities. + exclude-use-default: false + + exclude-rules: + # Don't warn on unused parameters. + # Parameter names are useful; replacing them with '_' is undesirable. + - linters: [revive] + text: 'unused-parameter: parameter \S+ seems to be unused, consider removing or renaming it as _' + + # staticcheck already has smarter checks for empty blocks. + # revive's empty-block linter has false positives. + # For example, as of writing this, the following is not allowed. + # for foo() { } + - linters: [revive] + text: 'empty-block: this block is empty, you can remove it' + + # It's okay if internal packages and examples in docs/ + # don't have package comments. + - linters: [revive] + path: '.+/internal/.+|^internal/.+' + text: 'should have a package comment' diff --git a/Makefile b/Makefile index 5eb38a8c..02333dfe 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,8 @@ -export GOBIN ?= $(shell pwd)/bin +# Directory containing the Makefile. +PROJECT_ROOT = $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) -GOLINT = $(GOBIN)/golint -STATICCHECK = $(GOBIN)/staticcheck +export GOBIN ?= $(PROJECT_ROOT)/bin +export PATH := $(GOBIN):$(PATH) BENCH_FLAGS ?= -cpuprofile=cpu.pprof -memprofile=mem.pprof -benchmem @@ -9,8 +10,6 @@ GO_FILES = $(shell \ find . '(' -path '*/.*' -o -path './vendor' ')' -prune \ -o -name '*.go' -print | cut -b3-) -MODULES = . ./tools - .PHONY: all all: build lint test @@ -18,35 +17,8 @@ all: build lint test build: go build ./... -.PHONY: install -install: - $(foreach dir,$(MODULES),( \ - cd $(dir) && \ - go mod download) && \ - ) true - .PHONY: lint -lint: $(GOLINT) $(STATICCHECK) - @rm -rf lint.log - @echo "Checking formatting..." - @gofmt -d -s $(GO_FILES) 2>&1 | tee lint.log - @echo "Checking vet..." - @go vet ./... 2>&1 | tee -a lint.log - @echo "Checking lint..." - @$(GOLINT) ./... 2>&1 | tee -a lint.log - @echo "Checking staticcheck..." - @$(STATICCHECK) ./... 2>&1 | tee -a lint.log - @echo "Checking for unresolved FIXMEs..." - @git grep -i fixme | grep -v -e Makefile | tee -a lint.log - @echo "Checking for license headers..." - @./check_license.sh | tee -a lint.log - @[ ! -s lint.log ] - -$(GOLINT): tools/go.mod - cd tools && go install golang.org/x/lint/golint - -$(STATICCHECK): tools/go.mod - cd tools && go install honnef.co/go/tools/cmd/staticcheck +lint: golangci-lint tidy-lint .PHONY: test test: @@ -64,4 +36,13 @@ bench: .PHONY: tidy tidy: - $(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true + go mod tidy + +.PHONY: golangci-lint +golangci-lint: + golangci-lint run + +.PHONY: tidy-lint +tidy-lint: + go mod tidy + git diff --exit-code -- go.mod go.sum diff --git a/callback.go b/callback.go index c5caaec8..dfe47ea6 100644 --- a/callback.go +++ b/callback.go @@ -24,7 +24,6 @@ package dig // called by Dig, and is passed to a [Callback] registered with // [WithProviderCallback] or [WithDecoratorCallback]. type CallbackInfo struct { - // Name is the name of the function in the format: // . Name string diff --git a/check_license.sh b/check_license.sh deleted file mode 100755 index 345ac8b8..00000000 --- a/check_license.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash -e - -ERROR_COUNT=0 -while read -r file -do - case "$(head -1 "${file}")" in - *"Copyright (c) "*" Uber Technologies, Inc.") - # everything's cool - ;; - *) - echo "$file is missing license header." - (( ERROR_COUNT++ )) - ;; - esac -done < <(git ls-files "*\.go") - -exit $ERROR_COUNT diff --git a/decorate_test.go b/decorate_test.go index 721ba801..206e8fcb 100644 --- a/decorate_test.go +++ b/decorate_test.go @@ -50,7 +50,7 @@ func (i *someInt) String() string { } func (i *someInt) Increment() { - *i += 1 + *i++ } func TestDecorateSuccess(t *testing.T) { diff --git a/dig_test.go b/dig_test.go index 321393c1..5cbf4ee8 100644 --- a/dig_test.go +++ b/dig_test.go @@ -1676,7 +1676,7 @@ func TestRecoverFromPanic(t *testing.T) { dig.AssertErrorMatches(t, err, tt.wantErr[0], tt.wantErr[1:]...) var pe dig.PanicError assert.True(t, errors.As(err, &pe), "expected error chain to contain a PanicError") - _, ok := dig.RootCause(err).(dig.PanicError) + _, ok := dig.RootCause(err).(dig.PanicError) //nolint:errorlint // want dig.PanicError assert.True(t, ok, "expected root cause to be a PanicError") }) }) @@ -1686,7 +1686,6 @@ func TestRecoverFromPanic(t *testing.T) { func giveInt() int { return 5 } func TestCallback(t *testing.T) { - t.Run("no errors", func(t *testing.T) { var ( provideCallbackCalled bool diff --git a/error.go b/error.go index 8aba9ee5..227fc35f 100644 --- a/error.go +++ b/error.go @@ -84,7 +84,6 @@ type digError interface { // // This is an error // } type PanicError struct { - // The function the panic occurred at fn *digreflect.Func @@ -473,7 +472,6 @@ func newErrMissingTypes(c containerStore, k key) errMissingTypes { func (e errMissingTypes) Error() string { return fmt.Sprint(e) } func (e errMissingTypes) writeMessage(w io.Writer, v string) { - multiline := v == "%+v" if len(e) == 1 { diff --git a/error_test.go b/error_test.go index ac9a5364..1656edef 100644 --- a/error_test.go +++ b/error_test.go @@ -217,7 +217,6 @@ func (e MyNonDigError) Error() string { } func TestRootCauseEndToEnd(t *testing.T) { - tests := []struct { desc string setup func(c *Container) diff --git a/glide.yaml b/glide.yaml deleted file mode 100644 index 972b804c..00000000 --- a/glide.yaml +++ /dev/null @@ -1,7 +0,0 @@ -package: go.uber.org/dig -license: MIT -testImport: -- package: github.com/stretchr/testify - subpackages: - - assert - - require diff --git a/internal/dot/graph_test.go b/internal/dot/graph_test.go index 7af1fb22..ae1930b1 100644 --- a/internal/dot/graph_test.go +++ b/internal/dot/graph_test.go @@ -27,9 +27,11 @@ import ( "github.com/stretchr/testify/assert" ) -type t1 struct{} -type t2 struct{} -type t3 struct{} +type ( + t1 struct{} + t2 struct{} + t3 struct{} +) func TestNewGroup(t *testing.T) { type1 := reflect.TypeOf(t1{}) diff --git a/invoke.go b/invoke.go index 8a70121a..766415b3 100644 --- a/invoke.go +++ b/invoke.go @@ -22,9 +22,10 @@ package dig import ( "fmt" + "reflect" + "go.uber.org/dig/internal/digreflect" "go.uber.org/dig/internal/graph" - "reflect" ) // An InvokeOption modifies the default behavior of Invoke. diff --git a/param.go b/param.go index ca068975..66c4d1e2 100644 --- a/param.go +++ b/param.go @@ -21,6 +21,7 @@ package dig import ( + "errors" "fmt" "reflect" "strconv" @@ -291,7 +292,7 @@ func (ps paramSingle) Build(c containerStore) (reflect.Value, error) { // If we're missing dependencies but the parameter itself is optional, // we can just move on. - if _, ok := err.(errMissingDependencies); ok && ps.Optional { + if errors.As(err, new(errMissingDependencies)) && ps.Optional { return reflect.Zero(ps.Type), nil } diff --git a/param_test.go b/param_test.go index 7a1f41ed..507529aa 100644 --- a/param_test.go +++ b/param_test.go @@ -68,7 +68,6 @@ func TestParamObjectSuccess(t *testing.T) { require.True(t, ok, "T1 must be a paramSingle") assert.Empty(t, t1.Name) assert.False(t, t1.Optional) - }) t.Run("optional field", func(t *testing.T) { @@ -78,7 +77,6 @@ func TestParamObjectSuccess(t *testing.T) { require.True(t, ok, "T2 must be a paramSingle") assert.Empty(t, t2.Name) assert.True(t, t2.Optional) - }) t.Run("named value", func(t *testing.T) { diff --git a/scope.go b/scope.go index dc0c7ac5..d5478aca 100644 --- a/scope.go +++ b/scope.go @@ -32,7 +32,7 @@ import ( // A ScopeOption modifies the default behavior of Scope; currently, // there are no implementations. type ScopeOption interface { - noScopeOption() //yet + noScopeOption() // yet } // Scope is a scoped DAG of types and their dependencies. diff --git a/tools/doc.go b/tools/doc.go deleted file mode 100644 index 01a0454f..00000000 --- a/tools/doc.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2021 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -// Package tools exists to make this directory a valid Go package. -// The tools.go has a build tag that excludes it from being considered by Go -// tooling except for dependency constraints. -package tools diff --git a/tools/go.mod b/tools/go.mod deleted file mode 100644 index 4b0bced5..00000000 --- a/tools/go.mod +++ /dev/null @@ -1,16 +0,0 @@ -module github.com/uber-go/dig/tools - -go 1.20 - -require ( - golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 - honnef.co/go/tools v0.4.5 -) - -require ( - github.com/BurntSushi/toml v1.2.1 // indirect - golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a // indirect - golang.org/x/mod v0.10.0 // indirect - golang.org/x/sys v0.8.0 // indirect - golang.org/x/tools v0.9.4-0.20230601214343-86c93e8732cc // indirect -) diff --git a/tools/go.sum b/tools/go.sum deleted file mode 100644 index 7d16a2ef..00000000 --- a/tools/go.sum +++ /dev/null @@ -1,26 +0,0 @@ -github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= -github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= -golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a h1:Jw5wfR+h9mnIYH+OtGT2im5wV1YGGDora5vTv/aa5bE= -golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= -golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= -golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= -golang.org/x/mod v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk= -golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.2.0 h1:PUR+T4wwASmuSTYdKjYHI5TD22Wy5ogLU5qZCOLxBrI= -golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= -golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.9.4-0.20230601214343-86c93e8732cc h1:mqZawFxUzsv+YVwGQO30cZegeV/YD6dAwsdGxi0tQQg= -golang.org/x/tools v0.9.4-0.20230601214343-86c93e8732cc/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -honnef.co/go/tools v0.4.5 h1:YGD4H+SuIOOqsyoLOpZDWcieM28W47/zRO7f+9V3nvo= -honnef.co/go/tools v0.4.5/go.mod h1:GUV+uIBCLpdf0/v6UhHHG/yzI/z6qPskBeQCjcNB96k= diff --git a/tools/tools.go b/tools/tools.go deleted file mode 100644 index 3c855c17..00000000 --- a/tools/tools.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) 2019 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -//go:build tools -// +build tools - -package tools - -import ( - // Tools we use during development. - _ "golang.org/x/lint/golint" - _ "honnef.co/go/tools/cmd/staticcheck" -) diff --git a/visualize_golden_test.go b/visualize_golden_test.go index c87adc42..99dcac35 100644 --- a/visualize_golden_test.go +++ b/visualize_golden_test.go @@ -40,7 +40,7 @@ func VerifyVisualization(t *testing.T, testname string, c *Container, opts ...Vi dotFile := filepath.Join("testdata", testname+".dot") if *generate { - err := os.WriteFile(dotFile, b.Bytes(), 0644) + err := os.WriteFile(dotFile, b.Bytes(), 0o644) require.NoError(t, err) return }