staticcheck: flag dubious, almost certainly wrong, shadowing

[dominikh]

@cespare encountered the following shadowing related bug:

var foo *Foo
if {
  foo := ...
  ...
}

f(foo)

This can be summarized as the following pattern: 1) variable declaration 2) followed by a block that shadows the declaration, before ever using it 3) followed by a read of the variable that is guaranteed to read the zero value.

I suspect that this will have few valid false positives. and we could probably put it in the "dubious" category.

[cespare]

@bo-chen pointed out that this can be slightly more general: the outer variable can also be initialized to some non-zero value as long as we read that same value after the inner block.

[bo-chen]

Thanks for the callout.
To be a bit more specific, the lint check can be broadened to:

(a) A variable is defined (assigned to or not) before an inner scope
(b) There is a variable of the same name in the inner scope.
(c) The original variable is used without assignment after that inner scope.

So anything that looks like this has a high risk of bugs and misunderstandings when reading the code:

x := 2;
...
if true {
  x := 3;
  ...
  fmt.Printf("%d\n", x);
} 
...
fmt.Printf("%d\n", x);

As opposed to something like this which would be safe and is how err is normally used:

x := 2;
...
if true {
  x := 3;
  ...
  fmt.Printf("%d\n", x);
} 
...
x := 1;
fmt.Printf("%d\n", x);

[dominikh]

I'll take that generalization into considering when checking the Go corpus, thanks.