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/astutil: strange comment placement with AddNamedImport #30724

Open
dsnet opened this issue Mar 11, 2019 · 5 comments
Open

x/tools/astutil: strange comment placement with AddNamedImport #30724

dsnet opened this issue Mar 11, 2019 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Mar 11, 2019

Using 00c44ba9c14f88ffdd4fb5bfae57fe8dd6d6afb1

Consider the following snippet:

package main

import (
	"go/parser"
	"go/printer"
	"go/token"
	"os"

	"golang.org/x/tools/go/ast/astutil"
)

const source = `package comments//

// X comment
var X int
`

func main() {
	fset := token.NewFileSet()
	file, _ := parser.ParseFile(fset, "", source, parser.ParseComments)

	astutil.AddNamedImport(fset, file, "tar", "archive/tar")
	astutil.AddNamedImport(fset, file, "zip", "archive/zip")

	(&printer.Config{Mode: printer.TabIndent | printer.UseSpaces, Tabwidth: 8}).Fprint(os.Stdout, fset, file)
}

This currently outputs:

package comments //

import (
	tar "archive/tar"
	zip "archive/zip"
) // X comment
var X int

Notice how the next comment group X comment is attached to the import block? It seems that this only occurs if an inline comment occurs on the package statement.

I expect this to output:

package comments //

import (
	tar "archive/tar"
	zip "archive/zip"
)

// X comment
var X int
@gopherbot gopherbot added this to the Unreleased milestone Mar 11, 2019
@dsnet dsnet changed the title x/tools/astutil: strange comment placement x/tools/astutil: strange comment placement with AddNamedImport Mar 11, 2019
@dsnet
Copy link
Member Author

dsnet commented Mar 11, 2019

This seems to be a regression introduced by CL/122736 to address #26290. That CL always adds 2 to the end of the last comment position, which I do not think is quite correct.

\cc @Gnouc

@dsnet
Copy link
Member Author

dsnet commented Mar 11, 2019

I think CL/122736 should possibly be reverted and that this issue should be fixed in gofmt. See #30741 as this problem isn't really specific to astutil.AddNamedImport.

@cuonglm
Copy link
Member

cuonglm commented Mar 11, 2019

@dsnet SGTM 👍

@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

@dsnet, do you plan to revert that CL yourself, or should someone on @ianthehat's team investigate this further?

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/170863 mentions this issue: cmd/gopherbot: CC triaged issues to owners

@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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants