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

fix: store and read severity from linters in the cache #4468

Merged
merged 1 commit into from
Mar 9, 2024

Conversation

ldez
Copy link
Member

@ldez ldez commented Mar 8, 2024

The severity from linters needs to be stored into the cache, if not, the severity between 2 calls is not consistent.

To test the PR
#!/bin/sh -e

## setup module
rm -rf testexample
mkdir testexample && cd $_
go mod init helloworld

## main.go
cat > main.go  <<EOF
// this is a very very very very very very very very very very very very very very very very very very very very very very long line.
package main

func main() {
	newDnsZone()
}

func newDnsZone() error {
	return nil
}
EOF

## .golangci.yml
cat > .golangci.yml  <<EOF
linters:
  disable-all: true
  enable:
    - revive
    - errcheck
    - lll

linters-settings:
  revive:
    ignore-generated-header: false
    severity: error
    confidence: 0.8
    rules:
      - name: var-naming
        severity: warning

severity:
  default-severity: error
  keep-linter-severity: true
  case-sensitive: true
  rules:
    - linters:
        - lll
      severity: warning
EOF

Before (i.e. v1.56.2)
$ golangci-lint cache clean

$ golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }' 
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "error"
}

$ golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "error"
}

The severity from linters is ignored.

After #4452
$ golangci-lint cache clean

$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }' 
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "warning"
}

$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "error"
}

The severity from linters is applied but the severity from var-naming has changed between the 2 calls because the severity is not inside the cache.

With this PR
$ golangci-lint cache clean

$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "warning"
}

$ ./golangci-lint run --out-format json | jq  '.Issues[] | {linter: .FromLinter, text: .Text, severity: .Severity }'
{
  "linter": "errcheck",
  "text": "Error return value is not checked",
  "severity": "error"
}
{
  "linter": "lll",
  "text": "line is 133 characters",
  "severity": "warning"
}
{
  "linter": "revive",
  "text": "var-naming: func newDnsZone should be newDNSZone",
  "severity": "warning"
}

The severity stays consistent.

@ldez ldez added bug Something isn't working area: cache labels Mar 8, 2024
@ldez ldez added this to the next milestone Mar 8, 2024
@ldez
Copy link
Member Author

ldez commented Mar 8, 2024

I cleaned to PR because I committed unrelated things by mistake.

@ldez ldez changed the title fix: store severity from linters in the cache fix: store and read severity from linters in the cache Mar 9, 2024
@ldez ldez merged commit 7e2840b into golangci:master Mar 9, 2024
12 checks passed
@ldez ldez deleted the fix/severity-cache branch March 9, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants