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

[jippi]

👋 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]

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]

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]

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]

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

[jippi]

The valuer worked for me 👍 though it was a bit tedious having to repeat this code a handful of times

func (d MyType) AsString() string {
	return d.String()
}

That being said, I do understand why not supporting fmt.Stringer directly might be the most compatible/less-magic path - forcing engineers to decide if Expr should see something as a string or not.

Having the Valuer loaded by default (perhaps as a bool option) and clearly calling it out in the docs would have made this easier for me to action and fix out of the box :)

[antonmedv]

Having the Valuer loaded by default (perhaps as a bool option) and clearly calling it out in the docs would have made this easier for me to action and fix out of the box :)

Could you tell what do you miss in the docs?