Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flag using a package name as a variable name #27

Closed
thejerf opened this issue Nov 25, 2013 · 4 comments
Closed

Flag using a package name as a variable name #27

thejerf opened this issue Nov 25, 2013 · 4 comments

Comments

@thejerf
Copy link

@thejerf thejerf commented Nov 25, 2013

I've now been bitten by this about four times in the last couple of weeks; importing a package "package/noun", and then ending up with code that looks something like:

noun := GetCurrentNoun()

if noun == nil {
    noun = noun.DefaultNoun()
}

which fails with a semi-bizarre error message about how DefaultNoun is undefined because type noun.Noun has no field or method DefaultNoun. Careful reading of the error message shows it is correct, but it is surprising.

@dsymonds

This comment has been minimized.

Copy link
Member

@dsymonds dsymonds commented Nov 25, 2013

Golint is for style checking, and this isn't so much a style problem as a correctness problem. I think this belongs in govet (if anywhere); you could file a feature request for that at http://golang.org/issue/new.

@dsymonds dsymonds closed this Nov 25, 2013
@thejerf

This comment has been minimized.

Copy link
Author

@thejerf thejerf commented Nov 26, 2013

Are you aware that it is perfectly legal Go to shadow a package with a variable? It is a style matter. It "works" perfectly, and it is not a violation of the standard. It's just a bad idea. The error message I show you is what happens when you get bitten by the bad idea, not what happens when you merely shadow the variable.

If you are, then OK, cool. Just checking.

@dsymonds

This comment has been minimized.

Copy link
Member

@dsymonds dsymonds commented Nov 26, 2013

I know it's legal Go, but so is fmt.Printf("missing arg for %d"). Govet is
for things that are legal Go (i.e. stuff that compiles and runs, but is
incorrect); golint is for things that are correct, but not consistent with
the standard Go coding style.

Perhaps this falls in between somewhat, though I'll note that govet already
checks for some forms of shadowing, so it seems like it belongs on that
side of the fence.

@thejerf

This comment has been minimized.

Copy link
Author

@thejerf thejerf commented Nov 26, 2013

Righty-o, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.