🌟 Let's talk about exclusion options inside issues section

[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.

---

The current configuration:

issues:
  # ...
  
  exclude-generated: strict
  
  # If set to true, `exclude` and `exclude-rules` regular expressions become case-sensitive.
  exclude-case-sensitive: false
  exclude:
    - abcdef
  exclude-rules:
    - path: _test\.go
      linters:
        - gocyclo
        - errcheck
        - dupl
        - gosec
    - path-except: _test\.go
      linters:
        - forbidigo
    - path: internal/hmac/
      text: "weak cryptographic primitive"
      linters:
        - gosec
    - linters:
        - staticcheck
      text: "SA9003:"
    - linters:
        - lll
      source: "^//go:generate "

  exclude-dirs-use-default: false
  exclude-dirs:
    - src/external_libs
    - autogenerated_by_my_lib
  exclude-files:
    - ".*\\.my\\.go$"
    - lib/bad.gO

  exclude-use-default: false
  include:
    - EXC0001
    - EXC0002
    - ...

  # ...

⛑️ The Problems

Useless Option

The option exclude-dirs-use-default comes from the "prehistorical" world of GOPATH.

• vendor$, testdata$ are useless inside this default configuration because the Go tooling already excludes them.
• Since go modules, Godeps$ is useless.
• examples$: This is unexpected because it excludes all directories ending with examples.
• third_party$, builtin$: we should not create "custom conventions", so those elements should not be excluded.

Duplicated Options

The goals of exclude-dirs and exclude-files are the same.

The matching can be complex in some cases:

• Add options to include/exclude paths #4635
• Use path exclude rules on absolute paths instead of relative paths #1178

exclude can be expressed with an exclude-rules.

Unclear Section

At the 1st level of the issues section, there are options about filtering (ex: max-issues-per-linter, new-from-rev), and about exclusions (ex: exclude-rules).

I don't think we should create a subsection for each topic but we should have at least one for exclusions.

Default Exclusions

There is a dedicated proposal for this topic:

• 🌟 Let's talk about default exclusions #5298

💭 Proposal

• All the options related to exclusions should be grouped inside a dedicated subsection of issues:
  • issues.exclude-generated -> issues.exclusions.generated
  • issues.exclude-case-sensitive -> issues.exclusions.case-sensitive: Useful? Remove? Merge with rules? (it just add (?i) prefix) -> by default?
  • issues.exclude-rules -> issues.exclusions.rules
• Removes (deprecates) issues.exclude-use-default. Replaced by issues.exclusions.default.
• Removes (deprecates) exclude-dirs-use-default.
• Removes (deprecates) issues.exclude.
• Removes (deprecates) issues.exclude-use-default (cf proposal about default exclusions)
• Removes (deprecates) issues.exclude-dirs. Replaced by issues.exclusions.paths.
• Removes (deprecates) issues.exclude-files. Replaced by issues.exclusions.paths.

Example

issues:
  # ...
  
  exclusions:
    default: # presets?
      - comments
      - stdErrorHandling
      - commonFalsePositives
    generated: strict # by default?
    case-sensitive: false # Useful? Remove? Merge with rules? (it just add `(?i)`)
    rules:
      - path: _test\.go
        linters:
          - gocyclo
          - errcheck
          - dupl
          - gosec
      - path-except: _test\.go
        linters:
          - forbidigo
      - path: internal/hmac/
        text: "weak cryptographic primitive"
        linters:
          - gosec
      - linters:
          - staticcheck
        text: "SA9003:"
      - linters:
          - lll
        source: "^//go:generate "
    paths:
      - src/external_libs
      - autogenerated_by_my_lib
      - ".*\\.my\\.go$"
      - lib/bad.go

  # ...

[bombsimon]

Related to this topic, can we do something to clarify that this is purely about excluding reports/issues and not analyzing? Don't know from the top of my head what that would be and the setting is clearly already under the issues section but since it's a somewhat common question and misconception I felt I wanted to put it out there on this topic.

[ldez]

Currently, I don't know if the section will be inside issues or inside linters or at 1st level.
Because linters and formatters have different needs regarding exclusions.

[ldez]

After working on the formatters, I think my proposal should be updated to:

linters: # <-- Important
  # ...
  
  exclusions:
    default:
      - comments
      - stdErrorHandling
      - commonFalsePositives
      - legacy
    generated: strict # by default?
    rules:
      - path: _test\.go
        linters:
          - gocyclo
          - errcheck
          - dupl
          - gosec
      - path-except: _test\.go
        linters:
          - forbidigo
      - path: internal/hmac/
        text: "weak cryptographic primitive"
        linters:
          - gosec
      - linters:
          - staticcheck
        text: "SA9003:"
      - linters:
          - lll
        source: "^//go:generate "
    paths:
      - src/external_libs
      - autogenerated_by_my_lib
      - ".*\\.my\\.go$"
      - lib/bad.go

  # ...

Linters (run command) and formatters (fmt command) are too different, so I think we should have a dedicated exclusions section by "type".

linters: 
  # ...
  
  exclusions:
    #...

fommatters:
  # ...
  
  exclusions:
  # ...

[ldez]

Introducing exclusions[].paths is a bit more complex than I expected: #1178 (comment)

[ldez]

After going down the rabbit hole, and spending too much time on it, I think the problem with exclusions[].paths had no simple solution:

• if the option is based on absolute paths some cases work but others not
• if the option is based on relative paths some cases work but others not

This is impossible with a simple option to handle all cases, the 2 sets of constraints are incompatible.

My current thoughts are either to have 2 options and/or have a specific placeholder (like ${configDir} we introduced for ruleguard rules).

The extra hidden problem here is consistency: exclusions[].paths and exclusions[].rules[].path should have the same behavior.