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

x/tools/cmd/goimports: rewriting imports leaves out stdlib imports with '/' in them #14075

Closed
ScottMansfield opened this issue Jan 22, 2016 · 3 comments
Milestone

Comments

@ScottMansfield
Copy link

@ScottMansfield ScottMansfield commented Jan 22, 2016

with imports like:

import "bytes"
import "encoding/binary"

and code that adds a dependency on another package, like bufio:

bufio.NewReader(os.Stdin)

goimports will rewrite the imports to look like:

import (
    "bufio"
    "bytes"
)
import "encoding/binary"

The formatting here is odd. I expected it to look like either:

import "bufio"
import "bytes"
import "encoding/binary"

where the established convention in the code of using import before every import is left alone or

import (
    "bufio"
    "bytes"
    "encoding/binary"
)

where the convention of using grouped imports is properly done.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jan 22, 2016

The behavior here is unrelated to the '/' character in "encoding/binary"; e.g., I can repro the issue with "os" instead of "encoding/binary".

goimports is just adding all of the new imports to the first import declaration in the file, and leaving other existing imports separate. Arguably this is working as documented. /cc @bradfitz

@mdempsky mdempsky added this to the Unreleased milestone Jan 22, 2016
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jan 23, 2016

I suppose we could make it have a merging pass afterwards. Somebody could add that to https://godoc.org/golang.org/x/tools/go/ast/astutil

/cc @griesemer

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 24, 2016

CL https://golang.org/cl/19828 mentions this issue.

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

Successfully merging a pull request may close this issue.

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