Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: Use golangci-lint #1150

Merged
merged 8 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 41 additions & 13 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ jobs:

build:
runs-on: ${{ matrix.os }}
name: Build and test

strategy:
matrix:
os: ["ubuntu-latest", "windows-latest"]
go: ["1.20.x", "1.21.x"]
include:
- go: 1.21.x
os: "ubuntu-latest"
latest: true

steps:
- name: Checkout code
Expand All @@ -31,21 +29,51 @@ jobs:
uses: actions/setup-go@v5
with:
go-version: ${{ matrix.go }}
cache: true

- name: Download Dependencies
run: go mod download

- name: Lint
if: matrix.latest
run: make lint

- name: Test
run: make cover

- name: Test documentation
run: make cover COVER_MODULES=./docs
if: matrix.latest

- name: Upload coverage to codecov.io
uses: codecov/codecov-action@v3

doctest:
runs-on: ubuntu-latest
name: Test documentation

steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: 1.21.x

- name: Test
run: make cover COVER_MODULES=./docs

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
70 changes: 70 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
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

linters-settings:
govet:
# These govet checks are disabled by default, but they're useful.
enable:
- niliness
- reflectvaluecompare
- sortslice
- unusedwrite

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/.+|^docs/.+'
text: 'should have a package comment'

# It's okay for tests to use dot imports.
- linters: [revive]
path: '_test\.go$'
text: 'should not use dot imports'

# Ignore logger.Sync() errcheck failures in example_test.go
# since those are intended to be uncomplicated examples.
- linters: [errcheck]
path: example_test.go
text: 'Error return value of `logger.Sync` is not checked'
95 changes: 44 additions & 51 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
export GOBIN ?= $(shell pwd)/bin
# Directory containing the Makefile.
PROJECT_ROOT = $(dir $(abspath $(lastword $(MAKEFILE_LIST))))

export GOBIN ?= $(PROJECT_ROOT)/bin
export PATH := $(GOBIN):$(PATH)

GOLINT = $(GOBIN)/golint
STATICCHECK = $(GOBIN)/staticcheck
FXLINT = $(GOBIN)/fxlint
MDOX = $(GOBIN)/mdox

GO_FILES = $(shell \
find . '(' -path '*/.*' -o -path './vendor' -o -path '*/testdata/*' ')' -prune \
-o -name '*.go' -print | cut -b3-)

MODULES = . ./tools ./docs ./internal/e2e

# 'make cover' should not run on docs by default.
# We run that separately explicitly on a specific platform.
COVER_MODULES ?= $(filter-out ./docs,$(MODULES))

.PHONY: all
all: build lint test

.PHONY: build
build:
go build ./...

.PHONY: install
install:
go mod download
.PHONY: lint
lint: golangci-lint tidy-lint license-lint fx-lint docs-lint

.PHONY: test
test:
Expand All @@ -35,52 +35,45 @@ cover:
go test -race -coverprofile=cover.out -coverpkg=./... ./... && \
go tool cover -html=cover.out -o cover.html) &&) true

$(GOLINT): tools/go.mod
cd tools && go install golang.org/x/lint/golint
.PHONY: tidy
tidy:
@$(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true

.PHONY: docs
docs:
cd docs && yarn build

$(STATICCHECK): tools/go.mod
cd tools && go install honnef.co/go/tools/cmd/staticcheck
.PHONY: golangci-lint
golangci-lint:
@$(foreach mod,$(MODULES), \
(cd $(mod) && \
echo "[lint] golangci-lint: $(mod)" && \
golangci-lint run --path-prefix $(mod)) &&) true

.PHONY: tidy-lint
tidy-lint:
@$(foreach mod,$(MODULES), \
(cd $(mod) && \
echo "[lint] tidy: $(mod)" && \
go mod tidy && \
git diff --exit-code -- go.mod go.sum) &&) true

.PHONY: license-lint
license-lint:
./checklicense.sh

.PHONY: fx-lint
fx-lint: $(FXLINT)
@$(FXLINT) ./...

.PHONY: docs-lint
docs-lint: $(MDOX)
@echo "Checking documentation"
@make -C docs check

$(MDOX): tools/go.mod
cd tools && go install github.com/bwplotka/mdox

$(FXLINT): tools/cmd/fxlint/main.go
cd tools && go install go.uber.org/fx/tools/cmd/fxlint

.PHONY: lint
lint: $(GOLINT) $(STATICCHECK) $(FXLINT) docs-check
@rm -rf lint.log
@echo "Checking formatting..."
@gofmt -d -s $(GO_FILES) 2>&1 | tee lint.log
@echo "Checking vet..."
@$(foreach dir,$(MODULES),(cd $(dir) && go vet ./... 2>&1) &&) true | tee -a lint.log
@echo "Checking lint..."
@$(foreach dir,$(MODULES),(cd $(dir) && $(GOLINT) ./... 2>&1) &&) true | tee -a lint.log
@echo "Checking staticcheck..."
@$(foreach dir,$(MODULES),(cd $(dir) && $(STATICCHECK) ./... 2>&1) &&) true | tee -a lint.log
@echo "Checking fxlint..."
@$(FXLINT) ./... | tee -a lint.log
@echo "Checking for unresolved FIXMEs..."
@git grep -i fixme | grep -v -e vendor -e Makefile -e .md | tee -a lint.log
@echo "Checking for license headers..."
@./checklicense.sh | tee -a lint.log
@[ ! -s lint.log ]
@echo "Checking 'go mod tidy'..."
@make tidy
@if ! git diff --quiet; then \
echo "'go mod tidy' resulted in changes or working tree is dirty:"; \
git --no-pager diff; \
fi

.PHONY: docs
docs:
cd docs && yarn build

.PHONY: docs-check
docs-check: $(MDOX)
@echo "Checking documentation"
@make -C docs check | tee -a lint.log

.PHONY: tidy
tidy:
@$(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true
10 changes: 4 additions & 6 deletions annotated.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ type Annotation interface {
}

var (
_typeOfError reflect.Type = reflect.TypeOf((*error)(nil)).Elem()
_nilError = reflect.Zero(_typeOfError)
_typeOfError = reflect.TypeOf((*error)(nil)).Elem()
_nilError = reflect.Zero(_typeOfError)
)

// annotationError is a wrapper for an error that was encountered while
Expand Down Expand Up @@ -178,7 +178,6 @@ func verifyValueQuote(value string) (string, error) {
return "", errTagValueSyntaxEndingQuote
}
return value[i+1:], nil

}

// Check whether the tag follows valid struct.
Expand Down Expand Up @@ -211,7 +210,6 @@ func verifyAnnotateTag(tag string) error {
tag = value
}
return nil

}

// Given func(T1, T2, T3, ..., TN), this generates a type roughly
Expand Down Expand Up @@ -644,8 +642,8 @@ func (la *lifecycleHookAnnotation) build(ann *annotated) (interface{}, error) {
}

var (
_typeOfLifecycle reflect.Type = reflect.TypeOf((*Lifecycle)(nil)).Elem()
_typeOfContext reflect.Type = reflect.TypeOf((*context.Context)(nil)).Elem()
_typeOfLifecycle = reflect.TypeOf((*Lifecycle)(nil)).Elem()
_typeOfContext = reflect.TypeOf((*context.Context)(nil)).Elem()
)

// buildHookInstaller returns a function that appends a hook to Lifecycle when called,
Expand Down
2 changes: 1 addition & 1 deletion annotated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1701,8 +1701,8 @@ func TestAnnotateApplySuccess(t *testing.T) {
require.NoError(t, app.Err())
})
}

}

func assertApp(
t *testing.T,
app interface {
Expand Down
7 changes: 5 additions & 2 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,11 @@ func (err errorWithGraph) Error() string {

// VisualizeError returns the visualization of the error if available.
func VisualizeError(err error) (string, error) {
if e, ok := err.(errWithGraph); ok && e.Graph() != "" {
return string(e.Graph()), nil
var erg errWithGraph
if errors.As(err, &erg) {
if g := erg.Graph(); g != "" {
return string(g), nil
}
}
return "", errors.New("unable to visualize error")
}
Expand Down
6 changes: 4 additions & 2 deletions app_unixes.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ package fx

import "golang.org/x/sys/unix"

const _sigINT = unix.SIGINT
const _sigTERM = unix.SIGTERM
const (
_sigINT = unix.SIGINT
_sigTERM = unix.SIGTERM
)
6 changes: 4 additions & 2 deletions app_wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ package fx

import "syscall"

const _sigINT = syscall.SIGINT
const _sigTERM = syscall.SIGTERM
const (
_sigINT = syscall.SIGINT
_sigTERM = syscall.SIGTERM
)
6 changes: 4 additions & 2 deletions app_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,7 @@ package fx

import "golang.org/x/sys/windows"

const _sigINT = windows.SIGINT
const _sigTERM = windows.SIGTERM
const (
_sigINT = windows.SIGINT
_sigTERM = windows.SIGTERM
)
2 changes: 1 addition & 1 deletion decorate.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func runDecorator(c container, d decorator, opts ...dig.DecorateOption) (err err
decorator := d.Target
defer func() {
if err != nil {
err = fmt.Errorf("fx.Decorate(%v) from:\n%+vFailed: %v", decorator, d.Stack, err)
err = fmt.Errorf("fx.Decorate(%v) from:\n%+vFailed: %w", decorator, d.Stack, err)
}
}()

Expand Down
3 changes: 1 addition & 2 deletions decorate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,7 @@ func TestDecorateSuccess(t *testing.T) {
})

t.Run("decoration must execute when required by a member of group", func(t *testing.T) {
type Drinks interface {
}
type Drinks interface{}
type Coffee struct {
Type string
Name string
Expand Down
Loading
Loading