proposal: Go 2: require explicit variable shadowing

[networkimprov]

Branched from #377.

Problem

Unintended shadows are easy to create with := and cause nearly invisible bugs.

Go takes one of eight possible paths for a, b := f(), as each var is defined, assigned, or shadowed. One cannot tell which without searching prior code, so it's easy to write something that behaves incorrectly.

Proposal

Benefits Provided

a) eliminate bugs due to unintended := shadows
b) reduce the complexity of := statements (a, b := f() would have three paths)
c) no learning curve for the go vet changes
d) backwards compatibility

Changes to go vet and var:

1. Let go vet flag implicit shadows, s := t.

2. Let var s T or var s = t in the new scope silence go vet for intended shadows.

   x, err := Fa()
   if err != nil {
      var err error     // explicit shadow; not flagged by go vet
      y, z, err := Fb()
      ...
      x := 1            // implicit shadow: flagged by vet
   }
   

3. Let var name allow redeclaration of the name within its scope, without creating a shadow. Python also permits this. (Not essential to (1) & (2), so could be omitted.)

   x, err := Fa()
   if err == nil {      // OR: if var err; err == nil 
      var err           // override shadowing in if-scope
      y, z, err := Fb() // no need for separate declarations of y & z
      ...
   }
   if err != nil { ... }
   

[beoran]

So, If I understand correctly, what you propose is only a change to go vet, then?

[deanveloper]

The title and contents of this proposal seem to be conflicting. The title says to require declaration using var name in order to take advantage of variable shadowing. The LanguageChange label also seems to suggest this. But the contents seem to suggest that this is a backwards-compatible change that is only enforced by go vet.

Either way, this is a good change and has gotten a +1 from me.

[networkimprov]

@beoran @deanveloper: see item 3.

[IngCr3at1on]

Call me crazy but neither of those examples seem idiomatic to me.

The following is readable even if you aren't insanely familiar with Go; while your implementation requires either always adding those comments or implying an extra familiarity of (in my opinion counter-intuitive) functionality in the var built-in.

var (
    y <type>
    z <type>
)
x, err := Fa()
if err == nil {
    y, z, err = Fb()
}
if err != nil { ... }

It reminds me of ruby variable scoping, which I personally have never been fond of.
You could complain that my version is more verbose but it's that verbosity that makes it more obvious what it does; simplicity is good 😄

[networkimprov]

@IngCr3at1on, your example doesn't reproduce mine. It should be:

x, err := Fa()
if err == nil {
   var (
      y <type>
      z <type>
   )
    y, z, err = Fb()
}
if err != nil { ... }

[IngCr3at1on]

@networkimprov funny enough, that's what I had but then I changed it because your example doesn't use y and z and therefore will result in a compile error (assuming y and z were to be handled after the second err check); my mistake in reading your code (but I feel this also reinforces my point does it not?).

[IngCr3at1on]

Using the example from the OP to expand on the point I was trying to make.

x, err := Fa()
if err == nil {      // OR: if var err; err == nil 
   var err           // override shadowing in if-scope
   y, z, err := Fb() // no need for separate declarations of y & z
   if y && z { ... } // Some random operations on y and z so that we're using these values...
}
// Now we check the error reassigned when Fb ran _after_ performing operations on y and z.
if err != nil { ... }

I'd like to understand what instances you would write something like that myself because again it seems particularly unsafe, allowing the error to be shadowed without at least a warning from vet seems very dangerous here.

I'd almost argue the better solution would be to explicitly disallow shadowing at the compiler, resulting in a build error.

[networkimprov]

allowing the error to be shadowed without at least a warning from vet seems very dangerous here

Per the first sentence of the text, "let var name prevent shadows of the name within its scope."

EDIT: there is no shadowing in the above example.

[IngCr3at1on]

@networkimprov I fail to see how quoting a sentence from your OP addresses my concerns... I'd like to understand (as a Go user and a software programmer) why such a change is actually wanted because so far as I can tell it does not improve on anything?

For my perspective it only makes the functionality of the var built in less obvious and therefore the language less idiomatic.

[DylanMeeus]

Whilst I am not convinced the proposed solution is good, because it's not 'unambigious behaviour' of the var keyword. Or in any case, it makes it less obvious as @IngCr3at1on has mentioned.

However, I do think that requiring variable shadowing to be an explicit operation is a good idea. But I'd propose a change that won't break existing codebases that use shadowing consciously by introducing a new keyword.

i := 10
for {
     shadow i := 9
     i -= 1
     fmt.Printf("%v\n", i)
     break
}
fmt.Printf("%v\n",i)

// output:
8
10

This is not actually a real proposal, but I just want to raise the idea of making shadowing more explicit. Perhaps I could turn that into a more thought out proposal :)