Go: a recipe option binds to its field's declared type

[knutwannheden]

Setting any non-string option on any Go recipe from the Moderne CLI fails outright. mod run . --recipe=org.openrewrite.golang.AddImport -PpackagePath=strings -PonlyIfReferenced=false dies with Internal error: reflect.Set: value of type string is not assignable to type bool, for both true and false. Since -P is the only way to set recipe options from the CLI, every non-string option on every Go recipe is currently unreachable.

The mechanism

newReflectConstructor bound options onto the recipe struct with no conversion at all:

f := elem.FieldByName(fieldName)
if f.IsValid() && f.CanSet() {
    f.Set(reflect.ValueOf(val))   // dynamic type must already be assignable
}

val is whatever encoding/json decoded into map[string]any. The CLI declares -P as picocli Map<String, Object>, so onlyIfReferenced=false parses to the Java String "false", serialises as a JSON string, and lands as a Go string against a bool field. Fixing only string→bool would not have been enough: a JSON number decodes to float64, so an int field failed identically even for a caller sending a correctly typed number.

The fix

Values are coerced to the field's declared type before being set. The reference for what a recipe option may be written as is Jackson, not RecipeIntrospectionUtils.convert — convert only handles String targets and string→enum, and passes "false" straight through to a boolean parameter. Java tolerates the flag because constructRecipe throws and RecipeLoader falls back to mapper.convertValue. The closest in-repo peer is C# RewriteRpcServer.ConvertOptionValue, which deserialises the fragment into the declared property type.

Two further defects surfaced while matching that policy:

PrepareRecipe now decodes with UseNumber, as pkg/rpc/rpc_object_data.go already does. Without it an integer option past 2^53 is silently corrupted before the binder sees it — 9007199254740993 bound as 9007199254740992 with no error, since a truncation check only rejects non-integral floats.

Option names resolve to fields case-insensitively when the capitalised spelling misses, so url reaches a URL field as it does on the Java host (ACCEPT_CASE_INSENSITIVE_PROPERTIES) and in C# (BindingFlags.IgnoreCase). Previously it produced Url, missed, and dropped the value silently, leaving the recipe running with a zero-valued option.

An option naming no field stays ignored, matching Java (Jackson with FAIL_ON_UNKNOWN_PROPERTIES disabled) and C#. Python is the only peer that errors, and only as a side effect of recipe_class(**options).

Breaking change

RecipeConstructor gains an error return, going from func(map[string]any) Recipe to func(map[string]any) (Recipe, error). There is no additive way to do this in Go, and no error channel means no way to name the failing option — today it is a recovered panic reported as -32603 Internal error with no mention of which recipe or option. It is now an OptionBindError naming the recipe, option, declared type and value, returned as -32602.

The module is v0.0.31, pre-1.0. The only external consumer is moderneinc/recipes-go, which calls Constructor in exactly two test files, both passing nil options and so incapable of producing a bind error. The alternative — panicking with a typed error and recovering at the RPC boundary — would preserve compatibility but keeps the broken contract alive for anyone still calling the old signature, and uses panic as control flow for a user typing a bad flag value.

Tests

GolangRecipeIntegTest.booleanRecipeOptionArrivingAsAString runs the wire path the CLI uses, asserting that onlyIfReferenced bound from a JSON string takes effect in both directions. It is mutation-checked: with the coercion reverted it reproduces the reported message verbatim, reflect.Set: value of type string is not assignable to type bool. TestPrepareRecipeBindsOptionsFromWireJSON is likewise mutation-checked against the UseNumber decode, failing with expected 9007199254740993, actual 9007199254740992.

pkg/recipe/options_test.go covers the conversion matrix directly: string↔bool both ways, float64/string/json.Number to int and uint, width overflow, non-integral and out-of-int64-range floats, pointer and slice options, and unconvertible values.

Scope

pkg/visitor/init.go looked like it needed the same treatment and does not — both f.Set(reflect.ValueOf(v)) calls assign the visitor pointer to its Self field for virtual dispatch and never see wire data. VisitorOptions in cmd/rpc/main.go is decoded and then read nowhere in the repo.

Whether the CLI should also coerce Java-side, where it does know the declared type from the option descriptors, is left open. The Go binder has to be robust against whatever any RPC client sends either way.

The two recipes-go test call sites are tracked separately and can only be adapted after this lands, since the fix does not compile against the rewrite-go that repo currently resolves.