Feat/go formatter - #33
Merged
Merged
Conversation
func Format[T Source](w io.Writer, src T, opts ...Option) error Format parses once, drops the imports nothing uses, sorts and groups the rest, and prints in gofmt style to the writer. Nothing reads the file system or runs the go command, so a source formats the same way on every machine. Source unions []byte and *bytes.Buffer, both of which give up their bytes without copying, so a generator recycles one buffer. It never adds an import. goimports resolves a missing one by searching the module cache and the build list, so a spec and a template produce different files on different machines and a maintainer cannot reproduce what a user reports. A template writes the imports its code needs, and code naming a package it did not import fails to compile. Out go golang.org/x/tools/imports, its LocalPrefix global two goroutines could race on, the $GOPATH/src resolver and BaseImport. Pruning drops an import when no selector uses the name it declares. An alias names the package exactly, and "_" and "." are never touched. Otherwise the name is guessed, and one guess is not enough: goimports reads "gopkg.in/yaml.v3" as yaml.v3 and drops a used import, and stripping the major version reads "k8s.io/api/apps/v1" as apps and drops another. Every legal candidate counts, so a surplus one can only keep an import. Ident.Obj separates a qualifier from a shadowed name, and those scopes cost a sixth of the allocations, so the parse skips them and asks for a second only when an imported name is also declared in the file. WithImportGroups opens a group per prefix between the standard library and the rest, carried in the options value rather than a global. go/format.Node would flatten that, re-sorting every parenthesized block by path, so Format prints through a printer.Config carrying gofmt's own mode bits, and TestMatchesGofmt pins the one bit with no exported name. Group separators are written by a one-line io.Writer, so a whole file streams and only a fragment buffers. WithGoFumpt applies the gofumpt rules from formatting/enable/gofumpt, its own module reached by a blank import, so the root go.mod does not require mvdan.cc/gofumpt. Speed depends on file size: imports.Process spends around 2.6ms per call on environment work before reading anything, and FormatLite, offered by go-swagger behind --with-custom-formatter, parses and prints then hands off to imports.Process to do it again. Medians of three runs, per file: input Format FormatLite goimports us KB us KB us KB 1 KB 102 26 277 95 2811 111 4 KB 514 78 1532 336 3425 279 40 KB 3621 741 15123 3204 11936 2207 160 KB 20700 3432 57359 14093 41924 9481 FormatLite costs three times the time and four times the memory at every size, so that is throughput and not a fixed charge. goimports costs 28 times on a 1 KB file and twice on a 160 KB one; generated models run 1 to 2 KB. FormatLite and goimports emit the same bytes, so grouping is the only change a generated file shows: a group per prefix here, against one merged group after the third-party imports. Fixtures live under formatting/testdata, whose corpus is a module of its own so its goldens compile. TestAgainstReference formats eleven sources with both Format and goimports and compares bytes: all eleven agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
app, err := genapp.New(
genapp.WithTemplates(templates),
genapp.WithOutputPath("./generated"),
)
app.RenderFile("models/pet.go", "modelValidator", pet)
GoGenApp does the three things a generator repeats for every file: run a
template, format what it produced, put it where it goes. Render writes to
an io.Writer; RenderFile writes under the output path, creating the
directories it needs.
It is a port of github.com/fredbi/core/codegen/genapp onto this
repository's templates-repo and formatting, and the swap removes most of
what was there. The repository validates when built, so the sync.Once,
the stored load error and the lazy branch inside Render go, and New
reports a broken template instead of the first render doing so. The
formatter carries its grouping per call, so imports.LocalPrefix and the
mutex guarding it go too, with the tab width and the import-check switch,
which no longer name anything. afero goes: splitting Render from
RenderFile leaves the tests a buffer and a temp dir, which is what the
file system abstraction was for. genapp re-exports nothing of its
dependencies.
A file appears whole or not at all: RenderFile writes beside the target
and renames over it. When the formatter rejects what a template rendered,
that file is kept as <target>.unformatted and the error names it, since a
parse error reports a line and a column and the source they came from
otherwise existed only in a pooled buffer.
Each render borrows that buffer from swag/pools/shared and returns it, so
a generator writing a few hundred files recycles a handful rather than
allocating one apiece.
Modules, without a toolchain:
- InitModule writes a go.mod through modfile, byte for byte what "go mod
init" writes, its go directive taken from the version this program was
built with. WithRequire, WithToolchain and WithGoVersion fill in the
rest, each checked before anything is written, since modfile takes a
require without looking at its version.
- PackagePath names the tree from the go.mod above it, or failing that
from the way down out of GOPATH/src. ModuleRequired reports whether the
path needs a go.mod of its own.
- TidyModule shells out, and is the only thing here needing go
installed: resolving a module graph is the go command's job. It takes a
context, runs with GOWORK off so a workspace that does not list the
generated module cannot refuse the run, and reports what the command
said rather than an exit status.
Answering a question now costs nothing: PackagePath creates no directory
and lays down no overlay file, where packages.Load needed both, and
ModuleRequired matches a sentinel rather than the text "go.mod file not
found". Containment under GOPATH goes through filepath.Rel, so a GOPATH on
one Windows volume and an output path on another is an entry that does not
match rather than a resolution that fails. genapp joins the root module,
adding golang.org/x/mod, already in the graph.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Each of these buried its verb in a relative clause, so the sentence read backwards and defined the thing by what happens to it: Name is the name the template is registered under. The path is the one the asset has once mounted. declared is a template the repository holds, at the address it was declared under. They now name the thing and give it a verb. No code changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
sortImports split the block at every blank line and sorted each run on its own, so "bytes" written in two groups survived twice and the output failed to compile with "bytes redeclared in this block". A template assembling its imports from several fragments hits that whenever two fragments contribute the same package. Drop sortRuns and sort the block as one run. The source's own blank lines no longer reach the output; groupBreaks and spacer put them back from the prefixes WithImportGroups was given, so the layout comes from the option alone. gofmt and goimports keep the run-per-group rule, which is a third place where Format parts company with them. The deduped corpus package is checked by the compiler, not just by a golden: TestCorpusCompiles builds it, and the old behaviour produced a file that does not build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The identifier an import binds is the imported package's own package clause, and the path only says where to find it. A generator writing an import still needs a name to qualify it with, and ImportedPackageName gives the one the naming convention implies: last path element, /v2 or later dropped, last segment of a hyphenated element, rejecting keywords and "_". Version elements are the reason it is worth having. "k8s.io/api/apps/v1" and "k8s.io/api/core/v1" both declare v1 and collide under the name they declare; they give apps and core here, so a generator can write the alias that keeps them apart. The guesses behind pruning move to names.go beside it as importedPackageNames, which returns a candidate list rather than one name: no path separates "k8s.io/api/apps/v1", declaring v1, from "github.com/go-openapi/testify/v2", declaring testify. The hyphen rule now offers pkg, mypkg and my for "example.com/my-pkg", where it used to offer nothing at all; measured over the hyphenated package directories in a module cache, that lifts the hit rate from 46% to 89%, with the right name first in 80% of them. x/tools/internal/imports carries the same rule as ImportPathToAssumedName, which returns one name and cuts at the first character an identifier may not hold, reading "example.com/my-pkg" as my. It is internal and cannot be imported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Pruning read a bare import's name off its path, so it deleted imports the code was using. "github.com/json-iterator/go" declares jsoniter and "github.com/prometheus/client_model/go" declares io_prometheus_client; neither path says so, and no rule reading a path can. Over the 4269 distinct import paths written in a module cache, the shape recurs wherever a polyglot repository puts its Go under a "go" subdirectory. An import is now deleted only when its name is stated rather than inferred: - an alias, which states it in the source; - the standard library, from a table generated by "go list std", asserted against it in internal/std; - WithResolvedImports, where the caller states it. A guess may still save an import, by matching a qualifier the file writes, but it never condemns one. Everything left over is reported in doubt and kept. Keeping an unused import is a compile error too, but it is "imported and not used", which names the line to delete, rather than "undefined: io_prometheus_client" at the use site. WithForceImportsPruning takes the caller's promise that every bare import declares the name ImportedPackageName gives, and prunes on the guesses. It condemns on the whole candidate list rather than on the canonical name, so a file writing v1.Pod keeps its "k8s.io/api/apps/v1". The two options compose: the map covers the paths the promise does not. Format now returns an ImportsReport beside its error, holding one record per import with the name it binds, whether that name was stated or guessed, and what became of it. It comes back on ErrInconsistentImports too, and is nil only when parsing failed. ErrInconsistentImports rejects two shapes a template can write: one package under two names, and one name bound to two packages. The first compiles and reads as though the two were different packages; the second does not compile. A clash between guessed names may not be real, since either package may declare something the path does not show, so those records carry ImportCollision and neither import is pruned. The generated table also replaces "first path element holds no dot" as the test for standard library, so a local module such as "myapp/models" no longer passes for one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Format never loads a package, so it cannot name one whose package clause does not follow its import path, and reports such an import in doubt rather than deleting it. resolve.Names loads them through x/tools/go/packages with NeedName and answers outright, returning the map WithResolvedImports takes: names, err := resolve.Names(ctx, report.PathsInDoubt()) The answer depends on the dependencies and on nothing else, so the map can be produced once and committed, and every generator run agrees wherever it runs. That is the difference from goimports resolving live: searching the build list makes the output depend on the machine. A path that does not resolve is named in the error, which wraps ErrUnresolved, and left out of the map. The map still holds what did resolve, so a caller may use that and act on the rest. It sits in the root module rather than one of its own: x/tools, x/mod and x/sync were required already, so it costs no new dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
An alias states the name an import binds, so a template that writes one everywhere gets exact pruning without promising anything and without a resolved map. The cost is source no Go author would write: import strfmt "github.com/go-openapi/strfmt" This option takes such an alias back out once the name is proven, leaving ordinary Go. Two things have to hold. The name must be stated by the standard library table or by WithResolvedImports, since dropping an alias on a guess breaks the build the moment the guess is wrong. And it must be the name ImportedPackageName gives, so the bare import left behind still says what it binds. The second test is why jsoniter "github.com/json-iterator/go" keeps its alias even with the name proven: dropping it compiles, and throws away the only thing in the file that says which package that is. A later run without the map would put it straight back in doubt. sql "database/sql/driver" keeps its alias too, because it renames the package rather than repeating it. describeImport used to stop at the alias and never consult the table or the map for an aliased import. It now records proven, the name the package declares, beside name, the name the file binds. They differ exactly when an alias renames something. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
An unresolved path came back as "some import paths did not resolve", which says nothing a caller can act on. go list explains itself well - "no required module provides package X; to add it: go get X" - and Names was discarding that for a tidier message. Each unresolved path now carries what go list said about it, flattened onto one line. A path go list did not mention at all gets a stand-in rather than an empty parenthesis. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…piler Both packages read syntax, not builds. An import of another module's internal package formats and resolves like any other, a //go:build line is copied through, and config_linux.go is treated as any other file. Say so, so nobody adds a check the go compiler already makes and makes better. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The options are where a caller needs to see behaviour rather than read about it, and several of them only make sense against each other. Each example prints real formatted output, so godoc shows what the option does to a file: - WithImportGroups: one group per prefix, in the order given; - WithForceImportsPruning: an unused bare import kept, then pruned; - WithResolvedImports: the promise deleting an import the code uses, and the map putting it back while the promise still covers the rest; - WithSimplifiedImportAliases: fmt and strfmt losing their aliases while jsoniter keeps its own; - ImportsReport and HasImportsInDoubt: what Format decided, and what it did not. resolve gains ExampleNames and a package-level Example running the whole loop: format, resolve what the report leaves in doubt, format again with the map, nothing in doubt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi
force-pushed
the
feat/go-formatter
branch
from
August 23, 2026 16:56
6cc993f to
493d0c3
Compare
TestTableIsCurrent compared the table's size with "go list std" and failed on oldstable, where go1.26 reports 179 packages against the 185 the table was built from. The count was never the claim worth checking, and it varies on two axes, not one: the standard library also differs by platform, so runtime/cgo is absent on windows and syscall/js exists only on js/wasm. The table is now the union of linux/amd64, darwin/arm64, windows/amd64 and js/wasm, which makes it a property of the Go release alone. Every package of each of those platforms is in it, and "go generate" on a Mac no longer drops runtime/cgo and churns the file. The generator rejects a path two platforms name differently; none does today. The test, renamed TestTableMatchesToolchain, asserts what matters for correctness: a path both sides hold declares the same name. A path only "go list std" holds means the table is behind, which leaves the formatter guessing rather than wrong, so it is reported instead of failed - except on GeneratedFor, the release recorded in the generated file, where the two must agree exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #33 +/- ##
==========================================
+ Coverage 75.96% 79.69% +3.73%
==========================================
Files 54 79 +25
Lines 3945 4798 +853
==========================================
+ Hits 2997 3824 +827
- Misses 947 973 +26
Partials 1 1 ☔ View full report in Codecov by Harness. |
The Windows runners check out with core.autocrlf=true, so every golden and fixture arrived with CRLF while the formatter emits LF, and every byte-for-byte comparison failed on the line ending alone: TestCorpus against its goldens, and the fragment tests against the space they restore. Nothing committed holds CRLF today, so this only pins how the files are checked out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Two failures, both on bytes that only differ there. isSpaceByte listed ' ', '\t' and '\n', dropping the '\r' that go/format/internal.go carries and documents. cutSpace then read the \r of a \r\n as content, matchSpace found no leading or trailing space to match, and a fragment written on Windows came back with the blank lines and indent around it silently gone. Format now answers go/format.Source byte for byte on \r\n input, for a whole file and for a fragment alike. The new test carries its sources inline: .gitattributes checks every fixture out with LF, so no file on disk can exercise this. dumpedPath scraped the kept file out of a RenderFile error by cutting at the next quote, but the message writes the path with %q, so on Windows every separator arrives escaped and the path came back as D:\\a\\codegen. It reads the quoted string with strconv.QuotedPrefix and strconv.Unquote instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
fredbi
force-pushed
the
feat/go-formatter
branch
from
August 23, 2026 17:59
37676f8 to
cfa022d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist