Revive default linters vs gocritic/govet

[ccoVeille]

Welcome

• Yes, I've searched similar issues on GitHub and didn't find any.
• I agree to follow this project's Code of Conduct

Your feature request related to a problem? Please describe

I'm posting here as I think it could be part of v2 configuration for linters:

• 🪩 It's time to create a v2 🚀 #5300
• 🌟 Let's talk about linters section of the configuration #5299

But also, it might be addressed as a "bug" for the v1.

There is one thing that is a pain for now with the revive linters.
It behaves from the others like govet or gocritic when we want to enable one rule.
The behavior of default rules is inconsistent between linters.

Here is the minimal configuration to reproduce the issue

$ GL_DEBUG=gocritic,revive,govet go run cmd/golangci-lint/main.go run --no-config --enable-only=gocritic,govet,revive 

DEBU [govet] Default analyzers (33): [appends asmdecl assign atomic bools buildtag cgocall composites copylocks defers directive errorsas framepointer httpresponse ifaceassert loopclosure lostcancel nilfunc printf shift sigchanyzer slog stdmethods stdversion stringintconv structtag testinggoroutine tests timeformat unmarshal unreachable unsafeptr unusedresult] 
DEBU [govet] Enabled by config analyzers (32): [appends asmdecl assign atomic bools buildtag cgocall composites copylocks defers directive errorsas framepointer httpresponse ifaceassert lostcancel nilfunc printf shift sigchanyzer slog stdmethods stdversion stringintconv structtag testinggoroutine tests timeformat unmarshal unreachable unsafeptr unusedresult] 

DEBU [gocritic] Enabled by default checks (34): [appendAssign argOrder assignOp badCall badCond captLocal caseOrder codegenComment commentFormatting defaultCaseOrder deprecatedComment dupArg dupBranchBody dupCase dupSubExpr elseif exitAfterDefer flagDeref flagName ifElseChain mapKey newDeref offBy1 regexpMust singleCaseSwitch sloppyLen sloppyTypeAssert switchTrue typeSwitchVar underef unlambda unslice valSwap wrapperFunc] 
DEBU [gocritic] Final used checks (34): [appendAssign argOrder assignOp badCall badCond captLocal caseOrder codegenComment commentFormatting defaultCaseOrder deprecatedComment dupArg dupBranchBody dupCase dupSubExpr elseif exitAfterDefer flagDeref flagName ifElseChain mapKey newDeref offBy1 regexpMust singleCaseSwitch sloppyLen sloppyTypeAssert switchTrue typeSwitchVar underef unlambda unslice valSwap wrapperFunc] 

DEBU [revive] Default analyzers checks (23): [blank-imports context-as-argument context-keys-type dot-imports empty-block error-naming error-return error-strings errorf exported increment-decrement indent-error-flow package-comments range receiver-naming redefines-builtin-id superfluous-else time-naming unexported-return unreachable-code unused-parameter var-declaration var-naming] 
DEBU [revive] Enabled by config analyzers checks (23): [blank-imports context-as-argument context-keys-type dot-imports empty-block error-naming error-return error-strings errorf exported increment-decrement indent-error-flow package-comments range receiver-naming redefines-builtin-id superfluous-else time-naming unexported-return unreachable-code unused-parameter var-declaration var-naming] 

(please note the rendering for revive here use the changes made in #5325)

So here we can see that

We have:

• gocritic 34 => 34
• govet 33 => 32 (The difference here is because of loopclosure rule)
• revive 23 => 23

Let's now use the following config file

example.yml

linters:
  disable-all: true
  enable:
    - gocritic
    - govet
    - revive

linters-settings:
  govet:
    enable:
      - nilness
 
  gocritic:
    enabled-checks:
      - sqlQuery

  revive:
    rules:
      - name: indent-error-flow

Here we are configuring one rule per linter

Here are the result

$ GL_DEBUG=gocritic,revive,govet go run cmd/golangci-lint/main.go run -enable-only=gocritic,govet,revive --config example.yml

DEBU [govet] Enabled by config analyzers (33): [appends asmdecl assign atomic bools buildtag cgocall composites copylocks defers directive errorsas framepointer httpresponse ifaceassert lostcancel nilfunc nilness printf shift sigchanyzer slog stdmethods stdversion stringintconv structtag testinggoroutine tests timeformat unmarshal unreachable unsafeptr unusedresult] 

DEBU [gocritic] Final used checks (35): [appendAssign argOrder assignOp badCall badCond captLocal caseOrder codegenComment commentFormatting defaultCaseOrder deprecatedComment dupArg dupBranchBody dupCase dupSubExpr elseif exitAfterDefer flagDeref flagName ifElseChain mapKey newDeref offBy1 regexpMust singleCaseSwitch sloppyLen sloppyTypeAssert sqlQuery switchTrue typeSwitchVar underef unlambda unslice valSwap wrapperFunc] 

DEBU [revive] Default analyzers checks (23): [blank-imports context-as-argument context-keys-type dot-imports empty-block error-naming error-return error-strings errorf exported increment-decrement indent-error-flow package-comments range receiver-naming redefines-builtin-id superfluous-else time-naming unexported-return unreachable-code unused-parameter var-declaration var-naming] 
DEBU [revive] Enabled by config analyzers checks (1): [indent-error-flow]

Here we can see that revive will only enable indent-error-flow, while it could be expected to enable all the default ones, plus indent-error-flow

• govet 33 = 32 + 1
• gocritic 35 = 34 + 1
• revive 1 != 23 + 1

Describe the solution you'd like

the v2 could be a moment to rethink the way revive rules are enabled

Describe alternatives you've considered

Asking to people to do not forget to add all the default revive rules when they simply want to add a one

Additional context

No response

Supporter

• I am a sponsor through GitHub or OpenCollective

[ldez]

This is how revive works, we want to be as close as possible to the standalone linters.