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

feat: syntax to not override severity from linters #4472

Merged
merged 11 commits into from
Mar 11, 2024

Conversation

ldez
Copy link
Member

@ldez ldez commented Mar 8, 2024

When I created #4452, I thought that only one linter produced issues with severities.

But it's not true as you can see here #4470

The previous option was global: affects all the linters.

The first problem is that revive uses warning and error as severity, but gosec uses low, medium, and high.

I don't want to convert one to another because:

  • 2 severities vs 3 severities
  • some tools allow only the usage of specific severity
  • it's not possible to say what is the best set of values for severities

The second problem is related to the fact that some tools allow only the usage of specific severity.
The current option will keep the severity value from all linters, then the values will be a mix of the different sets.

If you need to use only one set of severities, you need to override/replace the severities from the unsupported set of values.

Then, I reverted the previous PR #4452 (first commit) and replaced it with the usage of a specific wildcard value (@linter) as severity.

The value @ is inspired by some DNS convention, but we can use another character (* is ambiguous).

This value can be used as a default severity or as a rule severity.

One commit, one change.


The following examples illustrate the behavior.

the code sample
package main

import (
	"fmt"
	"os"
	"os/exec"
)

func main() {
	exec.Command("go", "help", os.Args[0])
}

func foo(s string) {
	fmt.Println("test")

}
no custom severities
linters:
  disable-all: true
  enable:
    - gosec
    - revive
    - wsl

linters-settings:
  revive:
    severity: error
    rules:
      - name: unexported-return
        disabled: true
      - name: unused-parameter
        severity: warning
$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "gosec",
  "text": "G204: Subprocess launched with a potential tainted input or cmd arguments",
  "severity": "medium"
}
{
  "linter": "revive",
  "text": "unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _",
  "severity": "warning"
}
{
  "linter": "wsl",
  "text": "block should not end with a whitespace (or comment)",
  "severity": ""
}
`@linter` as rule severity
linters:
  disable-all: true
  enable:
    - gosec
    - revive
    - wsl

linters-settings:
  revive:
    severity: error
    rules:
      - name: unexported-return
        disabled: true
      - name: unused-parameter
        severity: warning

severity:
  default-severity: tomato
  rules:
    - linters:
        - gosec
      severity: '@linter'
$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "gosec",
  "text": "G204: Subprocess launched with a potential tainted input or cmd arguments",
  "severity": "medium"
}
{
  "linter": "revive",
  "text": "unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _",
  "severity": "tomato"
}
{
  "linter": "wsl",
  "text": "block should not end with a whitespace (or comment)",
  "severity": "tomato"
}
`@linter` as default value
linters:
  disable-all: true
  enable:
    - gosec
    - revive
    - wsl

linters-settings:
  revive:
    severity: error
    rules:
      - name: unexported-return
        disabled: true
      - name: unused-parameter
        severity: warning

severity:
  default-severity: '@linter'
  rules:
    - linters:
        - gosec
      severity: carrot
$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "gosec",
  "text": "G204: Subprocess launched with a potential tainted input or cmd arguments",
  "severity": "carrot"
}
{
  "linter": "revive",
  "text": "unused-parameter: parameter 's' seems to be unused, consider removing or renaming it as _",
  "severity": "warning"
}
{
  "linter": "wsl",
  "text": "block should not end with a whitespace (or comment)",
  "severity": ""
}

Fixes #3111

@ldez ldez added enhancement New feature or improvement area: config Related to .golangci.yml and/or cli options area: severity labels Mar 8, 2024
@ldez ldez added this to the next milestone Mar 8, 2024
@ldez ldez force-pushed the feat/severity-override-new branch 2 times, most recently from 22c8c3d to 7a27d49 Compare March 9, 2024 14:31
@ldez
Copy link
Member Author

ldez commented Mar 9, 2024

Maybe @linter instead of @? 🤔

Copy link
Member

@alexandear alexandear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose @inherit instead of @. Or we can use @linter.

pkg/result/processors/severity.go Outdated Show resolved Hide resolved
pkg/result/processors/severity.go Outdated Show resolved Hide resolved
@ldez
Copy link
Member Author

ldez commented Mar 9, 2024

@inherit seems ambiguous because it can be seen as inherited from the default severity.

@ldez ldez force-pushed the feat/severity-override-new branch from c1582ec to 6c033fa Compare March 11, 2024 17:01
@ldez ldez changed the title feat: syntax value to not override severity from linters feat: syntax to not override severity from linters Mar 11, 2024
@ldez ldez merged commit ec52d3c into golangci:master Mar 11, 2024
12 checks passed
@ldez ldez deleted the feat/severity-override-new branch March 11, 2024 17:31
Copy link
Contributor

@Antonboom Antonboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

pkg/result/processors/severity.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: config Related to .golangci.yml and/or cli options area: severity enhancement New feature or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

revive: rules's severity config has no effect
3 participants