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: Go 2: remove dot imports from the language #29326

Open
ianlancetaylor opened this issue Dec 18, 2018 · 46 comments

Comments

@ianlancetaylor
Copy link
Contributor

commented Dec 18, 2018

This issue is broken out of #29036.

We should remove dot imports from the language (import . "path"). Quoting @rogpeppe:

The official guidelines suggest that a dot import should only be used "to let the file pretend to be part of package foo even though it is not". This is a stylistic choice and strictly unnecessary. The tests can still use package-qualified names with only minor inconvenience (the same inconvenience that any external user will see).

Other than that, I believe the most common use is to make the imported package feel "close to a DSL". This seems to be actively opposed to the aims of Go, as it makes the programs much harder to read.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

While experimenting with #20104 just a few days ago, dot imports were incredibly useful—instead of having to plumb a bunch of context through complicated code generation scripts and carefully conditionally adjust a dozen format strings, I could just dot import ssa in the relocated files. Long term, I would choose to do the work to eliminate the dot import. But it was very helpful as scaffolding. And not just for experimentation: the dot import is a single line change for reviewers, whereas qualifying every package use would have made the diff unreadable (and unrecognizable to git as a file move).

To be clear (unlike type aliases), this kind of refactoring is strictly speaking possible without dot imports. It is just a lot more graceful with them.

@agnivade

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

Other than that, I believe the most common use is to make the imported package feel "close to a DSL".

Yes, this is what Goa does; which is used by a lot of people. If we remove dot imports, the entire package will break.

I have no issues either way. Just wanted to point this out.

@beoran

This comment has been minimized.

Copy link

commented Dec 19, 2018

DSLs are an important use case for dot imports, I disagree that DSLs make programs harder to read. On the contrary, they make programs easier to read for specific types of program, such as the Goa examples shows, and more importantly also make a program easier to understand for non-programmer domain experts for whom the DSL has been designed. This is why I respectfully ask that this proposal be rejected.

@alanfo

This comment has been minimized.

Copy link

commented Dec 19, 2018

Although dot imports are a feature which should be used sparingly, I can't see the sense in banning them altogether.

There are definitely occasions when they are useful for testing and experimentation and there are some other reasonable uses as well - see for example golang/lint#179.

I often use them myself when importing the math package, so I can just write Cos instead of math.Cos. Admittedly this isn't the greatest of use cases but, for me at least, there is no risk of confusion as I would never use the names of math functions as public names in my own packages.

@deanveloper

This comment has been minimized.

Copy link

commented Dec 19, 2018

As per mentioned in #29036 (comment)

I've found dot-imports to be useful when writing tests. For instance:

package mypack_test

import . "github.com/deanveloper/mypack"

// ...

I wouldn't really mind the dot-import being removed. I was just adding my particular use case for them.

And to reiterate, I wouldn't mind dot-import being removed. It's just what I use them for.

@gotzmann

This comment has been minimized.

Copy link

commented Dec 19, 2018

Most of the popular programming language use some form of "dot import" without any hassles. Hows ftm.Printf is more clear then simple print or echo?

@bcmills

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

Tests are a great argument for removing dot-imports. I've seen lots of tests that failed to catch extremely redundant identifiers, because the tests and examples didn't include the package name as everyone else would.

@bcmills

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

At first glance math seems like a great argument for not removing them, but then consider functions like Jn, Y0, and Log: the former two are somewhat obscure, and the latter could easily collide with a similarly-named function in another package. I could see a reasonable argument for dot-importing trigonometric functions and rounding functions, but math overall seems like a bit of a stretch.

@deanveloper

This comment has been minimized.

Copy link

commented Dec 19, 2018

Hmm now that I think of it, I feel like I've heard a lot of this stuff before. I feel like this might be a duplicate, but I searched for proposals of removing dot imports but didn't find anything

@natefinch

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

Dot imports make your code lie. That's Bad.

When you write AssertEqual(t, got, expected) and your current package doesn't have an AssertEqual function... that code is lying. If you're reading that in a code review (i.e. not in an IDE with hover text etc), you would not be crazy to think AssertEqual is some function in the current package.

Also, if you then copy that code to a different file in the same package... it will fail to compile, and there's nothing goimports can do to figure out where AssertEqual comes from. So you have to Just Know™ where it comes from and add the import yourself. That's bad, too.

I don't believe DSLs of the type made popular in dynamic languages such as python and ruby belong in Go. Those languages specifically give you tools so that you can make non-native functionality look like native functionality. Those languages intentionally let you play fast and loose with types and what the code actually does when you type foo += bar can be vastly different depending on what foo and bar are. Go is not that language. foo += bar will always be simple to understand in Go. There's only a couple things it can possibly mean.

DSLs sacrifice clarity of what the machine is doing for clarity of what the code says. That's not Go's way. You should not need external context to understand where an identifier comes from and what it is calling or doing.

@networkimprov

This comment has been minimized.

Copy link

commented Dec 19, 2018

Maybe the proposal should be "flag dot imports with go vet"?

@ianlancetaylor and others have asserted that Go2 shall not break existing code except where unavoidable for essential new features.

@deanveloper

This comment has been minimized.

Copy link

commented Dec 19, 2018

@ianlancetaylor and others have asserted that Go2 shall not break existing code except where unavoidable for essential new features.

This seems to be his proposal, haha. I think that if there's a consensus that a feature is actively harmful that it should be (at least) considered for removal. IIRC Russ Cox (maybe it was Rob Pike) have talked about removing shadowing if we get the check keyword.

@networkimprov

This comment has been minimized.

Copy link

commented Dec 19, 2018

Assignment redeclaration would also be flagged by Go2 vet :-)

@deanveloper

This comment has been minimized.

Copy link

commented Dec 19, 2018

For now, yes, but what I was trying to say is that a full removal of a feature is something we have already considered (and seem to still be considering, looking at this proposal). Saying that significant members "have asserted that Go2 shall not break existing code [...]" makes it sound like feature removal is something that shouldn't even be listened to, rather than something we'd rather not do.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

commented Dec 19, 2018

@deanveloper I should clarify that although I opened the issue, it's really @rogpeppe 's proposal. I split out of #29036 to clarify that issue.

Personally I'm somewhat against this proposal because I don't think the benefit of clearer code is worth the cost of breaking existing packages. But I'm willing to be convinced otherwise.

@mikeschinkel

This comment has been minimized.

Copy link

commented Dec 30, 2018

I literally just learned about dot imports last week, and have been looking forward to using them in the future, mostly for the handful of random helper functions which can typically be isolated to one package. That way I can write more compact code that uses those helper functions and the code would be easier to reason about for me or anyone who pays attention to the imports.

Idea: If you want to do away with dot imports, consider allowing only one dot import per file?

@rogpeppe

This comment has been minimized.

Copy link
Contributor

commented Dec 30, 2018

For the record, the reason I bundled this issue up with #29036 was that it's necessary to do this if we want to make all imported symbols predictable, which is, I believe a useful property to have globally.

I sympathise with the use of dot imports in @josharian's scenario, but I see it as somewhat of a niche case, which surely could be addressed with a little more additional tooling to make it easier to package-qualify chosen identifiers in generated code. Getting git to recognize diffs of files that move between packages is often a problem anyway. There are many such cases where code is refactored and the diffs are large because the package qualifiers change. I see that as the more general problem here, and dot-imports aren't a good solution in most such cases.

In short, I think that the readability advantages of having properly qualified identifiers everywhere outweigh the occasional extra burden of adding the qualifiers.

@mateuszmmeteo

This comment has been minimized.

Copy link

commented Jan 29, 2019

In my case during image manipulations I'm often using "." import as possibility to load JPEG (image/jpeg) or PNG (image/png) lib.
Without "dot import" this library will be removed and I cannot use jpeg.Encode() method.

How do you want to solve such issue?

@deanveloper

This comment has been minimized.

Copy link

commented Jan 29, 2019

I don't understand your issue, can you provide an example? How are you using dot imports and why can't you just use jpeg.Encode?

@creker

This comment has been minimized.

Copy link

commented Jan 30, 2019

@mateuszmmeteo why do you need dot import for that? Usually you write import _ "image/png" and let image.Decode figure out the rest.

@StephanVerbeeck

This comment has been minimized.

Copy link

commented Apr 23, 2019

I am currently using lxn/walk which has a declarative package for defining forms.
Removing the "." import would brake ALL EXISTING code that uses this package.
And I personally would be very angry having to write "declarative.Button" instead of "Button".
I would have to write the name of the package several thousand times to be exact.
That is not my idea of making code more readable, seems to be more an idea of enforcing restrictions for which I see not a point that they should exist. The original designer who created the dot-import was correct in providing this functionality.

On the other hand, every programmer should also have the common sense to NOT have multiple dot-imports in the same GO-source. Because as long as there is only one dot-import it is PERFECTLY clear as to where the methods and types belong.

I would even go further to request that the intellisense of "Visual Studio Code" be improved to understand this.

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 23, 2019

"I would even go further to request that the intellisense of "Visual Studio Code" be improved to understand this."

Just FYI, GoLand already supports auto-complete on dot imports.

And I personally would be very angry having to write "declarative.Button" instead of "Button".

I agree with this in general with Go. There are a lot of cases when I really wish I could forgo having to prefix with a package name. I know this is a tangent but it would be great if we could define an alias like so:

type Button alias declarative.Button

Another thing that would be great is if Go would assume the same package when other members of the package are passed in to a func call with a simple dot prefix, e.g. instead of this:

forms.ShowDialog("Do you want to close?" forms.Yes, forms.No, forms.Cancel)

It would be great if Go assume the follow was equal to the previous:

forms.ShowDialog("Do you want to close?", .Yes, .No, .Cancel)

I'll be happy to break these two requests out into new tickets assuming I get positive reactions from those on the Go team.

@ianthehat

This comment has been minimized.

Copy link

commented Apr 24, 2019

I agree with this in general with Go. There are a lot of cases when I really wish I could forgo having to prefix with a package name. I know this is a tangent but it would be great if we could define an alias like so:

type Button alias declarative.Button

type Button = declarative.Button

works, but will give you an exported symbol, you can always do

type button = declarative.Button

if you don't want that

See https://golang.org/ref/spec#Alias_declarations

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 24, 2019

@ianthehat Interesting. Clearly I did not know that. Thank you!

(So much still to learn about Go...)

@mateuszmmeteo

This comment has been minimized.

Copy link

commented Apr 24, 2019

I would go with quite opposite (remove it - it will reduce readability. Example below). Let's say that you have multiple packages that implement "Client" struct. That's most cases in my experience.
In code I can use multiple of them. For example:

httpClient,err := http.Client(http.MethodGET, URL, parameters)
...
cacheClient,err := redis.Client(config.RedisConnenctionString)
...
cloudWatchLog,err := logging.Client(config.AWSSdkCredentials)
...
sqlClient,err := mysql.Client(config.SqlDbConnectionString)

We could use builders and define NewHttpClient(params).Build(), NewCacheClient(params).Build()... or we could do it manually in code what would make project code grow.

In code you're seeing what you're touching - don't need to make additional import aliasing. Don't see solution for that. Calling variables and base on that variable name? That would be unwise.

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 24, 2019

@mateuszmmeteo

"I would go with quite opposite"

Even after reading your code it is not clear to me what you are suggesting.

Calling variables and base on that variable name? That would be unwise.

How would it be unwise if the rule is the following?

"Any parameters prefixed with a simple dot (.) would inherit the package of the func being called."

As proposed it is 100% unambiguous.

@mateuszmmeteo

This comment has been minimized.

Copy link

commented Apr 24, 2019

@mikeschinkel

@mateuszmmeteo

"I would go with quite opposite"

Even after reading your code it is not clear to me what you are suggesting.

Calling variables and base on that variable name? That would be unwise.

How would it be unwise if the rule is the following?

"Any parameters prefixed with a simple dot (.) would inherit the package of the func being called."

As proposed it is 100% unambiguous.

In my example you would be required to write:

httpClient,err := .Client(http.MethodGET, URL, parameters)
...
cacheClient,err := .Client(config.RedisConnenctionString)
...
cloudWatchLog,err := .Client(config.AWSSdkCredentials)
...
sqlClient,err := .Client(config.SqlDbConnectionString)

Do I understand you correctly?

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 24, 2019

@mateuszmmeteo

No, that is not what I proposed. My proposal would not be applicable to your example.

Examples that would be applicable, by searching for examples in the Go SDK:

Currently:

f,err = os.Open(os.DevNul)
http.Handle("/tmpfiles/", http.StripPrefix("/tmpfiles/", http.FileServer(http.Dir("/tmp"))))
out = strings.ReplaceAll(strings.ReplaceAll(strings.TrimSuffix(out, "\n"), " ", ""), "\n", " ")

Using proposed:

f,err = os.Open(.DevNul)
http.Handle("/tmpfiles/",.StripPrefix("/tmpfiles/",.FileServer(.Dir("/tmp"))))
out = strings.ReplaceAll(.ReplaceAll(.TrimSuffix(out, "\n"), " ", ""), "\n", " ")

To clarify:

  • os.Open() means you can drop os. from .DevNul
  • http.Handle() means you can drop http. from .StripPrefix()
  • http.StripPrefix() means you can drop http. from .FileServer()
  • http.FileServer() means you can drop http. from .Dir()
  • strings.ReplaceAll() means you can drop strings. from .ReplaceAll()
  • The 2nd strings.ReplaceAll() means you can drop strings. from .TrimSuffix()

Is that clearer?

It would be a small change/help, but it would be nice for those cases where you are using a lot of package prefixed consts and vars as parameters.

@mateuszmmeteo

This comment has been minimized.

Copy link

commented Apr 24, 2019

I'm still thinking about interfaces and multiple implementations that you can use as parameters but yes, looks good.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

@mikeschinkel I think your proposal is pretty different than what @rogpeppe had in mind. If you’d like to pursue it, please open a new issue so we can keep the threads distinct. One question you could answer there is: Given a.F(b.G(.X)), is X from package a or b?

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 24, 2019

@josharian Thank you, I will.

Answering your question: package b

@mikeschinkel

This comment has been minimized.

Copy link

commented Apr 24, 2019

if you don't want that

See https://golang.org/ref/spec#Alias_declarations

To follow up, after trying it, this addresses my envisioned use-case:

type (
    Button = declarative.Button
) 

I post it here in case people in the future are googling for the same thing I wanted because your example did not work for me I almost did not realize there was an alternative.

@jellonek

This comment has been minimized.

Copy link

commented Jul 23, 2019

That would break maaaany test based on gingko/gomega where dot import really makes huge change in readability of tests.
IMO that would be a huge step backward.

@rogpeppe

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

@jellonek Could you point to an example test file that we could experiment with to see what readability difference it makes?

@jellonek

This comment has been minimized.

Copy link

commented Jul 30, 2019

Take a look on https://onsi.github.io/ginkgo/#adding-specs-to-a-suite or on real use in: https://github.com/containernetworking/cni/blob/master/libcni/api_test.go#L129
With dot imports that's close to normal human language description. Adding library name as prefix will simply disfigure this description.
Code like:

	Expect(err).NotTo(HaveOccurred())
	Expect(debugFile.Close()).To(Succeed())

is much more human friendly than:

	gomega.Expect(err).NotTo(gomega.HaveOccurred())
	gomega.Expect(debugFile.Close()).To(gomega.Succeed())
@rogpeppe

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

@jellonek Thanks for that code. I agree that the result won't look so "natural", but there's a trade-off here. Those two packages between them export more than 100 symbols. As an external reader of the code, it is really not clear which of those two packages are being referred to in any given case. For example, is Context something in gingko or gomega?

If either of these packages adds another symbol (as they're allowed to under backward-compatibility rules), it will break your code if that symbol clashes with one of the others.

There are two reasonable alternative approaches here:

  1. shorten the import identifier by choosing a shorter one (for example, K or gk instead of gingko). Here's what the source code might look like using K and M or using gk and gm. I've been using this approach for a while with gocheck and quicktest and you get used to it pretty quickly.
  2. define your own identifiers locally. This means you can write exactly the same code you're writing today, at the expense of a few more lines of definition. Here is an example for the identifiers you're using.
@jellonek

This comment has been minimized.

Copy link

commented Jul 31, 2019

The case for these symbols is that you should not be interested from which package the symbol was imported, as it should be interpreted by the meaning of it's name.

Your first proposal is only trying to slightly polish the situation which is the effect of removing the functionality for which these test seems to me as a perfect use case.

Second - looks much better, but requires copying and copying and copying the same boilerplate to each package containing tests (ok, that can be tool assisted, also the tool can keep the list of used symbols). It's more reasonable, but a bit smelly...

@rogpeppe

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

The case for these symbols is that you should not be interested from which package the symbol was imported, as it should be interpreted by the meaning of it's name.

You could say that about all names in Go. Some languages, such as C, don't even have package namespaces. Go has made a different decision: it has decided that code is more readable when external names are unambiguously qualified with a package identifier.

In this particular case, we've got some very generic names, such as By and And. How is the reader of the code supposed to infer the "meaning" of such general purpose names without going to the package documentation?

You also haven't addressed the problem that would occur if one of those packages added a symbol that's a duplicate of one of the others.

Your first proposal is only trying to slightly polish the situation which is the effect of removing the functionality for which these test seems to me as a perfect use case.

I'm afraid I can't parse this sentence.

Second - looks much better, but requires copying and copying and copying the same boilerplate to each package containing tests (ok, that can be tool assisted, also the tool can keep the list of used symbols). It's more reasonable, but a bit smelly...

Less smelly than importing an arbitrary set of remote symbols into the local namespace IMHO. I understand your use case, but I don't think it's worth the language compromise.

@jellonek

This comment has been minimized.

Copy link

commented Jul 31, 2019

Case with name duplication was too virtual to be responded, but ok - these packages are carefully maintained and developed together, so such clash would be found/eliminated on their level, before this code could reach my code.
But lets look on that from wider perspective (it can be hard, as English is not my native language and looks from above example that like we can have an issue with that) - ginkgo/gomega are only examples of using dot imports to provide own kind of DSL, where the domain is tests. Same approach could be used (idk if it is already) in other specific domains.

So the question is - do we want to make golang more strict, more pure, or do we want to keep elasticity, having in mind that elasticity always at the end will be abused. What gives us more benefits.

Btw. your point about being non sure to which package the particular symbol is associated is valid only when one is reading the code in some kind of dumb viewer, which does not have functionality of "go to definition". Especially when it's done without understanding the environment in which such code exists (in this case - without being familiar with used DSL).

So again - we can make golang more strict, more sharp, but we should be aware that we are entering the path already faded by python maintainers, which "improved" the language ignoring that there was an user base which were (and are still) using the language in a way which do not follows maintainers "vision".

@beoran

This comment has been minimized.

Copy link

commented Jul 31, 2019

As it is now, I see a lot of live (closed source) production code where, in conjunction with the go Chi router, a single Unicode identifier ' χ' is used for convenience. I find this less readable than a dot import. DSL's are meant to be accessible, often also for non-programmers, for whom the prefix means nothing and is a visual hinderance. For this particular use case, I think dot imports are essential and convenient.

@StephanVerbeeck

This comment has been minimized.

Copy link

commented Jul 31, 2019

Look here, As a professional user I forbid this change !!!
Stop changing syntax every day based on idealistic theoretical ideas that make no sense.
GOlang has already been poisoned enough with those thousands of new compilation warnings of the last year !!!!
To remove existing functionality will make professional use of the GO language impossible.
Companies like mine do not like the idea that even the base functionality of GO language is constantly in flux and done and finished modifications are again and again undone or changed by other developers later that have a different opinion.
Seriously I have a thousand compilation warnings on a rather small project right now at this moment and I am not going to fix any of them as we as a company decide on our coding-style-guide and this is not up to external unknowns to dictate all these rules for our developers. Thank you for understanding this simple fact of life.
If you feel offended by this then at least realize that your personal emotions are least relevant of all.
Development is pure business and not play and fun or hobby thinkering.

@jellonek

This comment has been minimized.

Copy link

commented Jul 31, 2019

Please don't use emotional tone. That does not help in discussion.

@cespare

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2019

I see the forbiddance of "DSL" code based on dot imports as a feature of this proposal rather than a downside. And the upside from #29036 seems significant.

@agnivade

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

What is the recommendation for users of DSL libraries like avo and Goa ? Shortening the import identifier does not feel elegant and re-defining identifiers is not scalable because depending on the code, the user needs to add/remove new identifiers.

A huge (guessing from the popularity of these libraries) amount of code will break if we accept this proposal. Seems like a significant cost to pay given the benefit it gives.

@balasanjay

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

Defining local short names for remote names works reasonably well for C++ code. I found a random example from an Abseil test here.

It also works reasonably well in JS's shiny new module system (and similarly in TS). I found a random example from Angular's codebase here.

To be clear, I'm not endorsing everything about these examples, or saying that Go should switch to using this model (I actually think using a single short identifier for external packages is actually a great trade-off). But as a replacement for dot-imports, it seems clear to me that having local short aliases for long names can work in practice.

I think the major downside for this solution would be that it "reexports" these new identifiers (assuming you capital case them). So it is fine in tests, but suboptimal in non-test code.

@jellonek

This comment has been minimized.

Copy link

commented Aug 1, 2019

@agnivade imo the most reliable would be to add a boilerplate file to each package which will define local aliases for all used identifiers (maybe all from library) in similar way as @rogpeppe proposed in this example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.