Adapt the type-assertion recipes to Go.TypeAssertion - #64
Merged
Conversation
openrewrite/rewrite#8517 models `x.(T)` as `Go.TypeAssertion` instead of `J.TypeCast`, so the four recipes that reached for a type assertion stopped matching and made no change. The same PR emits an `if` init clause's `;` from a `Semicolon` marker, which a recipe that synthesizes such an `if` has to attach itself; without it `CheckTemplateExecuteError` printed `if err := f() err != nil`. This requires `rewrite-go/v0.0.30`, which is not yet published.
knutwannheden
force-pushed
the
lucky-ferret
branch
from
August 17, 2026 09:35
4dbc158 to
00317fb
Compare
knutwannheden
marked this pull request as ready for review
August 18, 2026 10:16
…ewrites `go.sum` gains the entry for the published `rewrite-go/v0.0.30`. That release also carries openrewrite/rewrite#8521, which makes `RemoveImport` keep an import the file still references. `RelocateRawMessage` rewrites the only `json.RawMessage` in a file to `jsontext.Value` and then drops the import, but the field, the type expression and the conversion call all still carried the `encoding/json` attribution they were given at parse. The reference check read that as a live use and kept the import, so the file came out importing both packages. The rewritten nodes now carry `encoding/json/jsontext.Value`, which is what the source says once the rewrite has run, and the conversion drops a method type that no longer describes the call it is attached to. The import goes because nothing claims to use it.
knutwannheden
force-pushed
the
lucky-ferret
branch
2 times, most recently
from
August 18, 2026 10:36
a408833 to
1520507
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
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.
Motivation
openrewrite/rewrite#8517 hardens the Go parser and printer, and two of its changes are visible to recipes.
x.(T)is modeled asGo.TypeAssertionrather thanJ.TypeCast, because a prefix cast has nowhere to hold what stands between the expression and the dot. And anif/switchinit clause's;is emitted from aSemicolonmarker, since a line break can stand in for the written;.Every recipe that reached for a type assertion silently stopped matching, and the one recipe that synthesizes an if-init printed invalid Go. Against
rewrite-goat that commit, five tests fail; against the published v0.0.29 and against the commit right before #8517 they pass, so #8517 is the whole of it.Examples
The two shapes a recipe now works with:
Summary
RemoveRedundantInterfaceAssertion,UseCommaOkTypeAssertionandUseErrorsAsmatch*golang.TypeAssertion, reading the asserted expression fromLeft.Elementand the type fromAssertedType.RemoveRedundantSprintfcarries the prefix over to a*golang.TypeAssertionargument, the kindfmt.Sprintf("%s", x.(string))presents.CheckTemplateExecuteErrorattaches aSemicolonmarker to the init clause of theifit builds.rewrite-go/v0.0.30.Test plan
go test ./... -count=1green across all ten packages againstrewrite-goat openrewrite/rewrite@f97e158cfc (via a localreplace).rewrite-go/v0.0.30: green across all eleven packages.One more break, from a later commit in the same release
v0.0.30was tagged atcad175a62, fiverewrite-gocommits past the one this branch was verified against, and one of them broke twoencoding/json/v2tests. openrewrite/rewrite#8521 makesRemoveImportkeep an import the file still references.The check was right and the tree was wrong.
RelocateRawMessagerewrites the onlyjson.RawMessagein a file tojsontext.Value, but the nodes kept the attribution they were given at parse:So the reference check saw a live use of
encoding/jsonand correctly declined to remove the import, and the file came out importing both packages. SettingForcewould have silenced that by switching off exactly the safety the flag exists to preserve.Instead the rewrite now says what it means: the type expression and the field declaration carry
encoding/json/jsontext.Value, and the conversion call drops a method type that no longer describes it. The import is then removed by the default, unforced path, because nothing claims to use it.Worth carrying forward: a recipe that rewrites away the last use of a package has to re-attribute the nodes it touched, or the import survives. Nil-ing the type on the node being replaced is not enough — the enclosing declaration holds one too.
Test plan
go test ./... -count=1green across all ten packages againstrewrite-goat openrewrite/rewrite@f97e158cfc (via a localreplace).rewrite-go/v0.0.30: green across all eleven packages.One more break, from a later commit in the same release
v0.0.30was tagged atcad175a62, fiverewrite-gocommits past the one this branch was verified against, and one of them broke twoencoding/json/v2tests. openrewrite/rewrite#8521 madeRemoveImportconditional: an import the file still references survives unlessForceis set.RelocateRawMessagerewrites the onlyjson.RawMessagein a file tojsontext.Valueand then drops the now-dead import. It nils the type on the nodes it rewrites, but the struct field it rewrote keeps its attributedencoding/json.RawMessagetype, and the reference check reads that as a live use — so the import stayed and the file came out with bothencoding/jsonandencoding/json/jsontext.The recipe already runs a scan that establishes
RawMessageis the only usage; that scan is what decides the import is dead, so the removal now says so withForce: true. Worth noting the general shape: after any rewrite the LST's type attribution is stale, so a reference check driven by types will over-report a package as used. Recipes that rewrite away the last use of an import and know it will needForce.