cmd/gofmt: will change some comments to a less correct format
Abirdcfly opened this issue · 5 comments
What version of Go are you using (go version)?
$ go version go version go1.19 darwin/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env GO111MODULE="auto" GOARCH="amd64" GOBIN="/usr/local/opt/go/libexec/bin" GOCACHE="/Users/fupeng/Library/Caches/go-build" GOENV="/Users/fupeng/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/fupeng/go/pkg/mod" GONOPROXY="*tenxcloud.com" GONOSUMDB="*tenxcloud.com" GOOS="darwin" GOPATH="/Users/fupeng/go" GOPRIVATE="*tenxcloud.com" GOPROXY="https://goproxy.cn" GOROOT="/usr/local/opt/go/libexec" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64" GOVCS="" GOVERSION="go1.19" GCCGO="gccgo" GOAMD64="v1" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/fupeng/go/src/tools/go.mod" GOWORK="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/3v/mkwyck4x0f5bxv2gdv4_l8v00000gn/T/go-build1860409478=/tmp/go-build -gno-record-gcc-switches -fno-common"
What did you do?
Let me use a few examples of code from the https://github.com/golang/tools
example:
1. in go1.18 gofmt does not change these files
$ docker run --rm -v $PWD:/go/src/tools --rm -ti golang:1.18 gofmt -w -d /go/src/tools/gopls/internal/coverage/coverage.go
$ docker run --rm -v $PWD:/go/src/tools --rm -ti golang:1.18 gofmt -w -d /go/src/tools/internal/lsp/diff/lcs/labels.go
$ docker run --rm -v $PWD:/go/src/tools --rm -ti golang:1.18 gofmt -w -d /go/src/tools/internal/typeparams/coretype.go
$ docker run --rm -v $PWD:/go/src/tools --rm -ti golang:1.18 gofmt -w -d /go/src/tools/internal/typeparams/typeterm.go2. in go1.19 gofmt make some comments to a less correct format
$ gofmt -w -d gopls/internal/coverage/coverage.go internal/lsp/diff/lcs/labels.go internal/typeparams/coretype.go internal/typeparams/typeterm.go
diff gopls/internal/coverage/coverage.go.orig gopls/internal/coverage/coverage.go
--- gopls/internal/coverage/coverage.go.orig
+++ gopls/internal/coverage/coverage.go
@@ -12,9 +12,13 @@
// -o controls where the coverage file is written, defaulting to /tmp/cover.out
// -i coverage-file will generate the report from an existing coverage file
// -v controls verbosity (0: only report coverage, 1: report as each directory is finished,
-// 2: report on each test, 3: more details, 4: too much)
+//
+// 2: report on each test, 3: more details, 4: too much)
+//
// -t tests only tests packages in the given comma-separated list of directories in gopls.
-// The names should start with ., as in ./internal/regtest/bench
+//
+// The names should start with ., as in ./internal/regtest/bench
+//
// -run tests. If set, -run tests is passed on to the go test command.
//
// Despite gopls' use of goroutines, the counts are almost deterministic.
diff internal/lsp/diff/lcs/labels.go.orig internal/lsp/diff/lcs/labels.go
--- internal/lsp/diff/lcs/labels.go.orig
+++ internal/lsp/diff/lcs/labels.go
@@ -8,7 +8,8 @@
"fmt"
)
-// For each D, vec[D] has length D+1,
+// For each D, vec[D] has length D+1,
+//
// and the label for (D, k) is stored in vec[D][(D+k)/2].
type label struct {
vec [][]int
diff internal/typeparams/coretype.go.orig internal/typeparams/coretype.go
--- internal/typeparams/coretype.go.orig
+++ internal/typeparams/coretype.go
@@ -81,13 +81,13 @@
// restrictions may be arbitrarily complex. For example, consider the
// following:
//
-// type A interface{ ~string|~[]byte }
-//
-// type B interface{ int|string }
-//
-// type C interface { ~string|~int }
-//
-// type T[P interface{ A|B; C }] int
+// type A interface{ ~string|~[]byte }
+//
+// type B interface{ int|string }
+//
+// type C interface { ~string|~int }
+//
+// type T[P interface{ A|B; C }] int
//
// In this example, the structural type restriction of P is ~string|int: A|B
// expands to ~string|~[]byte|int|string, which reduces to ~string|~[]byte|int,
diff internal/typeparams/typeterm.go.orig internal/typeparams/typeterm.go
--- internal/typeparams/typeterm.go.orig
+++ internal/typeparams/typeterm.go
@@ -10,11 +10,10 @@
// A term describes elementary type sets:
//
-// ∅: (*term)(nil) == ∅ // set of no types (empty set)
-// 𝓤: &term{} == 𝓤 // set of all types (𝓤niverse)
-// T: &term{false, T} == {T} // set of type T
-// ~t: &term{true, t} == {t' | under(t') == t} // set of types with underlying type t
-//
+// ∅: (*term)(nil) == ∅ // set of no types (empty set)
+// 𝓤: &term{} == 𝓤 // set of all types (𝓤niverse)
+// T: &term{false, T} == {T} // set of type T
+// ~t: &term{true, t} == {t' | under(t') == t} // set of types with underlying type t
type term struct {
tilde bool // valid if typ != nil
typ types.TypeWhat did you expect to see?
I guess there are at least some comments that should not be changed
What did you see instead?
The comments on these files have changed
This is intended behavior; see https://tip.golang.org/doc/go1.19#go-doc and #51082. If you find a particular bug with the rewriting, I would suggest to make your bug report clearer, like #54312. You can always try to argue that this whole rewriting feature shouldn't happen at all, but then that's more of a feature reversal request than a bug report.
+// For each D, vec[D] has length D+1,
That an error in source formatting (extra space), that needs to be fixed manually
The rest look like gofmt working as intended.
@seankhliao please look at
The original spacing looks correct:
// -v controls verbosity (0: only report coverage, 1: report as each directory is finished,
// 2: report on each test, 3: more details, 4: too much)will gofmt format as fellow:
// -v controls verbosity (0: only report coverage, 1: report as each directory is finished,
//
// 2: report on each test, 3: more details, 4: too much)
//But this is incorrect, the 2: xxx 3: xxx 4: xxx here is just a manual line break because the previous line is too long.
the 2: xxx 3: xxx 4: xxx should be next to the previous line 0:xxx 1:xxx with some Space but no newline.
I have try some option:
//(Space)(Space)2://(Space)(Space)(Space)2://(Tab)2://(Tab)(Tab)2://(Space)2:- use
/* */
only option 5 and option 6 Keep my spacing from being gofmt to the above form. but option 5 make it look like:
// -v controls verbosity (0: only report coverage, 1: report as each directory is finished,
// 2: report on each test, 3: more details, 4: too much)It is also not good. So the only option available is to use /* */:
/* -v controls verbosity (0: only report coverage, 1: report as each directory is finished,
2: report on each test, 3: more details, 4: too much)
*/I would like to know if the above changes are bugs or feature? 😂
The original godoc was buggy: it considered the second line as a blockquote due to the spacing. The godoc needs to be manually fixed to either blockquote the whole thing, or to not use any leading spaces and keep the whole thing as a regular paragraph. The gofmt rewriting is making the bad godoc surface, so I think that's a good outcome, even if the godoc is still buggy.
I would like to know if the above changes are bugs or feature? 😂
Please keep it civil and respectful :)
Please keep it civil and respectful :)
Apologies for my poor English and translation tools, no rudeness intended here, I added emoticons to prevent misunderstandings, but it looks like it failed 😢
Thank you for your patience and answers.