go/printer: CL 609077 causes unintended formatting changes

[hoeppi-google]

go.dev/cl/609077 causes a change in formatting. Reading the CL description and the test cases leads me to believe that this change is unintended.

Reproducer:

package main

import (
	"bytes"
	"fmt"
	"go/parser"
	"go/printer"
	"go/token"
)

const content = `package foo

type S struct {
//- comment
Name *string 
}`

func main() {
	fset := token.NewFileSet()
	file, err := parser.ParseFile(fset, "", content, parser.ParseComments)
	if err != nil {
		panic(err)
	}
	var out bytes.Buffer
	if err := (&printer.Config{Mode: printer.TabIndent | printer.UseSpaces, Tabwidth: 8}).Fprint(&out, fset, file); err != nil {
		panic(err)
	}
	fmt.Println(out.String())
}

Before go.dev/cl/609077, this printed:

package foo

type S struct {
        // - comment
        Name *string
}

After go.dev/cl/609077, this prints:

package foo

type S struct {
        //- comment
        Name *string
}

[gabyhelp]

Related Issues and Documentation

• go/printer: Fprintf uses different settings than go fmt #16963 (closed)
• go/printer: Strange indentation behavior/ broken output under some circumstances #5946 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

cc @mateusz834

[mateusz834]

That is true that:

type S struct {
//- comment
Name *string 
}

was formatted to this:

type S struct {
    // - comment
    Name *string 
}

but, formatting:

type S struct {
    //- comment
    Name *string 
}

formats it to this:

type S struct {
    //- comment
    Name *string 
}

There is no change.

Before CL 609077 the godoc formatting was performed for all comments, that start at the first Column of a line, this is wrong (e.g. for comments at the first column inside of a func declaration). I think that the fix is right.

Maybe there should be a separate discussion to always start adding a space before a comment?

[gopherbot]

Change https://go.dev/cl/612137 mentions this issue: go/printer: revert "do not treat comments inside a ast.Decl as godoc"

[mateusz834]

CC @griesemer

[mateusz834]

This change was reverted, without any further discussion here with a comment:

it turned out to also introduce a change to the formatting as described in issue #69382, which wasn't intended.

This formatting change was intended, see my comment above: #69382 (comment) and the test case, with a similar case (go doc formatting in not executed).

go/src/go/printer/printer_test.go

Lines 919 to 935 in 90391c2

	{
	src: `package main
	func main() {
	//go:directive
	// test
	type a struct{}
	}
	`,
	want: `package main
	func main() {
	//go:directive
	// test
	type a struct{}
	}
	`,

Would like to hear how we should now move forward with this change.

[griesemer]

@mateusz834 Apologies for the revert w/o external discussion. W/o going details, this change broke some Google internal tests and in order to unblock this we decided to revert this change for now. The correct fix may well be that we need to make some changes to the internal code, but we haven't had the time to look into it. If we decide to do that, we may be able to simply reapply your CL. Thanks.