What version of Go are you using (go version)?

$ go version
Go 1.14.4

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/leighmcculloch/local/bin"
GOCACHE="/home/leighmcculloch/.cache/go-build"
GOENV="/home/leighmcculloch/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/leighmcculloch/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/leighmcculloch/local/bin/go/1.14.4"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/leighmcculloch/local/bin/go/1.14.4/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build157507746=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Fixed code according to the shadow analyzer, I renamed a variable from err to be distinguishable from another variable also named err that was in a higher scope.

What did you expect to see?

I expected using unique names for variables in nested scopes to be a best practice since the shadow analyzer recommends it.

What did you see instead?

I'm experiencing that renaming err variables in code is more likely to create more human errors than to use shadowed errvariables.

In Go code my colleagues and I have seen we have definitely found value in renaming shadowed variables, it's actually caught some bugs for us on a couple occassions, but when it comes to errors the opposite has been true. On one occasion the error lived on for a while in production code. Code had been written, a shadow was detected, a variable was renamed, the scope was removed and then this was the bug and fix when we discovered the bug that had been created by the attempt to not shadow: stellar/go#2469. I've caught myself on another occasion making the same mistake. This might just be my weakness but I suspect others might make the same mistake as me.

This issue isn't about that specific issue but rather treating a shadowed err can easily create buggy code because developers may be accustomed to seeing the error named err, making it really easy to forget that in this one rare scope the error is named dbErr and to do things like return the error from the wrong scope.

Proposal

Make err of type error an exception to the shadow checker. The shadow checker already makes several exceptions on the basis of those patterns being common and idiomatic. This is another one of those patterns.

cc @stamblerre