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

Jump to bottom

x/tools/cmd/goimports: lost line between package and import statements in output #26921

Open
sergeleger opened this issue Aug 10, 2018 · 4 comments
Open

x/tools/cmd/goimports: lost line between package and import statements in output #26921

sergeleger opened this issue Aug 10, 2018 · 4 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@sergeleger
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.10.3 linux/amd64

Does this issue reproduce with the latest release?

Yes with go1.10.3 and the following commits:

golang.org/x/tools/imports: 8cb83b71b42ccf5fe279fa8a24a6a8f65507dc9c
golang.org/x/tools/cmd/goimports: 059bec968c61383b574810040ba9410712de36c5

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/legers/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/legers/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build017712154=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Very similar to issue #26290, but no initial import statement.

$ cat x.go
package p // comment

// T does something useful.
func T() {
    var _ = fmt.Printf
}

$ goimports x.go
package p // comment

import "fmt" // T does something useful.
func T() {
	var _ = fmt.Printf
}

A blank line should have been inserted after the new import statement.
@gopherbot gopherbot added this to the Unreleased milestone Aug 10, 2018
@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2018

CC @bradfitz @josharian

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Aug 10, 2018
@fraenkel
Copy link
Contributor

well, with head, you get the opposite

package p // comment
import "fmt"

// T does something useful.
func T() {
	var _ = fmt.Printf
}

@wingyplus
Copy link
Contributor

wingyplus commented Aug 11, 2018 •
edited

I found that it's incorrect because of impDecl.TokPos = c.End() + 2. So I try changed to impDecl.TokPos = c.End() + 1, it returns correct order but not format the source:

package p // comment
import "fmt"

// T does something useful.
func T() {
        var _ = fmt.Printf
}

I think it's a bug in go/format package that it doesn't format import statement. Reproduce source is:

package main

import (
	"fmt"
	"go/format"
	"log"
)

const src = `package main // comment
import "fmt"

// T does something useful.
func T() {
  var _ = fmt.Printf
}
`

func main() {
	out, err := format.Source([]byte(src))
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(string(out))
}

@wingyplus wingyplus mentioned this issue Aug 11, 2018
Closed
@josharian
Copy link
Contributor

The root cause is #22631. However, we'll need to double-check once that has been fixed that goimports does something acceptable when built with and without that fix. In particular, I want to make sure that once the go/printer bug is fixed, it fixes the bug in the context of goimports, and if goimports does need further changes, that those further changes don't break goimports as built without the go/printer fix. Leaving this bug open for that.

@saheienko saheienko mentioned this issue Oct 16, 2018
Closed
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@josharian @wingyplus @fraenkel @bcmills @gopherbot @sergeleger