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

Incorrect information about gocritic disabled/enabled-checks being mutually exclusive? #4458

Open
5 tasks done
scop opened this issue Mar 5, 2024 · 2 comments
Open
5 tasks done
Labels
area: docs dependencies Relates to an upstream dependency feedback required Requires additional feedback

Comments

@scop
Copy link
Contributor

scop commented Mar 5, 2024

Welcome

Description of the problem

https://golangci-lint.run/usage/linters/#gocritic states that disabled-checks can not be combined with enabled-checks, and vice versa.

However, some tests of mine indicate that combining works just fine when combining also with enabled-tags. I don't know if there are more configurations in which they both work combined. Would be nice to clarify this in the docs.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.56.2 built with go1.22.0 from 58a724a0 on 2024-02-15T18:01:51Z

Configuration

linters:
  disable-all: true
  enable:
    - gocritic
linters-settings:
  gocritic:
    disabled-checks:
      - appendCombine # enabled through performance tag
    enabled-checks:
      - importShadow  # disabled by default
    enabled-tags:
      - performance

Go environment

$ go version && go env
# paste output here

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/scop/Documents/code /home/scop/Documents /home/scop /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 1 linters: [gocritic]     
INFO [loader] Go packages loading at mode 575 (compiled_files|imports|name|types_sizes|deps|exports_file|files) took 71.266967ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 158.85µs 
INFO [linters_context/goanalysis] analyzers took 384.533975ms with top 10 stages: gocritic: 384.529978ms, typecheck: 3.997µs 
INFO [runner] Processors filtering stat (out/in): path_prettifier: 2/2, uniq_by_line: 2/2, diff: 2/2, fixer: 2/2, max_from_linter: 2/2, path_prefixer: 2/2, cgo: 2/2, skip_files: 2/2, identifier_marker: 2/2, exclude: 2/2, path_shortener: 2/2, sort_results: 2/2, skip_dirs: 2/2, autogenerated_exclude: 2/2, nolint: 2/2, source_code: 2/2, severity-rules: 2/2, filename_unadjuster: 2/2, exclude-rules: 2/2, max_per_file_from_linter: 2/2, max_same_issues: 2/2 
INFO [runner] processing took 362.656µs with stages: exclude-rules: 93.321µs, nolint: 76.962µs, identifier_marker: 75.658µs, path_prettifier: 32.732µs, autogenerated_exclude: 28.248µs, source_code: 24.502µs, skip_dirs: 14.919µs, max_same_issues: 6.462µs, path_shortener: 2.047µs, uniq_by_line: 1.98µs, cgo: 1.413µs, max_from_linter: 1.344µs, filename_unadjuster: 743ns, max_per_file_from_linter: 734ns, exclude: 268ns, diff: 256ns, skip_files: 253ns, fixer: 236ns, sort_results: 228ns, severity-rules: 218ns, path_prefixer: 132ns 
INFO [runner] linters took 772.32058ms with stages: goanalysis_metalinter: 771.834462ms 
main.go:15:6: equalFold: consider replacing with strings.EqualFold(fmt, "FMT") (gocritic)
	_ = strings.ToLower(fmt) == strings.ToLower("FMT") // equalFold flagged
	    ^
main.go:9:2: importShadow: shadow of imported package 'fmt' (gocritic)
	fmt := fmt.Sprintf("%s", "foo") // importShadow flagged
	^
INFO File cache stats: 1 entries of total size 273B 
INFO Memory: 10 samples, avg is 58.5MB, max is 79.6MB 
INFO Execution took 851.272866ms                  

A minimal reproducible example or link to a public repository

package main

import (
	"fmt"
	"strings"
)

func main() {
	fmt := fmt.Sprintf("%s", "foo") // importShadow flagged

	var x []string
	x = append(x, fmt) // appendCombine enabled, but not flagged due config
	x = append(x, fmt)

	_ = strings.ToLower(fmt) == strings.ToLower("FMT") // equalFold flagged
}

Validation

  • Yes, I've included all information above (version, config, etc.).
@scop scop added the bug Something isn't working label Mar 5, 2024
@ldez ldez added area: docs dependencies Relates to an upstream dependency question Further information is requested enhancement New feature or improvement and removed bug Something isn't working question Further information is requested enhancement New feature or improvement labels Mar 5, 2024
@ldez
Copy link
Member

ldez commented Mar 5, 2024

Ping @Antonboom

@Antonboom
Copy link
Contributor

hi, @scop

I don't see a contradiction here.

disabled-checks can not be combined with enabled-checks

it means that the next situation disallowed:

  gocritic:
    disabled-checks:
      - appendCombine
    enabled-checks:
      - appendCombine

otherwise the config works the same as the golangci-lint config (well known fact?):

  1. apply enable-all / disable-all
  2. apply enabled-tags (presets)
  3. apply enabled-checks (linters)
  4. apply disabled-tags (presets)
  5. apply disabled-checks (linters)

Feel free to propose better documentation.

@Antonboom Antonboom added feedback required Requires additional feedback and removed question Further information is requested labels Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs dependencies Relates to an upstream dependency feedback required Requires additional feedback
Projects
None yet
Development

No branches or pull requests

3 participants