expr-lang / expr

Expression language and expression evaluation for Go
https://expr-lang.org
MIT License
5.85k stars 378 forks source link

Feature Request: Types implementing "fmt.Stringer" should be assignable/comparable to strings #648

Open jippi opened 2 months ago

jippi commented 2 months ago

πŸ‘‹ Hello,

I got the following error with this expression: merge_request.state in ["merged", "closed"]

reflect.Value.MapIndex: value of type gitlab.MergeRequestState is not assignable to type string

It would be very nice if types implementing fmt.Stringer interface to be assignable to strings, or generally treated as strings.

Alternatively, a expr interface types could implement, making them capable of self-casting to various scalar types.

For reference this is the gitlab.MergeRequestState type (used in merge_request.state) implementation

type MergeRequestState string

const (
    // All available
    MergeRequestStateAll MergeRequestState = "all"
    // In closed state
    MergeRequestStateClosed MergeRequestState = "closed"
    // Discussion has been locked
    MergeRequestStateLocked MergeRequestState = "locked"
    // Merge request has been merged
    MergeRequestStateMerged MergeRequestState = "merged"
    // Opened merge request
    MergeRequestStateOpened MergeRequestState = "opened"
)

var AllMergeRequestState = []MergeRequestState{
    MergeRequestStateAll,
    MergeRequestStateClosed,
    MergeRequestStateLocked,
    MergeRequestStateMerged,
    MergeRequestStateOpened,
}

func (e MergeRequestState) IsValid() bool {
    switch e {
    case MergeRequestStateAll, MergeRequestStateClosed, MergeRequestStateLocked, MergeRequestStateMerged, MergeRequestStateOpened:
        return true
    }
    return false
}

func (e MergeRequestState) String() string {
    return string(e)
}
antonmedv commented 1 month ago

Hello πŸ‘‹πŸ»,

This is a good suggestion. I think it was already proposed multiple times. Although I believe this should not be a default behavior, we can create an official patcher (https://github.com/expr-lang/expr/tree/master/patcher) for fmt.Stringer interface.

This new patcher will automatically add .String() method calls in these cases:

Also let's modify string() built in to add support for fmt.Stringer interface. We need this as in some caes Expr will bot be able to detect identifer type and users will need to manualy case to string.

Like in next situation, [foo, bar][0] has any type.

[foo, bar][0] + "sufix"
rrb3942 commented 1 month ago

There was some similar discussion around this for the ValueGetter patcher here: https://github.com/expr-lang/expr/pull/487#discussion_r1414454923

Off the top of my head time.Duration and time.Time both support fmt.Stringer, so automatic conversion might be unwanted in those cases.

jippi commented 1 month ago

being able to pick fmt.Stringer or expr.Stringer (as a ExprString() interface) would be neat to opt in to behavior and differentiate behaviour if desired :)

rrb3942 commented 1 month ago

@jippi You could take a look at https://pkg.go.dev/github.com/expr-lang/expr@v1.16.7/patcher/value and see if that does what you want (specifically implementing https://pkg.go.dev/github.com/expr-lang/expr@v1.16.7/patcher/value#StringValuer).