x/tools/cmd/goimports: rewrite 0X->0x, 0O->0o as gofmt does #37453
Comments
gopherbot
added
the
Tools
|
Hm. I generally agree, but |
Yes, exactly this. This is already how goimports works today and those of us who care about getting a particular behavior of goimports to match gofmt already know that we need to match the goimports build version of Go with the gofmt Go version. For instance, in Go 1.11 gofmt changed the alignment of certain literals (see 542ea5a). If I build (tip) goimports with Go 1.10, I get the Go 1.10 gofmt alignment. If I build goimports with Go 1.11, I get the Go 1.11 gofmt alignment. |
|
Maybe...but messing with someone's spacing feels different than preventing their code from compiling. Perhaps if we did it around 1.15 when there'd be some expectation everyone will have support for the new literals. |
|
@heschik I think you're confusing this with a different issue (possibly #33363). I'm not talking about preventing anyone's code from compiling. This is about:
|
|
Oh. Sorry, not paying enough attention. Sure, yes, let's do it. Do you happen to have the gofmt CL handy? Do you want to send a CL? |
|
The original gofmt change was CL 160184. I can send a goimports CL at some point if nobody beats me to it. |
cagedmantis
added
the
NeedsInvestigation
dmitshur
added
NeedsFix
|
Change https://golang.org/cl/231461 mentions this issue: |
Normalization of number prefixes and exponents was added in CL 160184 directly in cmd/gofmt. The same behavior change needs to be applied in the go/format package. This is done by moving the normalization code into go/printer, behind a new StdFormat mode, which is then re-used by both cmd/gofmt and go/format. Note that formatting of Go source code changes over time, so the exact byte output produced by go/printer may change between versions of Go when using StdFormat mode. What is guaranteed is that the new formatting is equivalent Go code. Clients looking to format Go code with standard formatting consistent with cmd/gofmt and go/format would need to start using this flag, but a better alternative is to use the go/format package instead. Benchstat numbers on go test go/printer -bench=BenchmarkPrint: name old time/op new time/op delta Print-8 4.56ms ± 1% 4.57ms ± 0% ~ (p=0.700 n=3+3) name old alloc/op new alloc/op delta Print-8 467kB ± 0% 467kB ± 0% ~ (p=1.000 n=3+3) name old allocs/op new allocs/op delta Print-8 17.2k ± 0% 17.2k ± 0% ~ (all equal) That benchmark data doesn't contain any numbers that need to be normalized. More work needs to be performed when formatting Go code with numbers, but it is unavoidable to produce standard formatting. Fixes #37476. For #37453. Change-Id: If50bde4035c3ee6e6ff0ece5691f6d3566ffe8d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/231461 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
Normalization of number prefixes and exponents was added in CL 160184 directly in cmd/gofmt. The same behavior change needs to be applied in the go/format package. This is done by moving the normalization code into go/printer, behind a new StdFormat mode, which is then re-used by both cmd/gofmt and go/format. Note that formatting of Go source code changes over time, so the exact byte output produced by go/printer may change between versions of Go when using StdFormat mode. What is guaranteed is that the new formatting is equivalent Go code. Clients looking to format Go code with standard formatting consistent with cmd/gofmt and go/format would need to start using this flag, but a better alternative is to use the go/format package instead. Benchstat numbers on go test go/printer -bench=BenchmarkPrint: name old time/op new time/op delta Print-8 4.56ms ± 1% 4.57ms ± 0% ~ (p=0.700 n=3+3) name old alloc/op new alloc/op delta Print-8 467kB ± 0% 467kB ± 0% ~ (p=1.000 n=3+3) name old allocs/op new allocs/op delta Print-8 17.2k ± 0% 17.2k ± 0% ~ (all equal) That benchmark data doesn't contain any numbers that need to be normalized. More work needs to be performed when formatting Go code with numbers, but it is unavoidable to produce standard formatting. Fixes golang#37476. For golang#37453. Change-Id: If50bde4035c3ee6e6ff0ece5691f6d3566ffe8d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/231461 Run-TryBot: Dmitri Shuralyov <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
|
Change https://golang.org/cl/237740 mentions this issue: |
For #37419 For #37453 For #37476 Change-Id: Ia032ec844773af421bc4217d5dd6e60996d8e91f Reviewed-on: https://go-review.googlesource.com/c/go/+/237740 Reviewed-by: Robert Griesemer <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
|
Change https://golang.org/cl/240683 mentions this issue: |
The StdFormat flag was added as part of CL 231461, where the primary aim was to fix the bug #37476. It's expected that the existing printer modes only adjust spacing but do not change any of the code text itself. A new printing flag served as a way for cmd/gofmt and go/format to delegate a part of formatting work to the printer—where it's more more convenient and efficient to perform—while maintaining current low-level printing behavior of go/printer unmodified. We already have cmd/gofmt and the go/format API that implement standard formatting of Go source code, so there isn't a need to expose StdFormat flag to the world, as it can only cause confusion. Consider that to format source in canonical gofmt style completely it may require tasks A, B, C to be done. In one version of Go, the printer may do both A and B, while cmd/gofmt and go/format will do the remaining task C. In another version, the printer may take on doing just A, while cmd/gofmt and go/format will perform B and C. This makes it hard to add a gofmt-like mode to the printer without compromising on above fluidity. This change prefers to shift back some complexity to the implementation of the standard library, allowing us to avoid creating the new exported printing flag just for the internal needs of gofmt and go/format today. We may still want to re-think the API and consider if something better should be added, but unfortunately there isn't time for Go 1.15. We are not adding new APIs now, so we can defer this decision until Go 1.16 or later, when there is more time. For #37476. For #37453. For #39489. For #37419. Change-Id: I0bb07156dca852b043487099dcf05c5350b29e20 Reviewed-on: https://go-review.googlesource.com/c/go/+/240683 Reviewed-by: Robert Griesemer <[email protected]> Reviewed-by: Russ Cox <[email protected]>
|
Now that Go 1.16 is out, the supported Go versions are 1.16 and 1.15. Both of them perform this canonicalization during |
In Go 1.13, gofmt started canonicalizing hex literals written as
0XABCas0xABCand also started rewriting the newly introduced octal literals written as0O123as0o123. But goimports doesn't do this rewriting. This means that code formatted by goimports may be further altered by gofmt, which I think we've always tried to avoid.The text was updated successfully, but these errors were encountered: