cockroachdb/crlfmt

Formatter for CockroachDB's additions to the Go style guide.

crlfmt's Issues

bad formatting for inline comment for return value

I want this:

func (r *limitedCommandResult) AddRow(
  ctx context.Context, row tree.Datums,
) (_ /* cont */ bool, _ error) {

the tool gives me this:

func (r *limitedCommandResult) AddRow(
	ctx context.Context, row tree.Datums,
) (
	_ bool, /* cont */
	_ error,
) {

Add format-only mode

Running crlfmt with imports processing on the entire repo takes over 90 seconds; this is currently the slowest part of our lint pipeline. For comparison, running gofmt -s on the whole repo takes about 9 seconds. Setting imports.Options.FormatOnly cuts crlfmt down to 16 seconds.

We don't lose anything by setting FormatOnly when run from the linter because any missing imports would be reported instead as a failure to build. The non-formatting parts of goimports are only necessary and useful when it is being run from an editor.

Support goimports `-srcdir` flag=

goimports has a flag -srcdir which is necessary for resolving imports in some cases (involving the vendor and internal paths, i think). The emacs gofmt/goimports support adds this flag automatically when the name of the command is goimports.

Concretely, the etcd package recently moved and we have the new paths in our vendor dir and i have the old path elsewhere on my gopath. goimports with the -srcdir flag picks up the right version while crlfmt does not. It looks like we need to add this flag to the import-adding part of crlfmt (and monkey-patch emacs go-mode to add the flag to crlfmt).

function name formatter can't count columns

It wants me to put a 107 line

    		-func (h *txnHeartbeat) finalTxnStatsLocked() (
    		-	duration, restarts int64, status roachpb.TransactionStatus,
    		-) {
    		+func (h *txnHeartbeat) finalTxnStatsLocked() (duration, restarts int64, status roachpb.TransactionStatus) {

tests don't pass

I think that tests are failing since #25?

cc @dt

Default `-tab` to 2

It's kind of silly that this CRL-specific tool configures itself to our 100-char line length by default but not our 2-space indentation, requiring extra configuration in everybody's editor.

crlfmt doesn't let me wrap function calls at less than 100 chars

The tool is removing wraps that I've added by hand. I think this is going way too far.
Radu raised the issue too in cockroachdb/cockroach#8823 (comment) but a quick discussion concluded that "less options is better". I'd like to revisit that. If I chose to wrap, let it be.
To lawyer up, this rule is not in the style guide. The style guide has vague wording saying to assume code is gonna be read on 100 cols terminals and to wrap at 100, but I'm pretty sure the intent was to set an upper bound.

Before:
func createTestClientForUser(
  t *testing.T,
  stopper *stop.Stopper,
  addr, user string,
  dbCtx client.DBContext,       
) *client.DB {


After:
func createTestClientForUser(
  t *testing.T, stopper *stop.Stopper, addr, user string, dbCtx 
 ) *client.DB {

cc @RaduBerinde @knz @paperstreet

Create wrapper that also calls `goimports`

Suggested by @RaduBerinde so we can swap out invocations of goimports in our editor configurations with crlfmt or a similar wrapper. See cockroachdb/cockroach#8823

crlfmt strips comments

/Users/andrei/work/src/github.com/cockroachdb/cockroach/storage/engine/mvcc.go:476
-func MVCCPutProto(
-   ctx context.Context,
-   engine ReadWriter,
-   ms *enginepb.MVCCStats, // can be nil is the key is unreplicated and doesn't affect stats
-   key roachpb.Key,
-   timestamp hlc.Timestamp,
-   txn *roachpb.Transaction,
-   msg proto.Message,
-) error {
+func MVCCPutProto(
+   ctx context.Context,
+   engine ReadWriter,
+   ms *enginepb.MVCCStats,
+   key roachpb.Key,
+   timestamp hlc.Timestamp,
+   txn *roachpb.Transaction,
+   msg proto.Message,
+) error {

Function declaration wrapping preservers incorrect newlines

As @knz pointed out in cockroachdb/cockroach#9687:

func updateStatsOnPut(
    key roachpb.Key,
    origMetaKeySize, origMetaValSize,
    metaKeySize, metaValSize int64,
    orig, meta *enginepb.MVCCMetadata,
) enginepb.MVCCStats {

This happens because the unformatted input had origMetaKeySize, origMetaValSize, and metaKeySize, metaValSize on different lines. crlfmt should reformat the list of arguments and remove newlines.

lint for unnecessary import aliases

I've observed goimports generating code like this from time to time:

import (
  fmt "fmt"
)

Not sure why it happens, but we should lint against it.

panic: runtime error: invalid memory address or nil pointer dereference

Failure below when saving this file:

package sql

func lol()

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4018d9]

goroutine 1 [running]:
panic(0x4fd560, 0xc42000e0e0)
        /usr/local/go/src/runtime/panic.go:500 +0x1a1
main.checkPath(0xc42006a0c0, 0x12, 0x51e415, 0x3, 0xc42003bd01)
        /go/src/github.com/cockroachdb/crlfmt/main.go:129 +0x2f9
main.main.func1(0xc42006a0c0, 0x12, 0x59f1c0, 0xc420075450, 0x0, 0x0, 0x0, 0x0)
        /go/src/github.com/cockroachdb/crlfmt/main.go:81 +0xbd
path/filepath.walk(0xc42006a0c0, 0x12, 0x59f1c0, 0xc420075450, 0xc42006a160, 0x0, 0x1)
        /usr/local/go/src/path/filepath/path.go:351 +0x81
path/filepath.Walk(0xc42006a0c0, 0x12, 0xc42006a160, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:398 +0xd5
main.main()
        /go/src/github.com/cockroachdb/crlfmt/main.go:87 +0x28f

crlfmt wants a 101-characters line

It doesn't like:

func (p *planner) ShowConstraints(
	ctx context.Context, n *parser.ShowConstraints,
) (planNode, error) {

and it wants:

func (p *planner) ShowConstraints(ctx context.Context, n *parser.ShowConstraints) (planNode, error) {

Which has 101 chars.

Can we please take this opportunity to also look at #5 and allow wrapping at less than 100 chars. I don't know how others feel, but this is eating a piece of my liver every time.

The output:

    	style_test.go:389: /Users/andrei/work/src/github.com/cockroachdb/cockroach/pkg/sql/show.go:719
    	style_test.go:389: -func (p *planner) ShowConstraints(
    	style_test.go:389: -	ctx context.Context, n *parser.ShowConstraints,
    	style_test.go:389: -) (planNode, error) {
    	style_test.go:389: +func (p *planner) ShowConstraints(ctx context.Context, n *parser.ShowConstraints) (planNode, error) {
    	style_test.go:389:

cc @jordanlewis

Panic: unreachable

I get a panic when trying to format my code:

# h32 @ DESKTOP-J1Q9JRK in ~/mysvc on git:main o [10:37:15] C:1
$ go install github.com/cockroachdb/crlfmt@latest
go: downloading github.com/cockroachdb/crlfmt v0.0.0-20220610162206-024b567ce87b
go: downloading github.com/cockroachdb/gostdlib v1.13.0
go: downloading github.com/cockroachdb/ttycolor v0.0.0-20180709150743-a1d5aaeb377d
go: downloading golang.org/x/tools v0.0.0-20200923014426-f5e916c686e1

# h32 @ DESKTOP-J1Q9JRK in ~/mysvc on git:main o [10:38:08] C:130
$ crlfmt .
panic: unreachable

goroutine 1 [running]:
github.com/cockroachdb/gostdlib/go/printer.(*printer).expr1(0xc0003d5cf8, {0x71a318, 0xc0003c2180}, 0x7, 0x1)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:967 +0x1bfd
github.com/cockroachdb/gostdlib/go/printer.(*printer).possibleSelectorExpr(0xc0003d5cf8?, {0x71a318?, 0xc0003c2180?}, 0x40c8c9?, 0xc0003c1560?)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:975 +0x49
github.com/cockroachdb/gostdlib/go/printer.(*printer).expr1(0xc0003d5cf8, {0x719f28, 0xc0003c21c0}, 0x0, 0x1)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:883 +0x1445
github.com/cockroachdb/gostdlib/go/printer.(*printer).expr(...)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1001
github.com/cockroachdb/gostdlib/go/printer.(*printer).printNode(0xc0003d5cf8, {0x670080?, 0xc0003c21c0?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1128 +0x331
github.com/cockroachdb/gostdlib/go/printer.(*Config).fprint(0xc0003d5ea8, {0x7181c8?, 0xc0003c6d50}, 0x40982d?, {0x670080, 0xc0003c21c0}, 0xc0003c6a50)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1293 +0x17c
github.com/cockroachdb/gostdlib/go/printer.(*printer).nodeSize(0xc0003d6620, {0x719118?, 0xc0003c21c0}, 0xf4240)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1599 +0x134
github.com/cockroachdb/gostdlib/go/printer.(*printer).exprList(0xc0003d6620, 0xc0003d63b8?, {0xc0003c2300, 0x3, 0x66?}, 0x20?, 0x1, 0x12?, 0x0)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:212 +0x8c5
github.com/cockroachdb/gostdlib/go/printer.(*printer).expr1(0xc0003d6620, {0x719f28, 0xc0003c2340}, 0x0, 0x1)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:893 +0x17b1
github.com/cockroachdb/gostdlib/go/printer.(*printer).expr(...)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1001
github.com/cockroachdb/gostdlib/go/printer.(*printer).printNode(0xc0003d6620, {0x670080?, 0xc0003c2340?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1128 +0x331
github.com/cockroachdb/gostdlib/go/printer.(*Config).fprint(0xc0003d67d0, {0x7181c8?, 0xc0003c6d20}, 0x40982d?, {0x670080, 0xc0003c2340}, 0xc0003c6a50)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1293 +0x17c
github.com/cockroachdb/gostdlib/go/printer.(*printer).nodeSize(0xc0003d73c0, {0x719118?, 0xc0003c2340}, 0xf4240)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1599 +0x134
github.com/cockroachdb/gostdlib/go/printer.(*printer).exprList(0xc0003d73c0, 0xc0003d6da8?, {0xc0003aa820, 0x1, 0x4cfe83?}, 0xc00036ec60?, 0x0, 0x1?, 0x0)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:212 +0x8c5
github.com/cockroachdb/gostdlib/go/printer.(*printer).stmt(0xc0003d73c0, {0x719da8, 0xc0003c2380}, 0x0)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1219 +0x137b
github.com/cockroachdb/gostdlib/go/printer.(*printer).stmtList(0xc0003d73c0, {0xc0003c24c0, 0x4, 0x13?}, 0x1, 0x1)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1027 +0x2bf
github.com/cockroachdb/gostdlib/go/printer.(*printer).block(0xc0003d73c0, 0xc0003a9b60, 0x1?)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1050 +0xc5
github.com/cockroachdb/gostdlib/go/printer.(*printer).funcBody(0xc0003d73c0, 0x8e, 0xb, 0xc0003a9b60)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1687 +0x268
github.com/cockroachdb/gostdlib/go/printer.(*printer).funcDecl(0x52bbcb?, 0xc0003a9b90)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1711 +0x17d
github.com/cockroachdb/gostdlib/go/printer.(*printer).decl(0xc0003d73c0?, {0x71a138?, 0xc0003a9b90?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1721 +0x51
github.com/cockroachdb/gostdlib/go/printer.(*printer).declList(0xc0003d73c0, {0xc0003ae400?, 0x9, 0x0?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1764 +0x449
github.com/cockroachdb/gostdlib/go/printer.(*printer).file(0xc0003d73c0?, 0xc00033dc00)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/nodes.go:1772 +0xf3
github.com/cockroachdb/gostdlib/go/printer.(*printer).printNode(0xc0003d73c0, {0x66c480?, 0xc00033dc00?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1152 +0x477
github.com/cockroachdb/gostdlib/go/printer.(*Config).fprint(0xc0003d75d0, {0x7181c8?, 0xc0003c6a20}, 0x0?, {0x66c480, 0xc00033dc00}, 0xc0003c6a50)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1293 +0x17c
github.com/cockroachdb/gostdlib/go/printer.(*Config).Fprint(...)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/go/printer/printer.go:1351
github.com/cockroachdb/gostdlib/x/tools/internal/imports.formatFile(0xc0000cd980?, 0xc0000cd980?, {0xc0003be000, 0x95f, 0x960}, 0x0, 0xc0003d7718)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/x/tools/internal/imports/imports.go:177 +0x17c
github.com/cockroachdb/gostdlib/x/tools/internal/imports.Process({0xc0000cd980, 0x12}, {0xc0003be000?, 0x10000000000000b?, 0x15e?}, 0x5f3?)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/x/tools/internal/imports/imports.go:63 +0x113
github.com/cockroachdb/gostdlib/x/tools/imports.Process({0xc0000cd980, 0x12}, {0xc0003be000, 0x95f, 0x960}, 0x64?)
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/x/tools/imports/forward.go:55 +0x185
main.checkBuf({0xc0000cd980, 0x12}, {0xc0003be000?, 0xc0000021a0?, 0x43c325?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/main.go:172 +0x1df
main.checkPath({0xc0000cd980, 0x12})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/main.go:125 +0x79
main.run.func1({0xc0000cd980, 0x12}, {0x71afe8?, 0xc0003b8820?}, {0x0?, 0x0?})
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/main.go:111 +0x10a
path/filepath.walk({0xc0000cd980, 0x12}, {0x71afe8, 0xc0003b8820}, 0xc000391ef8)
        /home/h32/.gvm/gos/go1.18.2/src/path/filepath/path.go:418 +0x123
path/filepath.walk({0xc0003ac9c7, 0x9}, {0x71afe8, 0xc0003b8750}, 0xc0003d7ef8)
        /home/h32/.gvm/gos/go1.18.2/src/path/filepath/path.go:442 +0x285
path/filepath.walk({0x6a21a0, 0x1}, {0x71afe8, 0xc0000a3c70}, 0xc0003d7ef8)
        /home/h32/.gvm/gos/go1.18.2/src/path/filepath/path.go:442 +0x285
path/filepath.Walk({0x6a21a0, 0x1}, 0xc000093ef8)
        /home/h32/.gvm/gos/go1.18.2/src/path/filepath/path.go:505 +0x6c
main.run()
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/main.go:98 +0x17f
main.main()
        /home/h32/.gvm/pkgsets/go1.18.2/global/pkg/mod/github.com/cockroachdb/[email protected]/main.go:56 +0x19