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

Add linter to the building pipeline #30896

Merged
merged 21 commits into from
Mar 22, 2022
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
169 changes: 169 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# DO NOT EDIT!
# This file is a rendered template, the source can be found in "./dev-tools/templates/.golangci.yml"
#
# options for analysis running
run:
# timeout for analysis, e.g. 30s, 5m, default is 1m
timeout: 15m

issues:
# Maximum count of issues with the same text.
# Set to 0 to disable.
# Default: 3
max-same-issues: 3
# Maximum issues count per one linter.
# Set to 0 to disable.
# Default: 50
max-issues-per-linter: 50

output:
sort-results: true

# Uncomment and add a path if needed to exclude
# skip-dirs:
# - some/path
# skip-files:
# - ".*\\.my\\.go$"
# - lib/bad.go

# Find the whole list here https://golangci-lint.run/usage/linters/
linters:
disable-all: true
enable:
- deadcode # finds unused code
- errcheck # checking for unchecked errors in go programs
- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
- goconst # finds repeated strings that could be replaced by a constant
- dupl # tool for code clone detection
- forbidigo # forbids identifiers matched by reg exps
- gosimple # linter for Go source code that specializes in simplifying a code
- misspell # finds commonly misspelled English words in comments
- nakedret # finds naked returns in functions greater than a specified function length
- prealloc # finds slice declarations that could potentially be preallocated
- nolintlint # reports ill-formed or insufficient nolint directives
- staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks
- stylecheck # a replacement for golint
- unparam # reports unused function parameters
- unused # checks Go code for unused constants, variables, functions and types

- govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
- ineffassign # detects when assignments to existing variables are not used
- structcheck # finds unused struct fields
- typecheck # Like the front-end of a Go compiler, parses and type-checks Go code
- varcheck # Finds unused global variables and constants
- asciicheck # simple linter to check that your code does not contain non-ASCII identifiers
- bodyclose # checks whether HTTP response body is closed successfully
- durationcheck # check for two durations multiplied together
- exportloopref # checks for pointers to enclosing loop variables
- goimports # Goimports does everything that gofmt does. Additionally it checks unused imports
- gosec # inspects source code for security problems
- importas # enforces consistent import aliases
- nilerr # finds the code that returns nil even if it checks that the error is not nil.
- noctx # noctx finds sending http request without context.Context
- unconvert # Remove unnecessary type conversions
- wastedassign # wastedassign finds wasted assignment statements.
- godox # tool for detection of FIXME, TODO and other comment keywords
- gomodguard # check for blocked dependencies

# all available settings of specific linters
linters-settings:
errcheck:
# report about not checking of errors in type assertions: `a := b.(MyStruct)`;
# default is false: such cases aren't reported by default.
check-type-assertions: true

errorlint:
# Check whether fmt.Errorf uses the %w verb for formatting errors. See the readme for caveats
errorf: true
# Check for plain type assertions and type switches
asserts: true
# Check for plain error comparisons
comparison: true

goconst:
# minimal length of string constant, 3 by default
min-len: 3
# minimal occurrences count to trigger, 3 by default
min-occurrences: 2

dupl:
# tokens count to trigger issue, 150 by default
threshold: 100

forbidigo:
# Forbid the following identifiers
forbid:
- fmt.Print.* # too much log noise
# Exclude godoc examples from forbidigo checks. Default is true.
exclude_godoc_examples: true

goimports:
local-prefixes: github.com/elastic

gomodguard:
blocked:
# List of blocked modules.
modules:
# Blocked module.
- github.com/pkg/errors:
# Recommended modules that should be used instead. (Optional)
recommendations:
- errors
- fmt
reason: "This package is deprecated, use `fmt.Errorf` with `%w` instead"

gosimple:
# Select the Go version to target. The default is '1.13'.
go: "1.17.6"

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

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: 0

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

nolintlint:
# Enable to ensure that nolint directives are all used. Default is true.
allow-unused: false
# Disable to ensure that nolint directives don't have a leading space. Default is true.
allow-leading-space: false
# Exclude following linters from requiring an explanation. Default is [].
allow-no-explanation: []
# Enable to require an explanation of nonzero length after each nolint directive. Default is false.
require-explanation: true
# Enable to require nolint directives to mention the specific linter being suppressed. Default is false.
require-specific: true

staticcheck:
# Select the Go version to target. The default is '1.13'.
go: "1.17.6"
checks: ["all"]

stylecheck:
Copy link
Member

Choose a reason for hiding this comment

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

For stylecheck (or some other linter) could we check that godoc follows the https://staticcheck.io/docs/checks/#ST1020. Like if you are going to put effort into writing docs for your exported function then follow the Go conventions 😄. FWIW we have that check in go-libaudit (had to disable one of the default excludes).

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually thought it was the default behaviour, I used to see this errors before in one of my previous projects. I tested locally and even with checks: ["all"] for staticcheck and stylecheck it didn't return any errors for missing or malformed go doc strings. I'll investigate further.

Copy link
Member

Choose a reason for hiding this comment

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

It's probably one of the default rules for excluding issues listed in https://golangci-lint.run/usage/configuration#command-line-options. Like maybe EXC0014.

Adding something like this can workaround it:

issues:
  include:
   # If you're going to write a comment follow the conventions.
   # https://go.dev/doc/effective_go#commentary.
   # comment on exported (.+) should be of the form "(.+)..."
   - EXC0014

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add it later as a separate PR, since this is already stable and approved.

# Select the Go version to target. The default is '1.13'.
go: "1.17.6"
checks: ["all"]

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

unused:
# Select the Go version to target. The default is '1.13'.
go: "1.17.6"
3 changes: 3 additions & 0 deletions CHANGELOG-developer.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ https://github.com/elastic/beats/compare/7.x..main[Check the HEAD diff]
==== Breaking changes
- Replace custom Pins type for a slice of string for defining the `ca_sha256` values.

==== Added
- Added linter step to the CI pipeline based on golangci-lint linter and Mage

=== Beats version 7.5.1
https://github.com/elastic/beats/compare/v7.5.0..v7.5.1[Check the HEAD diff]

Expand Down
9 changes: 6 additions & 3 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,12 @@ def runLinting() {
mapParallelTasks["${k}"] = v
}
}
mapParallelTasks['default'] = { cmd(label: 'make check-default', script: 'make check-default') }
mapParallelTasks['default'] = {
cmd(label: 'make check-default', script: 'make check-default')
cmd(label: 'Lint Last Change [Linux]', script: 'GOOS=linux mage -v llc')
cmd(label: 'Lint Last Change [Darwin]', script: 'GOOS=darwin mage -v llc')
cmd(label: 'Lint Last Change [Windows]', script: 'GOOS=windows mage -v llc')
}
mapParallelTasks['pre-commit'] = runPreCommit()
parallel(mapParallelTasks)
}
Expand Down Expand Up @@ -974,8 +979,6 @@ def dumpVariables(){
GOIMPORTS: ${env.GOIMPORTS}
GOIMPORTS_REPO: ${env.GOIMPORTS_REPO}
GOIMPORTS_LOCAL_PREFIX: ${env.GOIMPORTS_LOCAL_PREFIX}
GOLINT: ${env.GOLINT}
GOLINT_REPO: ${env.GOLINT_REPO}
GOPACKAGES_COMMA_SEP: ${env.GOPACKAGES_COMMA_SEP}
GOX_FLAGS: ${env.GOX_FLAGS}
GOX_OS: ${env.GOX_OS}
Expand Down
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ PYTHON_EXE?=python3
PYTHON_ENV_EXE=${PYTHON_ENV}/bin/$(notdir ${PYTHON_EXE})
VENV_PARAMS?=
FIND=find . -type f -not -path "*/build/*" -not -path "*/.git/*"
GOLINT=golint
GOLINT_REPO=golang.org/x/lint/golint
XPACK_SUFFIX=x-pack/

# PROJECTS_XPACK_PKG is a list of Beats that have independent packaging support
Expand Down
Loading