🌟 Let's talk about linters section of the configuration

[ldez]

Important

This is a proposal: I don't know if it is possible and what the impact could be inside the code.
The proposal may evolve.

---

This is an attempt to summarize and provide a solution for the management of the linters inside the configuration.

The current configuration looks like this:

linters:
  disable-all: true
  enable:
    - lintera
    - linterb
    - ...
  enable-all: true
  disable:
    - linterc
    - linterd
    - ...
  presets:
    - bugs
    - comment
    - complexity
    - ...
  fast: true

linters-settings:
  lintera:
    opt1: foo
  # ...

⛑️ The Problems

Presets

Existing presets

• 🐛: bugs
• 💬: comment
• 🧮: complexity
• 💢: error
• 💾: format
• 🔝: import
• 🎛️: metalinter
• 📦: module
• 📊: performance
• 🗂️: sql
• 🎩: style
• 🚦: test
• 🧹: unused

	🐛	💬	🧮	💢	💾	🔝	🎛️️	📦	📊	🗂️️	🎩	🚦	🧹
asasalint	🐛
asciicheck	🐛										🎩
bidichk	🐛
bodyclose	🐛								📊
canonicalheader											🎩
containedctx											🎩
contextcheck	🐛
copyloopvar											🎩
cyclop			🧮
decorder											🎩
depguard						🔝		📦			🎩
dogsled											🎩
dupl											🎩
dupword		💬
durationcheck	🐛
err113				💢							🎩
errcheck	🐛			💢
errchkjson	🐛
errname											🎩
errorlint	🐛			💢
exhaustive	🐛
exhaustruct											🎩	🚦
exptostd											🎩
fatcontext									📊
forbidigo											🎩
forcetypeassert											🎩
funlen			🧮
gci					💾	🔝
ginkgolinter											🎩
gocheckcompilerdirectives	🐛
gochecknoglobals											🎩
gochecknoinits											🎩
gochecksumtype	🐛
gocognit			🧮
goconst											🎩
gocritic							🎛️				🎩
gocyclo			🧮
godot		💬									🎩
godox		💬									🎩
gofmt					💾
gofumpt					💾
goheader											🎩
goimports					💾	🔝
gomoddirectives								📦			🎩
gomodguard						🔝		📦			🎩
goprintffuncname											🎩
gosec	🐛
gosimple											🎩
gosmopolitan	🐛
govet	🐛						🎛️
grouper											🎩
iface											🎩
importas											🎩
inamedparam											🎩
ineffassign													🧹
interfacebloat											🎩
intrange											🎩
ireturn											🎩
lll											🎩
loggercheck	🐛										🎩
maintidx			🧮
makezero	🐛										🎩
mirror											🎩
misspell		💬									🎩
mnd											🎩
musttag	🐛										🎩
nakedret											🎩
nestif			🧮
nilerr	🐛
nilnesserr	🐛
nilnil											🎩
nlreturn											🎩
noctx	🐛								📊
nolintlint											🎩
nonamedreturns											🎩
nosprintfhostport											🎩
paralleltest											🎩	🚦
perfsprint									📊
prealloc									📊
predeclared											🎩
promlinter											🎩
protogetter	🐛
reassign	🐛
recvcheck	🐛
revive							🎛️				🎩
rowserrcheck	🐛									🗂️
sloglint											🎩
spancheck	🐛
sqlclosecheck	🐛									🗂️
staticcheck	🐛						🎛️
stylecheck											🎩
tagalign											🎩
tagliatelle											🎩
tenv												🚦
testableexamples												🚦
testifylint	🐛											🚦
testpackage											🎩	🚦
thelper												🚦
tparallel											🎩	🚦
unconvert											🎩
unparam													🧹
unused													🧹
usestdlibvars											🎩
usetesting												🚦
varnamelen											🎩
wastedassign											🎩
whitespace											🎩
wrapcheck				💢							🎩
wsl											🎩
zerologlint	🐛

The current presets option is not related to "presets" but to "categories"/topics.
The scope of a preset is too large because they are "categories"/topics and not a consistent or recommended subset of linters as expected by the name "presets".

I think this is difficult to use because there are a lot of linters, linters are in several "presets", and a "preset" can contain potential "incompatible" linters.

ex:

• preset complexity: cyclop and gocyclo are redundant
• preset format: gci, gofmt, gofumpt, goimports are mainly redudant (cf proposal about formatters) and "incompatible".
• preset bugs: exhaustive, gochecksumtype can be related to bugs but they are very specific.
• rowserrcheck is inside sql and bugs
• etc.

Note: I use quotes around the word "categories" because it can be ambiguous due to the field Category inside the analysis.Diagnostic structure.

Default Linters

The current default set of linters is small and contains only "slow linters" and cannot be changed without a breaking change.

enable-all and disable-all

The options enable-all and disable-all are extremely useful.

• Let's discuss about the deprecation of enable-all #1888

But they have some "unexpected" behaviors:

• disable cannot be used with disable-all
• enable cannot be used with enable-all

In some way, these options are fighting against the default set of linters.

Fast

The fast option has an unexpected behavior: this is not a filter.

• The --fast flag does not overwrite the configuration file #1909 (comment)

Misc.

Also linters-settings option name contains a grammatical error: it should be linter-settings (note the s at the end of the word linter).

💭 The Proposal

• Removes (deprecates) the presets option. For now, I don't know if it is useful to provide a replacement due to the nature of this option but maybe topics.
• Removes (deprecates) enable-all and disable-all options.
• Removes (deprecates) the fast option.
• Adds a new option default:
  • This option will be a string (not a slice).
  • It will define the default set of linters
    • standard (or default-v1, or legacy): It enables the current default set of linters. This value will be the default.
      • if we use default-v1, or legacy, we can create a new set of "default" linters called standard or default-v2.
    • all: this is the equivalent of enable-all. It enables all linters.
    • none: this is the equivalent of disable-all. It disables all linters.
    • fast: Similar to the current fast. It enables all "fast" linters but this is not a filter on other enabled linters.
    • recommended: an "evolving" list of linters that we recommend for all projects. (This one can be ambiguous, so we can ignore it for now)
• Adds a flag-only option --fast-only: a filter on the enabled linters. (similar to --enable-only)
• Removes (deprecates) the section linters-settings and replace it with a subsection settings inside linters

linters:
  default: standard # standard/all/none/fast
  enable:
    - lintera
    - linterb
    - ...
  disable:
    - linterc
    - linterd
    - ...
  settings:
    lintera:
      opt1: foo
    # ...

---

Caution

Shareable configuration is another topic, and will not be handled in v2, so everything related to that is off-topic for now.
But be sure that the topic will be handled in the future.

[bombsimon]

I think removing presets sounds like a good idea exactly for your reasoning. I think most linters don't actually find bugs but more style that might be a bug.

---

Is the key/name default taken from ESLint? It seems like they do something like this:

export default [
    somePreset,
    { /* other rules */ }
]

I'm not sure I like that for golangci-lint, I think settings.default feels odd when you can set it to multiple values for just linters. I also think it should make sense spelling it out in the CLI, I'm not sure golangci-lint run --default none --enable gofmt is it. default feels more related to JavaScript than the config.

Sadly, I don't know what would be much better and I'm open to changing my mind, but maybe preset or just config could be something if we name the setups accordingly. E.g. maybe none should be named no-linters to not mix up with "no config" (or "no default" for that matter).

linters:
  config: standard
  enable:
    - linter-not-in-standard
  disable:
    - linter-in-standard-i-dont-want

golangci-lint run --config no-linters --enable gofmt

I see your very clear off topic note there, but I can't completely ignore the idea of being able to have a set of linters/settings and then being able to extend it in the back of my head as well which also influences my feelings on this topic.

---

Just to clarify, --fast-only will run only fast linters from enabled linters set whereas the default fast enables all fast linters? So essentially it's a rename of --fast to --fast-only?

[ldez]

About linters.default, I propose linters.base.

config is related to the configuration file.

Note: This is not settings.default but linters.default.
My proposal is not related to ESLint.

---

Just to clarify, --fast-only will run only fast linters from enabled linters set whereas the default fast enables all fast linters? So essentially it's a rename of --fast to --fast-only?

This is not a rename.

--fast enables "fast" linters but doesn't filter them.

It's a kind of "hidden" preset.

The replacement of --fast is more linters.default: fast.

--fast-only is a filter on the enabled linters.

[bombsimon]

About linters.default, I propose linters.base.

config is related to the configuration file.

Note: This is not settings.default but linters.default. My proposal is not related to ESLint.

Oops, yes I mean linters.default, nothing else. Hmm, base can work but just sounds like the same word as default so maybe just keep default if those are the only options.

This is not a rename.

--fast enables "fast" linters but doesn't filter them.

If --fast runs all fast linters even if they're not enabled I think our docs are wrong:

golangci-lint/pkg/commands/flagsets.go

Lines 27 to 28 in e85310c

	internal.AddFlagAndBind(v, fs, fs.Bool, "fast", "linters.fast", false,
	color.GreenString("Enable only fast linters from enabled linters set (first run won't be fast)"))

golangci-lint/.golangci.reference.yml

Lines 269 to 271 in e85310c

	# Enable only fast linters from enabled linters set (first run won't be fast)
	# Default: false
	fast: true

[ldez]

I recommend reading this comment: #1909 (comment)

TLDR: it filters presets but not linters defined in the enable section.

The documentation is right and wrong at the same time 😸

Your comments prove that nearly nobody understands how --fast works because the current implementation is not the expected behavior.

[ldez]

Just a note: linters.default defines a set of linters only. Not linter/configuration tuples.

[bombsimon]

Just a note: linters.default defines a set of linters only. Not linter/configuration tuples.

Yep, all clear, just trying to think about the future without thinking to much about the future here 😸 Given the linked comment, I totally see what --fast-only is for, I just thought that was what --fast did. I think it's very clear what the difference between --default fast and --fast-only is and I think it's a good opportunity to make --fast-only do what's expected.