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

Proposal: Go2: local/private to file #33202

Closed
phrounz opened this issue Jul 20, 2019 · 10 comments
Closed

Proposal: Go2: local/private to file #33202

phrounz opened this issue Jul 20, 2019 · 10 comments
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Milestone

Comments

@phrounz
Copy link

phrounz commented Jul 20, 2019

A nice feature would be to ensure that an function/type/variable/constant is only usable in the current file (in addition of being private to the current package), and raise a compilation error if not.

A fictional (not necessarily good) example: I make a database package, with files like database_init.go, database_query.go and database_fetch.go. Each of these files have several functions and a some of them are configuration values or low-level helpers, like convertFieldToMyObject(f interface{}) in database_fetch.go, or like const connectionEndpoint="localhost:3306" in database_init.go. I don't want all these stuff to be used in any file, this can cause mistakes or at least reduce the file splitting coherence.
This two-levels encapsulation (package encapsulation and file encapsulation) would increase code quality. Without it, a "huge" package can become a mess.

To do this, the word "local" could be used, for example.

local var foo = 2
local func bar() string { return "baz" }

Note that by essence:
local var Foo = 3
would not compile (cannot be both public and local).

Another way to do it would be to have a line separator in a Go file like ===local=== or local: with everything below this line until the end of file would be declared local, and everything above would be not. It's weirder and stricter, but I like it because it would ensure that non-local code remains on top of the file.

@gopherbot gopherbot added this to the Proposal milestone Jul 20, 2019
@DisposaBoy
Copy link

This sounds like the problem that Internal packages solve.

@ianlancetaylor ianlancetaylor added v2 An incompatible library change LanguageChange Suggested changes to the Go language labels Jul 20, 2019
@ianlancetaylor
Copy link
Contributor

I would say that if a huge package is a mess, split it up into multiple packages.

I don't see what we gain by adding another scoping level. When reading code it makes it harder to know which identifier is referred to.

@ianlancetaylor
Copy link
Contributor

There is no strong support for this idea. It makes the language more complicated for minimal gain. Therefore, this is a likely decline. Leaving open for a month for final comments.

@networkimprov
Copy link

When building an application, I believe it's not uncommon for a project to have one package (main) and divide it by subsystem using files. Naturally some identifiers in each file are only for use by that subsystem; they're often flagged with a naming convention, e.g. _privateFunction().

I do this regularly, so I see a rationale for this proposal. Dividing an application into separate packages is an unnecessary burden when none of the packages would be imported elsewhere. Also it may be necessary to use subsystem-local identifiers outside their intended scope for debugging.

But there are alternatives to a language change... We could give go vet a way to recognize file-local identifiers. Would that be worth consideration?

Only a tiny fraction of Go users follow the issue tracker, and perhaps many of them are focused on building open-source libraries, so it's not surprising to me that this issue has "no strong support" -- very few of the folks who might need it have heard about it.

@ianlancetaylor
Copy link
Contributor

"No strong support" in this case means, at present, 1 vote in favor and 17 votes against, expressed as emoji on the initial post. Perhaps a better way to say it would be "overall support vote is negative."

I agree that there is a rationale for this proposal. There is a rationale for every proposal. Nobody would make a proposal with no rationale. The question is whether it is worth the cost.

@networkimprov
Copy link

I offered an alternative and posed a question:

We could give go vet a way to recognize file-local identifiers. Would that be worth consideration?

By that I meant "project-specific file-local identifiers", e.g. a regular expression.

Instead of "rationale", I should have said, "I see a large constituency for this feature, which is probably not well-represented among those who follow the issue tracker."

@ianlancetaylor
Copy link
Contributor

What would it mean to have go vet recognize file-local identifiers? They would still have to be unique within the entire package. What sort of syntax would you suggest?

Also, note that this does not fall under the normal guidelines for go vet: https://golang.org/src/cmd/vet/README . It would be more appropriate for golint. But at that point I think that it would be a better fit for a special purpose tool that could be run by those people who care about it.

@phrounz
Copy link
Author

phrounz commented Aug 28, 2019

Dividing an application into separate packages is an unnecessary burden when none of the packages would be imported elsewhere.

Yes; and the "Internal packages" approach, while being theorically a solution, would require to split the package on many small packages which only one source file in each (like in my fictional database example above).

it would be a better fit for a special purpose tool that could be run by those people who care about it.

Well that's what I did so far, I coded a Perl script which analyses source files and finds out whether stuff in my packages use functions or types of other files of the same package which are below the placeholder line "// LOCAL PRIVATE BELOW".
But there are limits to this approach:

  • the Perl script relies on ugly regular expressions and the like
  • this script must be added in the build process

But yes, adding the feature to go vet or golint would be already useful though.

@networkimprov
Copy link

go vet could take an argument of type regexp; any identifier matching the regexp would be flagged if used outside the file where it's defined.

$ go vet --localid '_.*' # local identifiers begin with _
$ go vet --localid '.*_loc' # end with _loc

Such identifiers would of course be unique across the package; that's not a problem.

@ianlancetaylor
Copy link
Contributor

There were further comments since this was marked as a likely decline (#33202 (comment)) but they do not change our opinion. This should likely be a separate language checking tool for people who want to use it.

-- for @golang/proposal-review

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 Proposal-FinalCommentPeriod v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants