Coder Social home page Coder Social logo

Comments (10)

mvdan avatar mvdan commented on June 25, 2024 1

I think I do want to go in the direction of gofumpt versioning its use of go/printer. It will make using it as a library slightly more bloated, but so be it. If users can't trust on a pinned gofumpt version to behave consistently, then the tool isn't particularly useful. gofmt does not have this problem as it ships with Go itself.

from gofumpt.

mvdan avatar mvdan commented on June 25, 2024

I'm finding that the behaviour of gofumpt is sensitive to the version of Go used to build the binary.

For better or worse, that will always be the case, and the same applies to gofmt - both rely on go/printer, which implements the "canonical" gofmt formatting. If that changes between 1.18 and 1.19, then gofumpt will change depending on what Go version you built it with.

In particular, what you are seeing is https://go-review.googlesource.com/c/go/+/384264/, a go/printer change for golang/go#51082 which is documented in the 1.19 release notes.

There is literally no way to work around this without having one fork of go/printer per major Go version, and that's not something I am contemplating to do. Beyond being an ever-increasing amount of work for me forever, and making gofumpt larger and larger in code and binary size, it's just a very heavyweight approach to how -lang should work. Put simply, if someone really wants old Go formatting behavior, they can always use that older Go version to build the formatting tool. But I also don't expect this scenario to be very common.

I know that for reformatting godoc comments it may seem obvious to just disable that bit of code if -lang is older than 1.19, but in many cases it's not nearly that simple. A few releases ago, go/printer changed its heuristic for how it aligns inline comments vertically. Without keeping two copies of go/printer, the only alternative I would have is to manually add a -lang conditional that implements both heuristics at once.

I think perhaps -lang is not very well documented if you got confused by this situation. It is meant to be a language version, i.e. what Go language spec features it can rely on for formatting. For example, it will only rewrite 0777 to 0o777 if your -lang version (by default derived from go 1.X in go.mod) is new enough to support octal literals with the 0o prefix, since that was added in Go 1.13. I can imagine other rules or simplifications which would rely on newer syntax changes in the future.

So the flag was never intended to mean "format exactly like Go version 1.x" - hopefully that's clear. I'd be interested to hear where you got that impression, so I can improve the docs :)

from gofumpt.

mvdan avatar mvdan commented on June 25, 2024

It could always be possible for gofumpt to give an error if its -lang version is not the same as the Go version that built the binary, but that would feel overly restrictive in my opinion. Right now, I can use a recent gofmt on any Go project, even if it was written with Go 1.12. I should be able to do the same with gofumpt - it is a drop-in replacement for gofmt, after all. And gofmt will still apply those godoc comment rewrites to older projects as well.

from gofumpt.

adamroyjones avatar adamroyjones commented on June 25, 2024

Thanks for the quick, detailed, and illuminating reply—I appreciate it. Thanks for your work on gofumpt, as well. It's a wonderful tool.


Put simply, if someone really wants old Go formatting behavior, they can always use that older Go version to build the formatting tool. But I also don't expect this scenario to be very common.

The triggering scenario for me was the following; I believe you're right about its rarity.

My employer has a repository that requires the use of Go 1.18. This repository has a CI pipeline that uses golangci-lint. The binaries that are distributed with golangci-lint are built using Go 1.19:

go version -v $(which golangci-lint)

This inconsistency can cause CI pipelines to fail.

There are obvious solutions to that immediate problem, of course; I'll build gofumpt locally with Go 1.19 and use that.


I think perhaps -lang is not very well documented if you got confused by this situation. It is meant to be a language version, i.e. what Go language spec features it can rely on for formatting [...] the flag was never intended to mean "format exactly like Go version 1.x" - hopefully that's clear. I'd be interested to hear where you got that impression, so I can improve the docs :)

It was a breezy, optimistic assumption on my part. The documentation I saw was from the struct definition of format.Options here, which says that:

LangVersion corresponds to the Go language version a piece of code is written in. The version is used to decide whether to apply formatting rules which require new language features.

I didn't realise that this meant that the version is only used for that purpose; I assumed more than was written. It's a behaviour that resists a pithy explanation, though.


You're, of course, right that it'd be an unsustainable and unreasonable burden on you to maintain forks of go/printer. It'd also bloat and introduce fragility to the (lean!) codebase to start jamming in logic that tries to handle all possible versions of lang, all changing behaviours of go/printer, and so on.

One possibility would be for releases of gofumpt to be built and distributed on GitHub. This'd dampen down the variation in behaviour, but that likewise introduces a new burden—one that I'd be happy to share, if you'd benefit from that.

from gofumpt.

mvdan avatar mvdan commented on June 25, 2024

Thanks for being understanding! This is a tricky situation so I understand where you were coming from. I actually made a proposal to upstream to provide versioned gofmt downloads for a very similar reason, so that at a previous company we could all use exactly the same Go formatting even if some of the developers were on slightly newer Go versions: golang/go#26397

My employer has a repository that requires the use of Go 1.18. This repository has a CI pipeline that uses golangci-lint. The binaries that are distributed with golangci-lint are built using Go 1.19:

Arguably, that may be a problem for more than just formatting. If your work project builds and tests with Go 1.18, you should also use your tools (formatting, static analysis, editors with LSP, etc) with 1.18 as well. Using 1.19 may lead to subtle differences in behavior beyond just formatting.

I didn't realise that this meant that the version is only used for that purpose; I assumed more than was written.

I'll think about making the docs clearer - I'll ping you for a PR review if that's OK.

One possibility would be for releases of gofumpt to be built and distributed on GitHub.

Personally, the only reason I thought the project didn't need release binaries is because it is a tool for Go developers. For them, I imagine that running go install mvdan.cc/gofumpt@latest is easier than running a long curl command and placing the binary in the right place. go install also has the ability of automatically figuring out what the latest version is, for example.

Perhaps you're right that each gofumpt release should advocate what Go version it mainly targets. https://github.com/mvdan/gofumpt/releases/tag/v0.3.0 did say it was based on Go 1.18's gofmt, and that relates to code in cmd/gofmt like its flags, but not to go/printer. Perhaps the release notes should clarify that each version recommends to be built with a certain Go version, and provide binaries to make that a bit easier for some use cases like CI. Other Go versions will still most likely work, but with varying degrees of different formatting results.

from gofumpt.

adamroyjones avatar adamroyjones commented on June 25, 2024

Arguably, that may be a problem for more than just formatting. If your work project builds and tests with Go 1.18, you should also use your tools (formatting, static analysis, editors with LSP, etc) with 1.18 as well. Using 1.19 may lead to subtle differences in behavior beyond just formatting.

Yes: as good as Go is with its backward-compatibility guarantees, it'd be better if everything were on the same version. I'll have a look into what I can do.

I'll think about making the docs clearer - I'll ping you for a PR review if that's OK.

By all means!

Perhaps you're right that each gofumpt release should advocate what Go version it mainly targets.

I think that'd definitely help. An additional possibility would be for gofumpt to print a warning if the version used to build it differs from the recommended version. Would you welcome a PR that adds that functionality?

from gofumpt.

mvdan avatar mvdan commented on June 25, 2024

I'm not sure about the warning. Without any flags, when running gofumpt on its own codebase today with Go 1.19, it would default to -lang=1.18, derived from go.mod. Note that it's 1.18 even though I develop and build on Go 1.19, because I only need language features from 1.18 onwards. In other words, 1.18 is the minimum version of Go I support (language-wise), but then I currently target 1.19. And the target constantly moves too; when 1.20 is out, I may start using that, but I might not rush to drop support for 1.18 right away.

from gofumpt.

adamroyjones avatar adamroyjones commented on June 25, 2024

Yes, quite right—sorry. It's all a bit tricky! Perhaps the least bad solution would be:

  • for each gofumpt tag, provide a range of supported Go versions (which looks to be the case now);
  • for each gofumpt tag, each corresponding supported Go version, and each supported platform, build a corresponding binary with that version of Go and distribute it on GitHub;
  • add a bit of documentation in the readme that highlights that the behaviour of gofumpt depends on the version of Go it was built with; and
  • add a small clarifying note in the documentation that emphasises that the lang option doesn't try to fully "impersonate" that version of Go.

I'll defer to you and your judgement, of course. I'm happy to try and help however I can, whether it's writing documentation, reviewing things, or working on a pipeline.

from gofumpt.

mvdan avatar mvdan commented on June 25, 2024

Your points about the docs are certainly a good idea - happy to review a PR :)

I'm less convinced that providing so many gofumpt binaries will be a net benefit. My guess is that it would confuse users more than anything. We should recommend using a gofumpt version with one major version of Go, and that's easy enough. If a project absolutely cannot use that Go version just yet, they can either use our prebuilt binary, or they can choose to stay on a slightly older version for a bit.

I also just realised that having gofumpt's formatting be independent of the Go version isn't such a difficult task. Yes, we would have to keep a vendored copy of go/printer, and potentially other packages which affect its behavior like go/doc/comment, but we only need to keep one copy as long as -lang keeps its current behavior to only affect the use of Go language features.

Part of me still thinks that's nasty, as it would lead to some downstreams like gopls shipping multiple copies of the same packages - often for no benefit at all, if the versions are almost exactly the same.

from gofumpt.

adamroyjones avatar adamroyjones commented on June 25, 2024

I'm less convinced that providing so many gofumpt binaries will be a net benefit. My guess is that it would confuse users more than anything. We should recommend using a gofumpt version with one major version of Go, and that's easy enough. If a project absolutely cannot use that Go version just yet, they can either use our prebuilt binary, or they can choose to stay on a slightly older version for a bit.

I think you're right, having reflected on it; I think providing a single simple recommendation makes the most sense. Opinionated tools are, after all, opinionated.

from gofumpt.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.