Go linters configuration, the right version.

Originally posted here: https://olegk.dev/go-linters-configuration-the-right-version

TLDR: See the golangci-lint config that I find useful Github Gist

Intro

Ah, these mighty linters. Tools that intend to make our life better but might hurt our software engineering egos.

Somewhere in 2018, I was super eager to make my code and others' code better. Thankfully, I met @quasilyte on Gophers Slack and we made "The most opinionated Go source code linter" go-critic.

But we were not alone, golangci-lint started as a gometalinter replacement and became a multi-linter in the Go ecosystem. If you have a linter and it makes code better for everyone and not only for your specific use case, consider adding it to golangci-lint so everyone can benefit from that.

(a funny thing that I found during writing this post: both repositories start their git commit history Apr 29, 2018, LOL)

As for today golangci-lint contains 120+ linters and what I often notice:

• not everyone is aware of how to configure it
• what are the pros and cons of some fields
• what is The Best™ golangci-lint config
• etc.

In this post, I'm going to share my config and some thoughts on why I prefer this or that. The post might look long due to the big YAML sections (hah, YAML), but most of the stuff is quite simple.

Note: this post is based on golangci-lint version v1.52.1.

Linters environment

When you're planning to start using or running a linter you need to understand the environment. I see 2 cases:

• personal projects
  • where you decide how to write the code.
• team projects
  • where there are other members with their opinion.

For personal projects everything is cool - just do the stuff, run a linter with any config you want, no problems.

But with a team situation is different. If you are going to introduce a linter, change its config or disable a check - consider discussing it first, to have a team-wide agreement. This will cause less conflicts later (both code and personal).

Everything that is about style is subjective. 1 newline in a pull request can start a 10+ comments thread because everyone has something to say. I would like to say this was a joke but it's not, true story.

Running a linter

Software engineering or coding in general is like riding a bike. When you're young and not that experienced you gonna use these small additional wheels to move stably. But when you're experienced enough these wheels will distract you more and more. As you can guess wheels are linters.

After 5 years of writing Go daily I found that I don't see linter's suggestions because there is nothing to suggest. So, I switched from automatic runs to the manual, when I know/feel/think that something might be missed. There is a big chance that someone might find this idea over-optimistic but I know I'm not alone, consider this as a come out.

A short disclaimer: formatting or styling suggestions are fine, only bugs are unacceptable.

As for junior and middle engineers I strongly suggest to go 1st option and don't rush to disable the linter. Instead, I heavily recommend enabling more linters, to see more suggestions and keep learning from them. Even if you will not find yourself in a situation when the linter distracts you - it's fine, this doesn't impact your professional skills.

My handy command

I have a shell alias in my ~/.zshrc:

alias lnt="golangci-lint run --config=~/.golangci.yaml ./..."

Just type lnt in a specific directory and see the results. It's very handy when I do git clone of a repository and try to understand how good it is for me.

The config

It consists of 5 parts (as for golangci-lint v1.52.1):

• run
  • general config of the linter
• output
  • how the result should be presented
• issues & severity
  • how to treat the results
• linters & linters-settings
  • what linters to run and how to configure them

Let's discuss each of them.

run

The run object is responsible for the golangci-lint run. Here is what I found useful:

run:
  # Depends on your hardware, my laptop can survive 8 threads.
  concurrency: 8

  # I really care about the result, so I'm fine to wait for it.
  timeout: 30m

  # Fail if the error was met.
  issues-exit-code: 1

  # This is very important, bugs in tests are not acceptable either.
  tests: true

  # In most cases this can be empty but there is a popular pattern
  # to keep integration tests under this tag. Such tests often require
  # additional setups like Postgres, Redis etc and are run separately.
  # (to be honest I don't find this useful but I have such tags)
  build-tags:
    - integration 

  # Up to you, good for a big enough repo with no-Go code.
  skip-dirs:
    # - src/external_libs

  # When enabled linter will skip directories: vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  # Skipping `examples` sounds scary to me but skipping `testdata` sounds ok.
  skip-dirs-use-default: false

  # Autogenerated files can be skipped (I'm looking at you gRPC).
  # AFAIK autogen files are skipped but skipping the whole directory should be somewhat faster.
  #skip-files:
  #  - "protobuf/.*.go"

  # With the read-only mode linter will fail if go.mod file is outdated.
  modules-download-mode: readonly

  # Till today I didn't know this param exists, never ran 2 golangci-lint at once.
  allow-parallel-runners: false

  # Keep this empty to use the Go version from the go.mod file.
  go: ""

output

Let's briefly talk about output. As you can guess it's all about how results will be presented.

As I have shared my command (in My Handy command section) I'm using a bash alias. The results are saved in a lint.txt file that I can process in the editor.

output:
  # I prefer the simplest one: `line-number` and saving to `lint.txt`
  #
  # The `tab` also looks good and with the next release I will switch to it
  # (ref: https://github.com/golangci/golangci-lint/issues/3728)
  #
  # There are more formats which can be used on CI or by your IDE.
  format: line-number:lint.txt

  # I do not find this useful, parameter above already enables filepath
  # with a line and column. For me, it's easier to follow the path and
  # see the line in an IDE where I see more code and understand it better.
  print-issued-lines: false

  # Must have. Easier to understand the output.
  print-linter-name: true

  # No, no skips, everything should be reported.
  uniq-by-line: false

  # To be honest no idea when this can be needed, maybe a multi-module setup?
  path-prefix: ""

  # Slightly easier to follow the results + getting deterministic output.
  sort-results: true

issues & severity

Configuring the issues:

issues:
  # I found it strange to skip the errors, setting 0 to have all the results.
  max-issues-per-linter: 0

  # Same here, nothing should be skipped to not miss errors.
  max-same-issues: 0

  # When set to `true` linter will analyze only new code which are
  # not committed or after some specific revision. This is a cool 
  # feature when you're going to introduce linter into a big project.
  # But I prefer going gradually package by package. 
  # So, it's set to `false` to scan all code.
  new: false

  # 2 other params regarding git integration

  # Even with a recent GPT-4 release I still believe that
  # I know better how to do my job and fix the suggestions.
  fix: false

To be honest, I have never configured the severity part, it was always empty in my configuration, have nothing to recommend for you.

If you are using non-default parameters, please share your setup in the comments or DM me, happy to hear.

Linters to run

The core idea of the article: linters to enable or disable. Let me make a few disclaimers:

1) The following config is based on my experience and my preferences.

2) The linters that I keep disabled should not be treated as "this linter is bad" or "linter's author did a bad thing". Every linter makes sense, it just depends on the context and codebase.

How to find your best config: I recommend you enable all available linters, run on your codebase (or a few of them), analyze the results and decide what is important for you and what is not. Again: if this config will be used in a team - discuss it before adopting it, and be friendly.

(to get all available linters run: golangci-lint help linters)

In other words: YMMV (your mileage might vary). Keep that in mind. Let's go:

linters:
  # Set to true runs only fast linters.
  # Good option for 'lint on save', pre-commit hook or CI.
  fast: false

  enable:
    # Check for pass []any as any in variadic func(...any).
    # Rare case but saved me from debugging a few times.
    - asasalint

    # I prefer plane ASCII identifiers.
    # Symbol `∆` instead of `delta` looks cool but no thanks.
    - asciicheck

    # Checks for dangerous unicode character sequences.
    # Super rare but why not to be a bit paranoid?
    - bidichk

    # Checks whether HTTP response body is closed successfully.
    - bodyclose

    # Check whether the function uses a non-inherited context.
    - contextcheck

    # Check for two durations multiplied together.
    - durationcheck

    # Forces to not skip error check.
    - errcheck 

    # Checks `Err-` prefix for var and `-Error` suffix for error type.
    - errname

    # Suggests to use `%w` for error-wrapping.
    - errorlint

    # Checks for pointers to enclosing loop variables.
    - exportloopref

    # As you already know I'm a co-author. It would be strange to not use
    # one of my warmly loved projects.
    - gocritic

    # Forces to put `.` at the end of the comment. Code is poetry.
    - godot

    # Might not be that important but I prefer to keep all of them.
    # `gofumpt` is amazing, kudos to Daniel Marti https://github.com/mvdan/gofumpt
    - gofmt
    - gofumpt
    - goimports

    # Allow or ban replace directives in go.mod
    # or force explanation for retract directives.
    - gomoddirectives

    # Powerful security-oriented linter. But requires some time to
    # configure it properly, see https://github.com/securego/gosec#available-rules
    - gosec

    # Linter that specializes in simplifying code.
    - gosimple

    # Official Go tool. Must have. 
    - govet

    # Detects when assignments to existing variables are not used
    # Last week I caught a bug with it.
    - ineffassign

    # Even with deprecation notice I find it useful.
    # There are situations when instead of io.ReaderCloser 
    # I can use io.Reader. A small but good improvement.
    - interfacer

    # Fix all the misspells, amazing thing.
    - misspell

    # Finds naked/bare returns and requires change them.
    - nakedret

    # Both require a bit more explicit returns.
    - nilerr
    - nilnil

    # Finds sending HTTP request without context.Context.
    - noctx

    # Forces comment why another check is disabled.
    # Better not to have //nolint: at all ;)
    - nolintlint

    # Finds slices that could potentially be pre-allocated.
    # Small performance win + cleaner code.
    - prealloc

    # Finds shadowing of Go's predeclared identifiers.
    # I hear a lot of complaints from junior developers.
    # But after some time they find it very useful.
    - predeclared

    # Lint your Prometheus metrics name.
    - promlinter

    # Checks that package variables are not reassigned.
    # Super rare case but can catch bad things (like `io.EOF = nil`)
    - reassign

    # Drop-in replacement of `golint`.
    - revive

    # Somewhat similar to `bodyclose` but for `database/sql` package.
    - rowserrcheck
    - sqlclosecheck

    # I have found that it's not the same as staticcheck binary :\
    - staticcheck

    # Is a replacement for `golint`, similar to `revive`.
    - stylecheck

    # Check struct tags.
    - tagliatelle

    # Test-related checks. All of them are good.
    - tenv
    - testableexamples
    - thelper
    - tparallel

    # Remove unnecessary type conversions, make code cleaner
    - unconvert

    # Might be noisy but better to know what is unused
    - unparam

    # Must have. Finds unused declarations.
    - unused

    # Detect the possibility to use variables/constants from stdlib.
    - usestdlibvars

    # Finds wasted assignment statements.
    - wastedassign

And know some linters that I prefer to keep disabled. Again, my use case, yours might be different:

  disable:
    # Detects struct contained context.Context field. Not a problem.
    - containedctx

    # Checks function and package cyclomatic complexity.
    # I can have a long but trivial switch-case.
    #
    # Cyclomatic complexity is a measurement, not a goal.
    # (c) Bryan C. Mills / https://github.com/bcmills
    - cyclop

    # Abandoned, replaced by `unused`.
    - deadcode

    # Check declaration order of types, consts, vars and funcs.
    # I like it but I don't use it.
    - decorder

    # Checks if package imports are in a list of acceptable packages.
    # I'm very picky about what I import, so no automation.
    - depguard

    # Checks assignments with too many blank identifiers. Very rare.
    - dogsled

    # Tool for code clone detection.
    - dupl

    # Find duplicate words, rare.
    - dupword

    # I'm fine to check the error from json.Marshal ¯\_(ツ)_/¯
    - errchkjson

    # All SQL queries MUST BE covered with tests.
    - execinquery

    # Forces to handle more cases. Cool but noisy.
    - exhaustive
    - exhaustivestruct # Deprecated, replaced by check below.
    - exhaustruct

    # Forbids some identifiers. I don't have a case for it.
    - forbidigo

    # Finds forced type assertions, very good for juniors.
    - forcetypeassert

    # I might have long but a simple function.
    - funlen

    # Imports order. I do this manually ¯\_(ツ)_/¯
    - gci

    # I'm not a fan of ginkgo and gomega packages.
    - ginkgolinter

    # Checks that compiler directive comments (//go:) are valid. Rare.
    - gocheckcompilerdirectives

    # Globals and init() are ok.
    - gochecknoglobals
    - gochecknoinits

    # Same as `cyclop` linter (see above)
    - gocognit
    - goconst
    - gocyclo

    # TODO and friends are ok.
    - godox

    # Check the error handling expressions. Too noisy.
    - goerr113

    # I don't use file headers.
    - goheader

    # 1st Go linter, deprecated :( use `revive`.
    - golint

    # Reports magic consts. Might be noisy but still good.
    - gomnd

    # Allowed/blocked packages to import. I prefer to do it manually.
    - gomodguard

    # Printf-like functions must have -f.
    - goprintffuncname

    # Groupt declarations, I prefer manually.
    - grouper

    # Deprecated.
    - ifshort

    # Checks imports aliases, rare.
    - importas

    # Forces tiny interfaces, very subjective.
    - interfacebloat

    # Accept interfaces, return types. Not always.
    - ireturn

    # I don't set line length. 120 is fine by the way ;)
    - lll

    # Some log checkers, might be useful.
    - loggercheck

    # Maintainability index of each function, subjective.
    - maintidx

    # Slice declarations with non-zero initial length. Not my case.
    - makezero

    # Deprecated. Use govet `fieldalignment`.
    - maligned

    # Enforce tags in un/marshaled structs. Cool but not my case.
    - musttag

    # Deeply nested if statements, subjective.
    - nestif

    # Forces newlines in some places.
    - nlreturn

    # Reports all named returns, not that bad.
    - nonamedreturns

    # Deprecated. Replaced by `revive`.
    - nosnakecase

    # Finds misuse of Sprintf with host:port in a URL. Cool but rare.
    - nosprintfhostport

    # I don't use t.Parallel() that much.
    - paralleltest

    # Often non-`_test` package is ok.
    - testpackage

    # Compiler can do it too :)
    - typecheck

    # I'm fine with long variable names with a small scope.
    - varnamelen

    # gofmt,gofumpt covers that (from what I know).
    - whitespace

    # Don't find it useful to wrap all errors from external packages.
    - wrapcheck

    # Forces you to use empty lines. Great if configured correctly.
    # I mean there is an agreement in a team.
    - wsl

Linters config

Many of these linters can be configured more precisely via linters-settings config. Initially I planned to write about all of them but I think it will be just a waste of time. See Linters page and adapt to your own needs.

linters-settings:
  # I'm biased and I'm enabling more than 100 checks
  # Might be too much for you. See https://go-critic.com/overview.html
  gocritic:
    enabled-tags:
      - diagnostic
      - experimental
      - opinionated
      - performance
      - style
    disabled-checks:
      # These 3 will detect many cases, but they do sense
      # if it's performance oriented code
      - hugeParam
      - rangeExprCopy
      - rangeValCopy

  errcheck:
    # Report `a := b.(MyStruct)` when `a, ok := ...` should be.
    check-type-assertions: true # Default: false

    # Report skipped checks:`num, _ := strconv.Atoi(numStr)`.
    check-blank: true # Default: false

    # Function to skip.
    exclude-functions:
      - io/ioutil.ReadFile
      - io.Copy(*bytes.Buffer)
      - io.Copy(os.Stdout)

  govet:
    disable:
      - fieldalignment # I'm ok to waste some bytes

  nakedret:
    # No naked returns, ever.
    max-func-lines: 1 # Default: 30

  tagliatelle:
    case:
      rules:
        json: snake # why it's not a `snake` by default?!
        yaml: snake # why it's not a `snake` by default?!
        xml: camel
        bson: camel
        avro: snake
        mapstructure: kebab

Conclusions

I hope I haven't missed some linters. If so - don't forget to mention this in the comments. Here is the config as a Github gist. Feel free to copy and adapt it. There is no silver bullet perfect config, find yours.

Aaaand a huge kudos to Ludovic Fernandez who is currently keeping golangci-lint alive. Great job!

Thanks.

Links

DEV / Lobsters / Reddit / Twitter