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: Faulty Comment Re-Arrangement in Argument Lists #13113

Open
matttproud opened this issue Oct 30, 2015 · 8 comments
Open

cmd/gofmt: Faulty Comment Re-Arrangement in Argument Lists #13113

matttproud opened this issue Oct 30, 2015 · 8 comments
Assignees
Milestone

Comments

@matttproud
Copy link
Contributor

Hi,

I noticed today that gofmt (on the Playground at least) reformats /* */ comments that are inlined in argument lists inconsistently and in a non-faithful manner to their original ordering in the list:

Pre-Gofmt

fmt.Println(/* before first */ "first", /* before second */ "second")
demo

Post-Gofmt

fmt.Println( /* before first */ "first" /* before second */, "second")
demo

Notice how in the Post-Gofmt case it moves the comment that occurred after the comma to before.

This behavior is incongruent to what happens when the argument lists are spread across multiple lines:

Newline Distributed Pre-Gofmt

package main

import "fmt"

func main() {
    fmt.Println(
    /* before first */ "first")
    fmt.Println(
    /* before first */ "first",
    /* before second */ "second")   
}

demo

Newline Distributed Post-Gofmt

package main

import "fmt"

func main() {
    fmt.Println(
        /* before first */ "first")
    fmt.Println(
        /* before first */ "first",
        /* before second */ "second")
}

demo

For the pre cases, be sure to click Format in the Go Playground demo to see the post-state.

@ianlancetaylor ianlancetaylor changed the title gofmt: Faulty Comment Re-Arrangement in Argument Lists cmd/gofmt: Faulty Comment Re-Arrangement in Argument Lists Oct 30, 2015
@ianlancetaylor
Copy link
Contributor

This is a known issue, but I couldn't find an existing bug report for it.

@dominikh
Copy link
Member

This looks like #9460.

@rhysd
Copy link
Contributor

rhysd commented Jul 2, 2017

I ran into this issue (I'm not sure this is a real issue, tho).

Go does not support named argument. So I wrote parameter names as prefixed comment in function call as below.

SomeFunc(/*name:*/ n, /*debug:*/ true)

However, gofmt formats it as following.

SomeFunc(/*name:*/ n /*debug:*/, true)

The /*debug:*/ comment is placed in the first argument though it's for the second argument.

@nutanixph
Copy link

Is there any update on this? The problem exists in go 1.64.4

@griesemer
Copy link
Contributor

No update. Nobody is working on this.

@ncruces
Copy link
Contributor

ncruces commented Apr 4, 2022

But is this something where external contribution would be welcomed?
I'm interested in having a look.

Convention in some places (including Google) is that literals in function calls should be tagged with a comment like so:
time.Date(y, m, d, h, m, s, /*nsec=*/0, time.UTC)

gofmt tramples all over this.

Also, if the AST does not store the comma's position (seems to be the case for #9460, dunno here), would it be terrible to reverse the decision and put the comment always after the comma? That might generate some large diffs...

@griesemer
Copy link
Contributor

@ncruces gofmt is notoriously difficult to change (mea culpa), and because of all the implications we (the Go team) should make changes here. That said, we don't have any bandwidth for this at the moment. Note that external help is not discouraged, but since any changes will be carefully reviewed in detail, this may not help much.

@n-lim-pyres
Copy link

n-lim-pyres commented Dec 2, 2022

I'm having this problem with SQL requests.

My IDE colorizes and autocomplete SQL requests....
Only if I add a comment that hints the next string being a SQL request.

Example:

err = db.Select(&someSlice, /*language=sql*/ `
    SELECT something
    FROM somewhere
`)

My problem is that gofmt moves the comment to the end of the previous argument, which breaks the syntax highlighting

err = db.Select(&someSlice /*language=sql*/, `
    SELECT something
    FROM somewhere
`)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants