Skip to content

Attribute Go template output against shipped compiler export data - #8545

Merged
knutwannheden merged 4 commits into
mainfrom
explore-shipped-export-data-for-go-templates
Aug 21, 2026
Merged

knutwannheden merged 4 commits into
mainfrom
explore-shipped-export-data-for-go-templates

Conversation

@knutwannheden

@knutwannheden knutwannheden commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

A Go template is parsed by building a synthetic file and type-checking it. That file belongs to no module, so the parser uses importer.Default(), which resolves the stdlib and nothing else. ExpressionTemplate("errors.Wrap(err, msg)").Imports("github.com/pkg/errors") yields a MethodInvocation with MethodType == nilmatcher.IsResolved false — and every recipe that later reads that output sees an unattributed tree. The packages this hurts most are the ones a build machine often cannot fetch at all: encoding/json/v2 and encoding/json/jsontext are behind GOEXPERIMENT=jsonv2, and a machine with no module proxy access cannot resolve a third-party path either.

A recipe module can now carry the compiler export data for the packages its templates import:

go run .../cmd/goexportdata -o internal/exportdata encoding/json/v2 encoding/json/jsontext
template.WithAfter(fmt.Sprintf(`json.Marshal(%s, jsontext.WithIndent(%s))`, v, ind),
    template.Imports("encoding/json/v2", "encoding/json/jsontext"),
    template.ExportData(jsonv2exportdata.FS),
    template.SourceImports("encoding/json/v2", "encoding/json/jsontext"))

Against those blobs jsontext.WithIndent(" ") maps to a resolved MethodType with parameter types and a return type of encoding/json/internal/jsonopts.Options; without them it is MethodType == nil. No network and no go list at type-check time.

How the blobs are read

go list -export emits a full .a archive including object code. Keeping only its __.PKGDEF member takes encoding/json/v2 from 4.7 MB to 170 KB and github.com/pkg/errors from 205 KB to 10.6 KB; unpacked this would not be shippable. Blobs are not pinned to the machine that produced them — I rewrote the recorded GOOS/GOARCH to linux amd64 and the version to go1.19 and go1.99, and all still imported, because only the binary export format is checked on read.

Two findings shaped the implementation. A non-nil lookup takes go/importer off its default search entirely — with a blob-only lookup, strings.Contains went unresolved — so Importer must chain to importer.Default(), not replace it. And a package's own dependencies need no blobs of their own: importing a blob whose API mentions time.Time asked the lookup for exactly one path, and time.Time.Year() still resolved.

Why ExportData is per-template, not per-recipe

The obvious shape is one recipe-level option covering before and after. That is wrong here: patternComparator.matchProperties compares Identifier.Name and MethodInvocation select/name/arguments and reads no types at all, so the two sides answer to different things. On WithAfter export data decides the types the emitted code carries, which is what lets RemoveUnusedImports tell a superseded import from a live one. On WithBefore it only bears on shape:

attributed no types
generic type atomic.Pointer[int] ParameterizedType ParameterizedType
generic func Map[int](1) MethodInvocation{TypeParameters} MethodInvocation{Select: ArrayAccess}

So it belongs on a before-pattern only when the source is parsed the same way. It is a BeforeOption on both, doing different work on each.

Sets are unioned into one go/importer rather than one importer per set, so a package two sets both reference resolves to a single type instead of a copy per set.

Decoding export data dominates template application, so each template builds one importer and reuses it across every Apply: 788,804 → 20,642 ns/op against a 99 KB blob, of which the decode itself is 768,596 ns cold and 50.84 ns reused. That importer is shared, so it is mutex-guarded and TestTemplateIsSafeToApplyConcurrently runs under -race. Note this also speeds up templates that ship no export data at all (463,360 → 89,812 ns/op), because Apply previously rebuilt importer.Default() every call — a behaviour change outside this feature's lane, called out here rather than buried.

Tests

TestExportDataGivesTemplateOutputRealSignatures pins resolved parameter and return types; TestUnreadableExportDataDegradesToNoExportData pins that a stale blob lands exactly where no blob does. TestExportDataIsDecodedOncePerTemplate counts blob opens through a wrapping fs.FS and would catch a regression that reintroduces per-Apply decoding. TestGenerateKeepsTheOldBlobsWhenAPathFails covers regeneration atomicity, and TestGenerateIgnoresProgressOnStderr uses a go shim on PATH that writes to stderr first — without it, go list progress output is concatenated with the archive path on any cold module cache, which is the normal case for the machine that generates blobs. Fixtures are compiled at test time rather than committed, so they always match the running toolchain.

What this does not fix

A SourceImports package swap under a preserved qualifier needs the after-template attributed. Where it is not — no export data, or a blob this toolchain cannot read — both imports are emitted and bind the same name, and the file does not compile. TestSwapWithoutAttributionLeavesBothImports pins both routes to that state. The same output appears when any un-rewritten reference to the old package survives, since a template rewrites one expression while the swap is a whole-file decision; TestSwapLeavesBothImportsWhenACallIsNotRewritten covers that. Only a recipe with established whole-file coverage may swap this way, and SourceImports says so.

Two fixes outside this feature are folded in because a review of this branch surfaced them and they are two lines each: MatchResult.Get treats a typed-nil binding as unbound, since binding the nil result of a helper returning a concrete type leaves a non-nil interface that substitution dereferences into a panic (TestInstantiateTreatsATypedNilBindingAsUnbound), and instantiate_test.go gets the copyright year every other file in the change carries. Say the word if you would rather they went separately.

Three fixes this work surfaced were split out and have already landed on their own: PackageName for semantically versioned import paths (#8548), gofmt group-separator placement in import add/remove (#8549), and the qualifier fallback yielding to type attribution (#8555). This branch is now only the feature.

@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Aug 19, 2026
@knutwannheden
knutwannheden force-pushed the explore-shipped-export-data-for-go-templates branch 2 times, most recently from 4426cb9 to 7046bc5 Compare August 19, 2026 09:53
A template is type-checked as a file belonging to no module, against
whatever importer.Default() can load, so anything outside the stdlib
resolves to nothing and the emitted tree carries no types.

A recipe module can now carry the export data for the packages its
templates import. cmd/goexportdata generates it, keeping only the
__.PKGDEF member so a blob is a few percent of the archive go build
leaves behind (encoding/json/v2: 4.7 MB -> 170 KB), and emits an
embed.FS shim. pkg/exportdata resolves from those blobs first and falls
back to the toolchain, so a blob a newer toolchain cannot read costs
attribution and nothing else.

Each template builds one importer and reuses it across every Apply.
Decoding dominated the call before: Apply against a 99 KB blob was
788,804 ns/op, of which 768,596 was the decode, and is now 20,642.

RemoveUnusedImports kept a superseded import alive whenever another
import bound the same qualifier, which is the shape a major-version
package move takes once both sides are attributed. The qualifier
fallback now yields to attribution where attribution has accounted for
that name, and still keeps everything where nothing is attributed.
@knutwannheden
knutwannheden force-pushed the explore-shipped-export-data-for-go-templates branch from 7046bc5 to 63cd778 Compare August 19, 2026 10:37
…t-data-for-go-templates

# Conflicts:
#	rewrite-go/pkg/recipe/golang/internal/imports.go
#	rewrite-go/pkg/recipe/golang/internal/imports_test.go
@knutwannheden
knutwannheden force-pushed the explore-shipped-export-data-for-go-templates branch from ab75715 to d7f256b Compare August 20, 2026 11:10
@knutwannheden
knutwannheden marked this pull request as ready for review August 20, 2026 11:19
@knutwannheden
knutwannheden force-pushed the explore-shipped-export-data-for-go-templates branch from 010bf12 to 87bfe5f Compare August 21, 2026 12:59
@knutwannheden
knutwannheden merged commit 45438c1 into main Aug 21, 2026
1 check passed
@knutwannheden
knutwannheden deleted the explore-shipped-export-data-for-go-templates branch August 21, 2026 13:13
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant