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: match indentation when returning multiple composite literals #7195

Open
dsymonds opened this Issue Jan 23, 2014 · 7 comments

Comments

Projects
None yet
4 participants
@dsymonds
Copy link
Member

dsymonds commented Jan 23, 2014

http://play.golang.org/p/IxtYF1rXe9
-----
package main

type T struct {
    x int
    y string
}

func f() (*T, *T) {
    return &T{
            x: 1,
            y: "foo",
        }, &T{
            x: 2,
            y: "bar",
        }
}
-----

This doesn't happen when there's only one return value, or when one of the return values
is "nil".
@dsymonds

This comment has been minimized.

Copy link
Member Author

dsymonds commented Jan 23, 2014

Comment 1:

It also happens if one of the return values is an anonymous func, which is where I keep
tripping over this.
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Apr 3, 2014

Comment 2:

Labels changed: added suggested, removed release-go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Apr 3, 2014

Comment 3:

Labels changed: added release-none.

@griesemer

This comment has been minimized.

Copy link
Contributor

griesemer commented Apr 21, 2014

Comment 4:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@agnivade agnivade changed the title gofmt: Bad handling of multiple composite literal returns go/printer: bad handling of multiple composite literal returns Jun 16, 2018

@agnivade

This comment has been minimized.

Copy link
Member

agnivade commented Jun 16, 2018

@dsymonds - Just to clarify - you expected something like this ?

func f() (*T, *T) {
	return &T{
		x: 1,
		y: "foo",
	}, &T{
		x: 2,
		y: "bar",
	}
}

i.e. one less indent when arranging the composite literals.

@dsymonds

This comment has been minimized.

Copy link
Member Author

dsymonds commented Jun 16, 2018

Yes, I'd expect those two closing braces for the &T literals to be at the same indentation as the return statement, but also the fields should only get a single level of indent, like what you get when you declare a var in the same way: (https://play.golang.org/p/0pH8-b_MdNI)

package main

type T struct {
	x int
	y string
}

func f() (*T, *T) {
	// Declaring a looks right.
	a := &T{
		x: 1,
		y: "foo",
	}

	// Returning a single composite literal looks right.
	return &T{
		x: 1,
		y: "foo",
	}

	// Returning multiple values looks wrong
	return &T{
			x: 1,
			y: "foo",
		}, &T{
			x: 2,
			y: "bar",
		}
}

@agnivade agnivade changed the title go/printer: bad handling of multiple composite literal returns go/printer: match indentation when returning multiple composite literals Jun 16, 2018

@agnivade

This comment has been minimized.

Copy link
Member

agnivade commented Jun 28, 2018

Analysis: This is due to the indentList() function in go/printer/nodes.go L1229. If that function returns false, then the literals line up as intended.

And looking at the comment, it seems to be intentionally written to solve issue #1207 (which was incidentally filed by @dsymonds yourself :)).

// Heuristic: indentList returns true if there are more than one multi-
// line element in the list, or if there is any element that is not
// starting on the same line as the previous one ends.

The difference appears if the element is itself a composite literal or not. For example, if that function returns false, this issue is solved. But issue 1207 breaks.

Breaks -

return &T{
       	x:	1,
       	y:	"foo",
},
      	nil

Fixes-

return &T{
        x:	1,
        y:	"foo",
 }, &T{
       	x:	2,
       	y:	"bar",
 }

Will leave it to @griesemer to take a final call if we want to add more heuristic to check if the elements are composite literals or not.

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.