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

cmd/gofmt: -r 'bar(x) -> bar(x)' mistakenly replaces a comment with a newline in closures inside function #23275

Open
qfel opened this issue Dec 28, 2017 · 4 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@qfel
Copy link

qfel commented Dec 28, 2017

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

go version go1.9.2 linux/amd64

What did you do?

$ gofmt -r 'bar(x) -> bar(x)' /tmp/test.go

/tmp/test.go is:

package main

func main() {
    // Call foo().
    foo()
    bar(func() {
        // Call foo().
        foo()
    })
}

What did you expect to see?

Same contents

What did you see instead?

package main

func main() {
        // Call foo().
        foo()
        bar(func() {

                foo()
        })

}

Notice the comment in the closure passed to bar was replaced with an empty line, and an empty line was added after the call to bar.

The identity rewrite rule is for simplicity, this happens with more useful rewrites that act on bar too.

@odeke-em odeke-em changed the title gofmt -r messes comments in closures inside function calls cmd/gofmt: -r 'bar(x) -> bar(x)' mistakenly replaces a comment with a newline in closures inside function Dec 28, 2017
@odeke-em
Copy link
Member

/cc @griesemer

@m4ns0ur
Copy link
Contributor

m4ns0ur commented Dec 29, 2017

The "recreate comments list" is filtering the comment inside the closure

@griesemer
Copy link
Contributor

@m4ns0ur Thanks for investigating. In this example, while bar(x) is replaced with the same looking expression, it's nevertheless a different, new expression, and as a result (I suspect) the comment that was associated with the old bar(x) is gone. If my suspicion is true, this is working as intended (even though not as expected).

I don't know why there's a new empty line before the closing }, though.

gofmt -r is somewhat experimental, unfortunately.

@griesemer griesemer added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 3, 2018
@griesemer griesemer added this to the Unreleased milestone Jan 3, 2018
@griesemer griesemer self-assigned this Jan 3, 2018
@dominikh
Copy link
Member

dominikh commented Jan 3, 2018

Related: #7417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants