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

Flag --disable-all does not disable all #1188

Closed
3 tasks done
rhcarvalho opened this issue Jun 16, 2020 · 4 comments
Closed
3 tasks done

Flag --disable-all does not disable all #1188

rhcarvalho opened this issue Jun 16, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@rhcarvalho
Copy link

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint
$ golangci-lint --version
golangci-lint has version v1.27.0 built from (unknown, mod sum: "h1:VYLx63qb+XJsHdZ27PMS2w5JZacN0XG8ffUwe7yQomo=") on (unknown)
Config file
$ cat .golangci.yml
linters:
  disable-all: true
  enable:
    - bodyclose
    - deadcode
    - depguard
    - dogsled
    - dupl
    - errcheck
    - gochecknoglobals
    - gochecknoinits
    - goconst
    - gocritic
    - gocyclo
    - gofmt
    - goimports
    - golint
    - gosec
    - gosimple
    - govet
    - ineffassign
    - interfacer
    - lll
    - maligned
    - misspell
    - nakedret
    - prealloc
    - scopelint
    - staticcheck
    - structcheck
    - typecheck
    - unconvert
    - unparam
    - unused
    - varcheck
run:
  skip-dirs:
    - echo
    - example/echo
issues:
  exclude:
    - "not declared by package utf8"
    - "unicode/utf8/utf8.go"
  exclude-rules:
    - path: _test\.go
      linters:
        - prealloc
    - path: errors_test\.go
      linters:
        - unused
Go environment
$ go version && go env
go version go1.14.3 darwin/amd64
Verbose output of running
$ golangci-lint cache clean
$ golangci-lint run -v
# not relevant

Thanks for golangci-lint!

Maybe this is a user error. I expected to be able to run a single linter at a time by combining --disable-all and -E, however when I run:

$ golangci-lint run --disable-all
doc.go:9: line is 152 characters (lll)
<long line snipped>
internal/testing/f.go:12:20: response body must be closed (bodyclose)
        _, err := http.Get("http://4.6.4.6")
                          ^

Note that at least two linters were run -- lll and bodyclose.

I'm using the latest release v1.27.0 and I have run golangci-lint cache clean, it makes no difference.

If I try to run a single linter, I still get reports from other linters:

$ golangci-lint run --disable-all -E godot
gin/sentrygin.go:84:1: Top level comment should end in a period (godot)
// Check for a broken connection, as this is what Gin does already
^
client.go:36:1: Top level comment should end in a period (godot)
// can be enabled by either using Logger.SetOutput directly or with Debug client option
^
client.go:65:1: Top level comment should end in a period (godot)
// ClientOptions that configures a SDK Client
^
doc.go:9: line is 152 characters (lll)
<snip>
internal/testing/f.go:12:20: response body must be closed (bodyclose)
        _, err := http.Get("http://4.6.4.6")
                          ^

The command line help suggests --disable-all should work as is, specially when --presets runs a certain group of linters and implies --disable-all.

$ golangci-lint run --help | grep disable-all
      --disable-all                    Disable all linters
  -p, --presets strings                Enable presets (bugs|complexity|format|performance|style|unused) of linters. Run 'golangci-lint linters' to see them. This option implies option --disable-all
@rhcarvalho rhcarvalho added the bug Something isn't working label Jun 16, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 16, 2020

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@rhcarvalho
Copy link
Author

I think this was indeed user error.

golangci-lint run --no-config --disable-all

Seems to work okay.

Perhaps there could be a note in the help for --disable-all that explains a config file takes precedence?

Or rather flags should take precedence over config file? Why does golangci-lint run --disable-all flag still enable linters that are listed in the config?!

If this is working as intended, feel free to close the issue.

@bombsimon
Copy link
Member

I was about to close this since it's always been this way but I had a look at the documentation which clearly states

The config file has lower priority than command-line options. If the same bool/string/int option is provided on the command-line and in the config file, the option from command-line will be used. Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and config file.

So I guess we either need to update the documentation or actually ensure that all flags, including --disable-all takes precedence over the configuration file.

@iwankgb
Copy link
Contributor

iwankgb commented Jul 4, 2020

It looks correct to me:

  • --disable-all was passed from command line
  • list of enabled linters was parsed from config file (and it is not possible to overwrite it completely as:

Slice options (e.g. list of enabled/disabled linters) are combined from the command-line and config file.

In my opinion changing existing behavior would not be backward compatible and the only option seems to be to update documentation. Unfortunately I can't see how documentation can be improved as it clearly states how config file and flags are treated.

Therefore - I believe that is issue can be closed (as @rhcarvalho suggests).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants