diff --git a/.github/workflows/pr-linter-check.yml b/.github/workflows/pr-linter-check.yml new file mode 100644 index 000000000..f2dc7c6e6 --- /dev/null +++ b/.github/workflows/pr-linter-check.yml @@ -0,0 +1,14 @@ +name: Pull Request Linter Check +on: [pull_request] +jobs: + + build: + name: Run Linter Check + runs-on: ubuntu-latest + steps: + + - name: Check out the code + uses: actions/checkout@v2 + + - name: Linter check + run: make lint diff --git a/Makefile b/Makefile index baaf1a949..9d73fb4b0 100644 --- a/Makefile +++ b/Makefile @@ -44,6 +44,9 @@ MANIFEST_PLATFORMS ?= amd64 ppc64le arm arm64 GIT_SHA = $(shell git rev-parse HEAD) GIT_DIRTY = $(shell git status --porcelain 2> /dev/null) +# The default linters used by lint and local-lint +LINTERS ?= "gosec,goconst,gofmt,goimports,unparam" + ### ### These variables should not need tweaking. ### @@ -158,6 +161,7 @@ shell: build-dirs build-image -v "$$(pwd)/.go/std:/go/std:delegated" \ -v "$$(pwd)/.go/std/$(GOOS)/$(GOARCH):/usr/local/go/pkg/$(GOOS)_$(GOARCH)_static:delegated" \ -v "$$(pwd)/.go/go-build:/.cache/go-build:delegated" \ + -v "$$(pwd)/.go/golangci-lint:/.cache/golangci-lint:delegated" \ -w /github.com/vmware-tanzu/velero \ $(BUILDER_IMAGE) \ /bin/sh $(CMD) @@ -219,12 +223,32 @@ ifneq ($(SKIP_TESTS), 1) @$(MAKE) shell CMD="-c 'hack/verify-all.sh'" endif +lint: +ifneq ($(SKIP_TESTS), 1) + @$(MAKE) shell CMD="-c 'hack/lint.sh $(LINTERS)'" +endif + +local-lint: +ifneq ($(SKIP_TESTS), 1) + @hack/lint.sh $(LINTERS) +endif + +lint-all: +ifneq ($(SKIP_TESTS), 1) + @$(MAKE) shell CMD="-c 'hack/lint.sh $(LINTERS) true'" +endif + +local-lint-all: +ifneq ($(SKIP_TESTS), 1) + @hack/lint.sh $(LINTERS) true +endif + update: @$(MAKE) shell CMD="-c 'hack/update-all.sh'" build-dirs: @mkdir -p _output/bin/$(GOOS)/$(GOARCH) - @mkdir -p .go/src/$(PKG) .go/pkg .go/bin .go/std/$(GOOS)/$(GOARCH) .go/go-build + @mkdir -p .go/src/$(PKG) .go/pkg .go/bin .go/std/$(GOOS)/$(GOARCH) .go/go-build .go/golangci-lint build-image: cd hack/build-image && docker build --pull -t $(BUILDER_IMAGE) . diff --git a/changelogs/unreleased/2615-tbatard b/changelogs/unreleased/2615-tbatard new file mode 100644 index 000000000..4e4c7a1a3 --- /dev/null +++ b/changelogs/unreleased/2615-tbatard @@ -0,0 +1 @@ +Add linter checks to Makefile \ No newline at end of file diff --git a/golangci.yaml b/golangci.yaml new file mode 100644 index 000000000..0b6da27f7 --- /dev/null +++ b/golangci.yaml @@ -0,0 +1,415 @@ +# This file contains all available configuration options +# with their default values. + +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + timeout: 5m + + # 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: + # - mytag + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + #skip-dirs: + # - src/external_libs + # - autogenerated_by_my_lib + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + + # 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. + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + # skip-files: + # - ".*\\.my\\.go$" + # - lib/bad.go + + # 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 + + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners: false + + +# 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 + + # make issues output unique by line, default is true + uniq-by-line: true + + +# all available settings of specific linters +linters-settings: + dogsled: + # checks assignments with too many blank identifiers; default is 2 + max-blank-identifiers: 2 + dupl: + # tokens count to trigger issue, 150 by default + threshold: 100 + 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: false + + # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`; + # default is false: such cases aren't reported by default. + check-blank: false + + # [deprecated] comma-separated list of pairs of the form pkg:regex + # the regex is used to ignore names within pkg. (default "fmt:.*"). + # see https://github.com/kisielk/errcheck#the-deprecated-method for details + # ignore: fmt:.*,io/ioutil:^Read.* + + # path to a file containing a list of functions to exclude from checking + # see https://github.com/kisielk/errcheck#excluding-functions for details + # exclude: /path/to/file.txt + exhaustive: + # indicates that switch statements are to be considered exhaustive if a + # 'default' case is present, even if all enum members aren't listed in the + # switch + default-signifies-exhaustive: false + funlen: + lines: 60 + statements: 40 + gocognit: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + nestif: + # minimal complexity of if statements to report, 5 by default + min-complexity: 4 + goconst: + # minimal length of string constant, 3 by default + min-len: 3 + # minimal occurrences count to trigger, 3 by default + min-occurrences: 3 + 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. + # enabled-checks: + # - rangeValCopy + + # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty + # disabled-checks: + # - regexpMust + + # 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 + # disabled-tags: + # - experimental + + settings: # settings passed to gocritic + captLocal: # must be valid enabled check name + paramsOnly: true + # rangeValCopy: + # sizeThreshold: 32 + gocyclo: + # minimal code complexity to report, 30 by default (but we recommend 10-20) + min-complexity: 10 + godot: + # check all top-level comments, not only declarations + check-all: false + godox: + # report any comments starting with keywords, this is useful for TODO or FIXME comments that + # might be left in the code accidentally and should be resolved before merging + keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting + - NOTE + - OPTIMIZE # marks code that should be optimized before merging + - HACK # marks hack-arounds that should be removed before merging + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: github.com/org/project + golint: + # minimal confidence for issues, default is 0.8 + min-confidence: 0.8 + gomnd: + settings: + mnd: + # the list of enabled checks, see https://github.com/tommy-muehle/go-mnd/#checks for description. + checks: argument,case,condition,operation,return,assign + gomodguard: + allowed: + modules: # List of allowed modules + # - gopkg.in/yaml.v2 + domains: # List of allowed module domains + # - golang.org + blocked: + modules: # List of blocked modules + # - github.com/uudashr/go-module: # Blocked module + # recommendations: # Recommended modules that should be used instead (Optional) + # - golang.org/x/mod + # reason: "`mod` is the official go.mod parser library." # Reason why the recommended module should be used (Optional) + versions: # List of blocked module version constraints + # - github.com/mitchellh/go-homedir: # Blocked module with version constraint + # version: "< 1.1.0" # Version constraint, see https://github.com/Masterminds/semver#basic-comparisons + # reason: "testing if blocked version constraint works." # Reason why the version constraint exists. (Optional) + govet: + # report about shadowed variables + check-shadowing: true + + # settings per analyzer + settings: + printf: # analyzer name, run `go tool vet help` to see all analyzers + funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf + - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf + + # enable or disable analyzers by name + enable: + - atomicalign + enable-all: false + disable: + - shadow + disable-all: false + depguard: + list-type: blacklist + include-go-root: false + packages: + - github.com/sirupsen/logrus + packages-with-error-message: + # specify an error message to output when a blacklisted package is used + - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log" + 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: 120 + # tab width in spaces. Default to 1. + tab-width: 1 + maligned: + # print struct with more effective memory layout or not, false by default + suggest-new: true + 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 + 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 + 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: true + # 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 + rowserrcheck: + packages: + - github.com/jmoiron/sqlx + testpackage: + # regexp pattern to skip files + skip-regexp: (export|internal)_test\.go + 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: + # 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 + whitespace: + multi-if: false # Enforces newlines (or comments) after every multi-line if statement + multi-func: false # Enforces newlines (or comments) after every multi-line function signature + wsl: + # If true append is only allowed to be cuddled if appending value is + # matching variables, fields or types on line above. Default is true. + strict-append: true + # Allow calls and assignments to be cuddled as long as the lines have any + # matching variables, fields or types. Default is true. + allow-assign-and-call: true + # Allow multiline assignments to be cuddled. Default is true. + allow-multiline-assign: true + # Allow declarations (var) to be cuddled. + allow-cuddle-declarations: false + # Allow trailing comments in ending of blocks + allow-trailing-comment: false + # Force newlines in end of case at this limit (0 = never). + force-case-trailing-whitespace: 0 + # Force cuddling of err checks with err var assignment + force-err-cuddling: false + # Allow leading comments to be separated with empty liens + allow-separated-leading-comment: false + + # The custom section can be used to define linter plugins to be loaded at runtime. See README doc + # for more info. + # custom: + # Each custom linter should have a unique name. + # example: + # The path to the plugin *.so. Can be absolute or local. Required for each custom linter + # path: /path/to/example.so + # The description of the linter. Optional, just for documentation purposes. + # description: This is an example usage of a plugin linter. + # Intended to point to the repo location of the linter. Optional, just for documentation purposes. + # original-url: github.com/golangci/example-linter + +linters: +# enable: +# - megacheck +# - govet +# disable: +# - maligned +# - prealloc + disable-all: true + presets: +# - bugs +# - unused + 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: +# - abcdef +# +# # 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 +# +# # Exclude known linters from partially hard-vendored code, +# # which is impossible to exclude via "nolint" comments. +# - path: internal/hmac/ +# text: "weak cryptographic primitive" +# linters: +# - gosec +# +# # Exclude some staticcheck messages +# - linters: +# - staticcheck +# text: "SA9003:" +# +# # Exclude lll issues for long lines with go:generate +# - linters: +# - lll +# source: "^//go:generate " + + # 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 + + # The default value is false. If set to true exclude and exclude-rules + # regular expressions become case sensitive. + exclude-case-sensitive: false + + # The list of ids of default excludes to include or disable. By default it's empty. + include: + - EXC0002 # disable excluding of issues about comments from golint + + # 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 + + # Show only new issues created after git revision `REV` + new-from-rev: REV + + # Show only new issues created in git patch with set file path. + new-from-patch: path/to/patch/file + +severity: + # Default value is empty string. + # Set the default severity for issues. If severity rules are defined and the issues + # do not match or no severity is provided to the rule this will be the default + # severity applied. Severities should match the supported severity names of the + # selected out format. + # - Code climate: https://docs.codeclimate.com/docs/issues#issue-severity + # - Checkstyle: https://checkstyle.sourceforge.io/property_types.html#severity + # - Github: https://help.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity: error + + # The default value is false. + # If set to true severity-rules regular expressions become case sensitive. + case-sensitive: false + + # Default value is empty list. + # When a list of severity rules are provided, severity information will be added to lint + # issues. Severity rules have the same filtering capability as exclude rules except you + # are allowed to specify one matcher per severity rule. + # Only affects out formats that support setting severity information. + rules: + - linters: + - dupl + severity: info diff --git a/hack/build-image/Dockerfile b/hack/build-image/Dockerfile index ac4087010..db0baa970 100644 --- a/hack/build-image/Dockerfile +++ b/hack/build-image/Dockerfile @@ -45,3 +45,6 @@ RUN wget --quiet https://github.com/goreleaser/goreleaser/releases/download/v0.1 tar xvf goreleaser_Linux_x86_64.tar.gz && \ mv goreleaser /usr/bin/goreleaser && \ chmod +x /usr/bin/goreleaser + +# get golangci-lint +RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.27.0 \ No newline at end of file diff --git a/hack/lint.sh b/hack/lint.sh new file mode 100755 index 000000000..b9aeed43e --- /dev/null +++ b/hack/lint.sh @@ -0,0 +1,36 @@ +#!/bin/bash +# +# Copyright 2020 the Velero contributors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +LINTERS=${1:-"gosec,goconst,gofmt,goimports,unparam"} +ALL=${2:-false} + +HACK_DIR=$(dirname "${BASH_SOURCE[0]}") + +# Printing out cache status +golangci-lint cache status + +if [[ $ALL == true ]] ; then + action="" +else + action="-n" +fi + +# Enable GL_DEBUG line below for debug messages for golangci-lint +# export GL_DEBUG=loader,gocritic,env +CMD="golangci-lint run -E ${LINTERS} $action -c $HACK_DIR/../golangci.yaml" +echo "Running $CMD" + +eval $CMD diff --git a/site/docs/master/development.md b/site/docs/master/development.md index cf3b3c4fc..22d2fead0 100644 --- a/site/docs/master/development.md +++ b/site/docs/master/development.md @@ -19,6 +19,19 @@ The following files are automatically generated from the source code: You can run `make verify` to ensure that all generated files (clientset, listers, shared informers, docs) are up to date. +## Linting + +You can run `make lint` which executes golangci-lint inside the build image, or `make local-lint` which executes outside of the build image. +Both `make lint` and `make local-lint` will only run the linter against changes. + +Use `lint-all` to run the linter against the entire code base. + +The default linters are defined in the `Makefile` via the `LINTERS` variable. + +You can also override the default list of linters by running the command + +`$ make lint LINTERS=gosec` + ## Test To run unit tests, use `make test`.