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

go/printer: "Line of stars" detection false positive for commented out block of code with each line dereferencing a pointer. #12164

Open
dmitshur opened this Issue Aug 17, 2015 · 2 comments

Comments

Projects
None yet
3 participants
@dmitshur
Copy link
Member

dmitshur commented Aug 17, 2015

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

go version go1.4.2 darwin/amd64

What operating system and processor architecture are you using?

OS X 10.10.5, amd64.

What did you do?

Consider the following program:

package main

import "fmt"

func main() {
    var i [3]int
    p0 := &i[0]
    p1 := &i[1]
    p2 := &i[2]

    *p0 = 5
    *p1 = 6
    *p2 = 7

    fmt.Println("Hello, playground:", *p0, *p1, *p2)
}

If you comment out the first block of code and run gofmt on it, you get:

package main

import "fmt"

func main() {
    /*
        var i [3]int
        p0 := &i[0]
        p1 := &i[1]
        p2 := &i[2]
    */

    *p0 = 5
    *p1 = 6
    *p2 = 7

    fmt.Println("Hello, playground:", *p0, *p1, *p2)
}

However, if you comment out the second block of code like this:

package main

import "fmt"

func main() {
    var i [3]int
    p0 := &i[0]
    p1 := &i[1]
    p2 := &i[2]

    /*
    *p0 = 5
    *p1 = 6
    *p2 = 7
    */

    fmt.Println("Hello, playground:", *p0, *p1, *p2)
}

What did you expect to see?

After formatting the above code with gofmt, I would expect it to be formatted similarly to when any other piece of code is commented out, so it should be:

package main

import "fmt"

func main() {
    var i [3]int
    p0 := &i[0]
    p1 := &i[1]
    p2 := &i[2]

    /*
        *p0 = 5
        *p1 = 6
        *p2 = 7
    */

    fmt.Println("Hello, playground:", *p0, *p1, *p2)
}

What did you see instead?

package main

import "fmt"

func main() {
    var i [3]int
    p0 := &i[0]
    p1 := &i[1]
    p2 := &i[2]

    /*
    *p0 = 5
    *p1 = 6
    *p2 = 7
     */

    fmt.Println("Hello, playground:", *p0, *p1, *p2)
}

Which is not consistent with anything.

I'm pretty sure I know why it happens. It has to do with go/printer detecting a so called "line of stars" style of comments, which normally look like this:

/*
 * Line
 * of
 * stars.
 */

See relevant code at

/*
* Check for vertical "line of stars" and correct prefix accordingly.
*/
lineOfStars := false
if i := strings.Index(prefix, "*"); i >= 0 {
// Line of stars present.
if i > 0 && prefix[i-1] == ' ' {
i-- // remove trailing blank from prefix so stars remain aligned
}
prefix = prefix[0:i]
lineOfStars = true
.

However, in this case, it's not a comment block because there is no space after the * as in normally formatted comments, it's just commented out code that happens to falsely trigger the line of stars comment block handling.

@griesemer

This comment has been minimized.

Copy link
Contributor

griesemer commented Aug 18, 2015

I probably went overboard trying to recognize the "Line of stars".

This might be easily fixable by recognizing the "Line of stars" only if stars followed by text have a blank after the star. Of course, for "* p = 0" this would still fail, but people tend to write "p" rather than " p".

Another option might be to simply remove the "Line of stars" logic.

Not urgent.

@dmitshur

This comment has been minimized.

Copy link
Member Author

dmitshur commented Aug 21, 2015

This might be easily fixable by recognizing the "Line of stars" only if stars followed by text have a blank after the star. Of course, for "* p = 0" this would still fail, but people tend to write "p" rather than " p".

Yep.

Another option might be to simply remove the "Line of stars" logic.

I was thinking that too, and if possible, I think it's a better solution because it's a chance to make Go a little simpler. I've seen a lot of Go code, and line of stars is exceedingly rare. It seems it'd be better not to have it.

Not urgent.

Agreed. I filed this simply to track the issue, since I had encountered it just recently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.