google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.31k stars 210 forks source link

Integer token should be valid Starlark.Float #461

Closed gbouv closed 1 year ago

gbouv commented 1 year ago

I have a custom builtin which expects a float argument. When I write my Starlark, I would like to be able to input a simple integer. Sometimes, I don't care about digits after the decimal point, and I would expect the interpreter to safely assume 50=50.0. Currently, the interpreter crashes with "got int, want float".

Simple repro:

func main() {
    starlarkCode := `result = custom_builtin(float_arg=50)`
    thread := &starlark.Thread{
        Name: "test",
    }
    predeclared := starlark.StringDict{
        "custom_builtin": starlark.NewBuiltin("custom_builtin", customBuiltin),
    }
    globals, err := starlark.ExecFile(thread, "", starlarkCode, predeclared)
    if err != nil {
        panic(err)
    }
    fmt.Println(globals["result"])
}

func customBuiltin(_ *starlark.Thread, fn *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
    var floatArg starlark.Float
    if err := starlark.UnpackArgs(fn.Name(), args, kwargs, "float_arg", &floatArg); err != nil {
        return nil, err
    }
    return floatArg, nil
}

The above works fine when float_arg=50.0 but throws an error when float_arg=50, which is a bit misleading

Does that look like a reasonable change? I can try to look into it. I looked at UnpackArgs as well as the Float and Int but I am not sure what could be changed to allow this. Do you have some guidance?

adonovan commented 1 year ago

int and float are distinct types in Starlark, so this is working as intended. If you want your function to accept either type, then use var x any; UnpackArgs(..., &x) and then do a type-switch on x. Or define a custom IntOrFloat type that defines the Unpack method and pass a variable of that type to UnpackArgs.