golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.62k stars 17.49k forks source link

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

Open matttproud opened 8 years ago

matttproud commented 8 years ago

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 commented 8 years ago

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

dominikh commented 8 years ago

This looks like #9460.

rhysd commented 7 years ago

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 commented 3 years ago

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

griesemer commented 3 years ago

No update. Nobody is working on this.

ncruces commented 2 years ago

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 commented 2 years ago

@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 commented 1 year ago

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
`)