Skip to content

[ci] Switch to golangci-lint #279

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

Merged
merged 3 commits into from
Mar 1, 2021
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
2 changes: 1 addition & 1 deletion .buildkite/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ export PACKAGE=github.com/m3db/m3db-operator
echo "--- :git: Updating git submodules"
git submodule update --init --recursive
echo "--- Running unit tests"
make clean-all test-ci-unit lint metalint bins test-all-gen build-integration
make clean-all test-ci-unit lint bins test-all-gen build-integration
6 changes: 0 additions & 6 deletions .excludemetalint

This file was deleted.

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

# 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: true

# list of build tags, all linters use it. Default is empty list.
build-tags: []

# 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:
- generated/.*
- vendor/.*
- pkg/client/.*
- pkg/assets/.*

# 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: []

# by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
# If invoked with -mod=readonly, the go command is disallowed from the implicit
# automatic updating of go.mod described above. Instead, it fails when any changes
# to go.mod are needed. This setting is most useful to check that go.mod does
# not need updates, such as in a continuous integration and testing system.
# If invoked with -mod=vendor, the go command assumes that the vendor
# directory holds the correct copies of dependencies and ignores
# the dependency descriptions in go.mod.
# modules-download-mode: readonly|release|vendor
modules-download-mode: readonly


# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
format: colored-line-number

# 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:
govet:
# report about shadowed variables
check-shadowing: true
goimports:
# put imports beginning with prefix after 3rd-party packages;
# it's a comma-separated list of prefixes
local-prefixes: github.com/m3db/m3db-operator
gocyclo:
# 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
- github.com/golang/protobuf/jsonpb
- google.golang.org/protobuf/encoding/protojson
- github.com/golang/protobuf/proto
- google.golang.org/protobuf/proto
- github.com/tj/assert
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"
github.com/golang/protobuf/jsonpb: "replace with github.com/gogo/protobuf/jsonpb"
google.golang.org/protobuf/encoding/protojson: "replace with github.com/gogo/protobuf/jsonpb"
github.com/golang/protobuf/proto: "replace with github.com/gogo/protobuf/proto"
google.golang.org/protobuf/proto: "replace with github.com/gogo/protobuf/proto"
github.com/tj/assert: "use github.com/stretchr/testify/assert"
github.com/m3db/m3x/instrument: "use github.com/m3db/m3/instrument"
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:
- someword
exhaustive:
default-signifies-exhaustive: true
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: 100
# 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.

# 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
gci:
# gci control golang package import order and make it always deterministic
local-prefixes: github.com/m3db/m3db-operator

linters:
enable:
- deadcode
- dogsled
- dupl
- errcheck
- exhaustive
- gci
- goconst
- gocritic
- gocyclo
- goimports
- golint
- gosimple
- govet
- ineffassign
- lll
- maligned
- megacheck
- misspell
- prealloc
- staticcheck
- structcheck
- typecheck
- unconvert
- unparam
- varcheck
enable-all: false
disable:
- gomnd
- gochecknoinits
# Globals gonna global
- gochecknoglobals
# Overly harsh about long functions
- funlen
# Linter that checks that every comment ends in a period.
- godot
# Linter that makes you always use _test packages.
- testpackage
# Overly opinionated about how to construct errors
- goerr113
# Noisy warnings about whether "nolint" directives are necessary
- nolintlint
# Deprecated project due to being prone to bad suggestions.
- interfacer
# Valid use for not explicitly setting every field when they are optional nil/empty.
- exhaustivestruct
# We allow cuddling assignment following conditions because there are valid
# logical groupings for this use-case (e.g. when evaluating config values).
- wsl
# Wrapcheck can cause errors until all callsites checking explicit error
# types like io.EOF are converted to use errors.Is instead. Re-enable this
# linter once all error checks are upgraded.
- wrapcheck
# godox prevents using TODOs or FIXMEs which can be useful for demarkation
# of future work.
- godox
# New line required before return would require a large fraction of the
# code base to need updating, it's not worth the perceived benefit.
- nlreturn
disable-all: false
presets:
# bodyclose, errcheck, gosec, govet, scopelint, staticcheck, typecheck
- bugs
# deadcode, ineffassign, structcheck, unparam, unused, varcheck
- unused
# gofmt, goimports
- format
# depguard, dupl, gochecknoglobals, gochecknoinits, goconst, gocritic,
# golint, gosimple, interfacer, lll, misspell, stylecheck, unconvert
- style
fast: false


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:
# Exclude table-driven tests from scopelint (https://github.com/golangci/golangci-lint/issues/281).
- "Using the variable on range scope `tt` in function literal"
- "Using the variable on range scope `test` in function literal"
# It's common to shadow `err` and rarely indicates a problems. See
# https://github.com/golang/go/issues/19490 for further details.
- 'shadow: declaration of "err" shadows declaration'
# We commonly expose profiling information on /debug/pprof so we need to disable the gosec
# lint for it.
- "Profiling endpoint is automatically exposed on /debug/pprof"
# The logger is often our last option to communicate that an error occurred so if it returns
# an error we don't have an alternative to use. Since it's already unlikely that `Log` will
# return an error anyway we often skip checking the error for brevity.
- "Error return value of `\\(github.com\\/go-kit\\/kit\\/log.Logger\\).Log` is not checked"
# The caller is responsible for closing the Body of an `http.Response`. However, this step
# is usually performed in a defer function after the response has already been processed and
# so errors, which are already rare, can usually be safely ignored.
- "Error return value of `[a-zA-Z.]+.Body.Close` is not checked"
# The errcheck linter already checks for unhandled errors so we can disable the equivalent
# lint by gosec.
- "G104: Errors unhandled"

# Excluding configuration per-path, per-linter, per-text and per-source
exclude-rules:
# Exclude lll issues for long lines with go:generate
- linters:
- lll
source: "^//go:generate "
# Exclude some linters from running on tests files.
# - path: _test\.go
# linters:
# - gocyclo
# - errcheck
# - dupl
# - gosec


# 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: false

# 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 created after git revision `REV`
new-from-rev: a0ecbf9d10486673d7581f4e6a8b0f30497f8a66
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How many errors are there if we remove this? I.e. is it worth it to bite the bullet now and fix them?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, it looks pretty gnarly. This SGTM.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Around 600 errors. Top 5 linters by error count:

 138 paralleltest
 115 gci
 105 lll
  44 errcheck
  30 errorlint

36 changes: 0 additions & 36 deletions .metalinter.json

This file was deleted.

Loading