Skip to content

proposal: Go 2: allow (require?) package selector on identifiers from the current package #29342

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

Closed
natefinch opened this issue Dec 19, 2018 · 7 comments
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Milestone

Comments

@natefinch
Copy link
Contributor

natefinch commented Dec 19, 2018

reading #29326, the arguments about using the package specifier in its tests made sense, and made me rethink using the package selector for identifiers in the current package.

Currently, code in the io package is only place in the entire Go universe you'll ever see Reader refer to io.Reader.

func LimitReader(r Reader, n int64) Reader

This is inconsistent, and can be confusing. It also means that if you move/copy this function to a different package, you have to change the signature to this:

func LimitReader(r io.Reader, n int64) io.Reader

Isn't this ^^ more readable and understandable in the first place? Why can't this be the signature in the io package too? Everyone else using LimitReader is going to be calling its argument and return value io.Reader.. but the docs and the code of LimitReader elide the io and I believe this is unfortunate.

I would prefer if Go would at least allow (maybe require? Would take more thought) for the code and the docs to use the package identifier for the current package. That way the package's own code would be consistent with any of the code that uses that package - i.e. using the package selector for all identifiers.

@gopherbot gopherbot added this to the Proposal milestone Dec 19, 2018
@deanveloper
Copy link

An alternative would be to leave it the way it is for now, and just have the documentation put the package name in front.

@natefinch
Copy link
Contributor Author

Yeah, I do like that – sorta – except then the docs don't match the code, which is unfortunate. That would be a very minimal change that wouldn't modify the language, so a pretty low bar. But I'm not sure it would be worth it to have the code different from the docs.

@natefinch
Copy link
Contributor Author

One potential complication for this proposal... when a package imports another package with the same package name as itself.

like github.com/pkg/errors importing stdlib errors .... you'd have to rename the import. However, I think would definitely be more clear than current code.

Wait, so this package calls errors.New, but that's not the same as the errors.New that I call when I import it? Now I'm confused...

@ianlancetaylor ianlancetaylor added LanguageChange Suggested changes to the Go language v2 An incompatible library change labels Dec 19, 2018
@cespare
Copy link
Contributor

cespare commented Dec 19, 2018

A downside of this would be that you would need to change the code when you rename a package.

@natefinch
Copy link
Contributor Author

True, but ... do people do that very often? If I have, it's not very often. And, you'd already have to change the code for everything that uses the code in that package anyway, so it's just one more folder to update.

This is also something eminently doable with tooling.

@deanveloper
Copy link

ideally all we would need to do is update x/tools/cmd/gomvpkg

@ianlancetaylor
Copy link
Contributor

Requiring the package name is a non-starter, as that would break all existing Go code.

(As a technicality, the package name is actually not in any namespace and is not accessible, though that could be changed.)

Permitting an optional package name doesn't seem useful. It makes locating the source of an identifier slightly more ambiguous for the reader of the code. It doesn't solve a problem that we have today.

@golang golang locked and limited conversation to collaborators Jan 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants