Shadowed Variable Checking is Broken with Default Settings

[Zamiell]

Thank you for creating the issue!

• 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 1.24.0 built from 6fd4383 on 2020-03-15T11:38:01Z

Config file

$ cat .golangci.yml
# none; using the default

Go environment

$ go version && go env
go version go1.14 windows/amd64                                                                                                                                              set GO111MODULE=                                                                                                                                                             set GOARCH=amd64                                                                                                                                                             set GOBIN=                                                                                                                                                                   set GOCACHE=C:\Users\james\AppData\Local\go-build                                                                                                                            set GOENV=C:\Users\james\AppData\Roaming\go\env                                                                                                                              set GOEXE=.exe                                                                                                                                                               set GOFLAGS=                                                                                                                                                                 set GOHOSTARCH=amd64                                                                                                                                                         set GOHOSTOS=windows                                                                                                                                                         set GOINSECURE=                                                                                                                                                              set GONOPROXY=                                                                                                                                                               set GONOSUMDB=                                                                                                                                                               set GOOS=windows                                                                                                                                                             set GOPATH=C:\Users\james\go                                                                                                                                                 set GOPRIVATE=                                                                                                                                                               set GOPROXY=https://proxy.golang.org,direct                                                                                                                                  set GOROOT=c:\go                                                                                                                                                             set GOSUMDB=sum.golang.org                                                                                                                                                   set GOTMPDIR=                                                                                                                                                                set GOTOOLDIR=c:\go\pkg\tool\windows_amd64                                                                                                                                   set GCCGO=gccgo                                                                                                                                                              set AR=ar                                                                                                                                                                    set CC=gcc                                                                                                                                                                   set CXX=g++                                                                                                                                                                  set CGO_ENABLED=1                                                                                                                                                            set GOMOD=                                                                                                                                                                   set CGO_CFLAGS=-g -O2                                                                                                                                                        set CGO_CPPFLAGS=                                                                                                                                                            set CGO_CXXFLAGS=-g -O2                                                                                                                                                      set CGO_FFLAGS=-g -O2                                                                                                                                                        set CGO_LDFLAGS=-g -O2                                                                                                                                                       set PKG_CONFIG=pkg-config                                                                                                                                                    set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\james\AppData\Local\Temp\go-build017672494=/tmp/go-build -gno-record-gcc-switches
--

Verbose output of running

$ golangci-lint run -v
level=info msg="[config_reader] Config search paths: [./ C:\\Users\\james\\test C:\\Users\\james C:\\Users C:\\]"                                                            level=info msg="[lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]"                             level=info msg="[lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]"                             level=info msg="[loader] Go packages loading at mode 575 (compiled_files\|deps\|exports_file\|imports\|name\|files\|types_sizes) took 280.3909ms"                                  level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 0s"                                                                                                  level=info msg="[runner/goanalysis_metalinter/goanalysis] analyzers took 0s with no stages"                                                                                  level=info msg="[runner/unused/goanalysis] analyzers took 0s with no stages"                                                                                                 level=info msg="[runner] processing took 0s with stages: autogenerated_exclude: 0s, max_per_file_from_linter: 0s, skip_files: 0s, exclude: 0s, path_shortener: 0s, diff: 0s,  max_same_issues: 0s, max_from_linter: 0s, cgo: 0s, filename_unadjuster: 0s, identifier_marker: 0s, uniq_by_line: 0s, source_code: 0s, path_prettifier: 0s, skip_dirs: 0s, e xclude-rules: 0s, nolint: 0s"                                                                                                                                                level=info msg="[runner] linters took 26.9292ms with stages: goanalysis_metalinter: 25.9318ms, unused: 997.4µs"                                                              level=info msg="File cache stats: 0 entries of total size 0B"                                                                                                                level=info msg="Memory: 5 samples, avg is 22.2MB, max is 25.7MB"                                                                                                             level=info msg="Execution took 351.1888ms"
--

Description of the Problem

I'm using the following "main.go" file:

package main

import "fmt"

func main() {
	a := 1
	fmt.Println(a)
	if true {
		a := 2
		fmt.Println(a)
	}
}

Question 1 - Obviously, I am shadowing a variable in the above code snippet. If I am not mistaken, this should be caught by govet. So something seems broken, because according to the documentation in the README.md file, the "vetshadow" linter is supposed to be enabled by default. What gives?

Question 2 - Poking around in the default options, I noticed that it has the following specification for govet:

  govet:
    # report about shadowed variables
    check-shadowing: true

    # settings per analyzer
    settings:
      printf: # analyzer name, run `go tool vet help` to see all analyzers
        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf

    # enable or disable analyzers by name
    enable:
      - atomicalign
    enable-all: false
    disable:
      - shadow
    disable-all: false

Can someone explain why does it simultaneously says check-shadowing: true and disable: -shadow? Is that a bug? If it is not a bug, can we add a comment or something and explain what the heck is going on there?

[Zamiell]

Specifically, it looks like this line is lying to the userbase:
https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml#L153
Because the default value appears to be false.

[torbjornvatn]

I have the same issue, and it doesn't work to enable it explicitly in my own .golangci.yml file either.

But from what I can see @Zamiell the -shadow flag has been removed from go vet and moved into a separate tool that has to be installed. golang/go#29260 (comment)
So I guess golangci-lint has to install that tool to make this work

[ldez]

Can someone explain why does it simultaneously says check-shadowing: true and disable: -shadow? Is that a bug? If it is not a bug, can we add a comment or something and explain what the heck is going on there?

@Zamiel The file https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml does not contain the default options, it's just an example of all options, so there is no meaning for this configuration.

---

@torbjornvatn the shadow linter is already supported: #697

---

The linter seems to work as expected:

no problem

package main

import "fmt"

func main() {
	a := 1
	fmt.Println(a)
	if true {
		a := 2
		fmt.Println(a)
	}
}

problem

package main

import "fmt"

func main() {
	a := 1
	if true {
		a := 2
		fmt.Println(a)
	}
	fmt.Println(a)
}

cmd/foo.go:8:3: shadow: declaration of "a" shadows declaration at line 6 (govet)
                a := 2
                ^
exit status 1

[butuzov]

@ldez I believe it would be beneficial to remove "check-shadowing:" setting as misleading for newcomers.

I just run into it, while checking why with enabled check-shadowing we have this issue in code. I can pick it (just in case).

found my logic error., I was expecting next code to trigger shadowing warning.

package testdataabstract

import (
	"context"
	ctxpkg "context"
	"fmt"
)

func ReadFromContext(context context.Context) context.Context {
	return ctxpkg.WithValue(context, "key", "val")
}

[Zamiell]

@ldez

1. Why was this closed? The issue does not seem to be resolved.
2. More specifically, can you answer: is this line a bug or not?
   https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml#L270
3. If it is not a bug, then why does enabling it not do anything?
4. Why isn't shadow variable checking enabled by default?

[ldez]

I was thinking that I already answered your questions #1008 (comment)

1. Why was this closed? The issue does not seem to be resolved.

because there is no bug, the behavior is expected.

2. More specifically, can you answer: is this line a bug or not?
   https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml#L270

It's not a bug, .golangci.example.yml does not contain the default options, it's just an example of all options, so there is no meaning for this configuration.

3. If it is not a bug, then why does enabling it not do anything?

because in your example there is no shadowing.

no shadow

package main

import "fmt"

func main() {
	a := 1
	fmt.Println(a)
	if true {
		a := 2
		fmt.Println(a)
	}
}

shadow

package main

import "fmt"

func main() {
	a := 1
	if true {
		a := 2
		fmt.Println(a)
	}
	fmt.Println(a)
}

cmd/foo.go:8:3: shadow: declaration of "a" shadows declaration at line 6 (govet)
                a := 2
                ^
exit status 1

4. Why isn't shadow variable checking enabled by default?

Because it's not enabled by default in govet.

[Zamiell]

Ah, ok, I see. Thanks for the quick reply Idez.