expr-lang/expr

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

Opened this issue · 4 comments

jippi commented

👋 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

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:

  • All binary nodes:
    • foo + bar
    • foo in [x, y, z]
    • foo matches bar
  • Function calls in string params:
    • func(foo)
  • Anything else?

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

There was some similar discussion around this for the ValueGetter patcher here: #487 (comment)

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

  • 👍1
jippi commented

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 :)