pggen

pggen copied to clipboard

Adds the field names when instantiating the compositeField and textPreferrer struct in the generated code.

This allows to run the fieldalignment linter without errors on the generated code.

@jschaf I have the following linter errors:

make lint
golangci-lint run
internal/pgdocker/pgdocker.go:20:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"
	^
internal/gomod/gomod.go:9:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"
	^
internal/parser/interface.go:10:2: SA1019: "io/ioutil" has been deprecated since Go 1.16: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"
	^
make: *** [Makefile:44: lint] Error 1

This seems unrelated to the changes I am submitting here. However I'm happy to add a commit to this PR with a fix if you want me to?

No issues and I'm likely to merge. First, I'd like to understand which lint the change fixes. Seems like fieldalignment is unaffected since the ordering when defining textPreferrer is unchanged.

@jschaf thanks for your quick reply an sorry I was not very clear. I run fieldalignment on the code generated by pggen with the -fix flag. This will change the code by reorganizing the order of fields of every struct at the struct declaration site only in the most space-efficient way. The main candidates here are the struct holding the query params and query outputs (rows).

However fieldalignment is a very simple (dumb) tool and will change the definition of all non-optimized structs it finds (in a given file, package, etc.) But it will not change the order of fields in struct instantiations! Hence the need to use named fields in struct instantiations such that the order is irrelevant. (All of the generated code already does that except for the changes in this PR.)

It's true that in the case of textPreferrer, the fields order is already in the best order so it does not change anything. The main issue is for compositeField which is reorganized by fieldalignment as (defaultVal is now first):

type compositeField struct {
	defaultVal pgtype.ValueTranscoder
	name       string
	typeName   string
}

However I did not see a reason not to fix both. And having the names used for textPreferrer ensures any change to the struct (like adding a field) will force the instantiation to use the field name as well and won't break in the future.

I though this also made sense with the rest of the code which uses the field names in struct instantiations. Reading the last point of the design goals of pggen in CONTRIBUTING.md I thought this was aligned (pun intended):

Generated code should look like a human wrote it. The generated code should be near perfect, including formatting. pggen output doesn't depend on gofmt.

Awesome, thank you for the explanation and code. Makes sense and definitely fits in with the goals of generating human-like code.