go: cmd/compile: add hint to type errors when builtin identifiers are shadowed

Please answer these questions before submitting your issue. Thanks!

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

1.9.2

Does this issue reproduce with the latest release?

Yes

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

darwin/amd64

What did you do?

The following code containing an assignment to len fails to compile, but go vet doesn’t warn about why: https://play.golang.org/p/ap8LOPyn6M

package main

import (
	"fmt"
)

func main() {
	arr := []int{0, 1}
	len := 5
	fmt.Println(fmt.Sprintf("What is life? %d", len(arr)))
}

What did you expect to see?

A warning about assignment to the len variable. (Go also allows the user to assign to other built-in-but-not-reserved names, such as cap and append.)

What did you see instead?

No warnings.

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 18 (14 by maintainers)

Most upvoted comments

@otterley shadowing builtin functions isn’t particularly encouraged, but it can be useful sometimes. Similarly, it is sometimes useful to name a function parameter url, clashing with the net/url package name.

Perhaps the right solution here would be to improve the compile error messages, by adding something like did you mean to shadow the builtin 'len' function?. That is usually the right answer to “it took me five minutes to figure out the problem behind a compile error”.

mvdan on Nov 21, 2017

@mvdan Let’s leave it as is for now. Also, go/types already reports not just the type but also what it is (variable, constant, etc.) which makes it clearer.

griesemer on Feb 27, 2018

I have rewritten the issue title. I don’t think a full-blown proposal is necessary for this, but I would like some input from others like @griesemer and @mdempsky.

I have to say that in my early days of Go development I did shadow builtin names like len, new, and copy sometimes. Now it’s in my muscle memory not to do so, but these hints would have certainly helped me back then.

mvdan on Nov 21, 2017

These built-in functions are identifiers, not keywords, for good reason. One of the reasons is to allow adding new built-ins in the future without breaking existing code. The other is to allow users to shadow them at their discretion. In neither case should vet warn about perfectly fine code.

it’s only a compile error if len etc. are subsequently used in their idiomatic forms as documented. Otherwise, no warning will be issued.

If they’re being used incorrectly, it will cause a compile time error. If they are being used correctly, the user intended to use them that way. Neither case warrants a vet check IMHO.

dominikh on Nov 21, 2017