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: Add golangci-lint, enforce Go best practices #335

Merged
merged 13 commits into from
Dec 9, 2019

Conversation

cpu
Copy link
Member

@cpu cpu commented Dec 9, 2019

This branch adds golangci-lint to the CI build/makefile. (Linters linting linters! 😵)

The configuration I've added (see .golangci-lint.yml) enables the following lints:

bodyclose: checks whether HTTP response body is closed successfully [fast: true, auto-fix: false]
deadcode: Finds unused code [fast: true, auto-fix: false]
dogsled: Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
gocyclo: Computes and checks the cyclomatic complexity of functions [fast: true, auto-fix: false]
gofmt: Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
goimports: Goimports does everything that gofmt does. Additionally it checks unused imports [fast: true, auto-fix: true]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
interfacer: Linter that suggests narrower interface types [fast: true, auto-fix: false]
misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: true, auto-fix: false]
structcheck: Finds unused struct fields [fast: true, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false]
unconvert: Remove unnecessary type conversions [fast: true, auto-fix: false]
unparam: Reports unused function parameters [fast: true, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: true, auto-fix: false]

I disabled a handful of others. Mostly because they are too opinionated (gocritic, whitespace, wsl, etc), not applicable (depguard, godox, gosec), or produced too many findings to address in this PR (golint, stylecheck, goconst). One day I'd like to come back around to enabling that last category when I can find the time to implement all of the renames/fixes required.

I broke up each commit in this branch based on the lint error that was being fixed. Mostly these are small diffs and a few caught real bugs (e.g. unit tests that were never run, dead code, missed err checking).

The home-spun make format-check target can be replaced entirely since it's now subsumed by the gofmt linter run by golangci-lint.

@cpu cpu self-assigned this Dec 9, 2019
@cpu cpu requested a review from zakird December 9, 2019 20:41
@zakird zakird merged commit c74b45b into zmap:master Dec 9, 2019
@cpu cpu deleted the cpu-who-lints-the-linters branch December 9, 2019 21:43
aaomidi pushed a commit to aaomidi/zlint that referenced this pull request Nov 29, 2022
This commit introduces the changes that were committed with
the PR at google/certificate-transparency-go#609
to zcrypto. This makes `GOOS=js GOARCH=wasm go build ./...` work.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants