Allow overriding singletons (add int + custom type)

[theoilie]

First of all, thanks for all the hard work on this project. It's working great and being very flexible up to this point which is admittedly stretching somewhat far.

Feature request checklist

• There are no issues that match the desired change
• The change is large enough it can't be addressed with a simple Pull Request
• If this is a bug, please file a Bug Report.

Change
Following up from this comment, I'm wondering if there's currently a way to override singletons. Specifically, I want to override addition of an int type with a custom fixed-point decimal type (a simple wrapper over https://github.com/cockroachdb/apd).

Example
Create env options with the custom type:

import (
	"fmt"
	"strconv"

	"github.com/cockroachdb/apd"
	"github.com/google/cel-go/cel"
	"github.com/google/cel-go/common/types"
)

type Decimal struct {
	*apd.Decimal
}

var _ traits.Adder = &Decimal{}
var _ ref.Val = &Decimal{}

var TypeDecimal = cel.ObjectType("Decimal", traits.AdderType)

func (d *Decimal) Add(other ref.Val) ref.Val {
	right, err := toDecimal(other)
	if err != nil {
		return types.NewErr(err.Error())
	}
	result := new(apd.Decimal)
	_, err = Context.Add(result, d.Decimal, right.Decimal)
	if err != nil {
		return types.NewErr(err.Error())
	}
	decRes, err := NewDecimal(result)
	if err != nil {
		return types.NewErr(err.Error())
	}
	return decRes
}

...

// Create an env with these options
opts := []cel.EnvOption{
		cel.Types(TypeDecimal),
		cel.Function("decimal",
			cel.Overload("decimal_string", []*cel.Type{cel.StringType}, TypeDecimal,
				cel.UnaryBinding(func(arg ref.Val) ref.Val {
					if str, ok := arg.(types.String); ok {
						d, err := NewDecimal(string(str))
						if err != nil {
							return types.NewErr(err.Error())
						}
						return d
					}
					return types.NewErr("invalid argument to decimal()")
				}),
			),
		cel.Function("_+_", cel.Overload("_+_decimal", []*cel.Type{TypeDecimal, TypeDecimal}, TypeDecimal)),
		cel.Function("_+_", cel.Overload("_+_decimal_int", []*cel.Type{TypeDecimal, cel.IntType}, TypeDecimal)),

        // PROBLEM IS HERE: This is the one that doesn't work because it just calls Add() on cel.IntType, which doesn't know anything about our custom TypeDecimal
		cel.Function("_+_", cel.Overload("_+_int_decimal", []*cel.Type{cel.IntType, TypeDecimal}, TypeDecimal)),
	}

Try to do addition. The final example is the one that doesn't work due to no such overload which I believe comes from the int type's Add() function.

func TestCustomDecimal(t *testing.T) {
	dollarToCents := ...

	tests := []struct {
		name     string
		expr     string
		expected int64
	}{
		{"Decimal + Decimal", `decimal("0.20") + decimal("0.30")`, 50},
		{"Decimal + Int", `decimal("0.75") + 1`, 175},
		{"Int + Decimal", `2 + decimal("0.75")`, 275}, // This is the example that fails due to "no such overload" while the others succeed
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			cents, err := dollarToCents.eval(tt.expr)
			if err != nil {
				t.Fatalf("failed to evaluate expression: %v", err)
			}

			assert.Equal(t, tt.expected, cents, "for expression: %s", tt.expr)
		})
	}
}

Alternatives considered
I'm not aware of any other way to allow for syntax like 1 + decimal("1.0") unless the + operator on the Int type is overridden. For my use case unfortunately it's not good enough to be able to only do decimal("1.0") + 1 without allowing the other way around.

[theoilie]

Would a solution be to allow overriding a singleton as long as AnyType isn't used? As long as the types are narrow (only int+decimal, not int+any), perhaps something like this could work without unintended side effects if it were allowed? (It's disallowed with the error singleton function incompatible with specialized overloads)

cel.Function("_+_", cel.Overload("_+_decimal", []*cel.Type{TypeDecimal, TypeDecimal}, TypeDecimal),
	cel.Overload("_+_decimal_int", []*cel.Type{TypeDecimal, cel.IntType}, TypeDecimal,
		cel.BinaryBinding(func(lhs, rhs ref.Val) ref.Val {
			left, err := toDecimal(lhs)
			if err != nil {
				return types.NewErr(err.Error())
			}
			right, err := toDecimal(rhs)
			if err != nil {
				return types.NewErr(err.Error())
			}
			return left.Add(right)
		}),
	),
	cel.Overload("_+_int_decimal", []*cel.Type{cel.IntType, TypeDecimal}, TypeDecimal,
		cel.BinaryBinding(func(lhs, rhs ref.Val) ref.Val {
			left, err := toDecimal(lhs)
			if err != nil {
				return types.NewErr(err.Error())
			}
			right, err := toDecimal(rhs)
			if err != nil {
				return types.NewErr(err.Error())
			}
			return left.Add(right)
		}),
	),
),

[TristonianJones]

Hi @theoilie if Decimal type supports the traits.Adder interface, then you just need to add the overload signature (no functional bindings) and the singleton will defer to the implementation of Add supported by your type. Give common/types/int.go a look to see how CEL does this today. (Looks like you got the right idea)

In terms of cross type support you'll need to type convert to decimal from int, decimal(1)

[theoilie]

Hi @theoilie if Decimal type supports the traits.Adder interface, then you just need to add the overload signature (no functional bindings) and the singleton will defer to the implementation of Add supported by your type. Give common/types/int.go a look to see how CEL does this today. (Looks like you got the right idea)

In terms of cross type support you'll need to type convert to decimal from int, decimal(1)

Hi @TristonianJones, thanks for the speedy response! To clarify, this custom Decimal type supports the traits.Adder interface, so adding Decimal + Decimal or Decimal + Int is no problem. Where I'm struggling is adding Int + Decimal without type conversions.

In other words, on my Decimal type I implemented Add(other ref.Val) ref.Val to work when other is another Decimal or a CEL Int. The problem is that on CEL's built-in Int type, this Add(other ref.Val) function doesn't support when other is a custom Decimal type. I understand you're saying to convert int to decimal, but my aim is to allow non-technical users to define an expression like x + y without making them think about types. Currently this works when x is a Decimal and y is an Int, but not vice-versa. So I'm wondering if there's already a way to do this, or if maybe one solution would be allowing me to override Int's Add(other ref.Val) only when other is of this custom Decimal type, and leave it as the default implementation otherwise? That's the proposal I was trying to convey with the functional bindings above.

[TristonianJones]

@theoilie CEL doesn't permit type coercion by design, which means that all type conversions are intended to be explicit. CEL will let you do custom additions with custom types, but not across types. The intent is to make it clear that operators should always use CEL semantics, but custom functions may have their own semantics.

For example, you can easily support the addition of two Decimal values and type converters from decimal(string), decimal(int), etc. If you want to add across types, then you'd need a custom function, e.g. decimal("10.0").add(1) It's not pretty, but it's also very clear that the operation is doing something other than what CEL would do.

Does that make sense?

-Tristan

[theoilie]

@theoilie CEL doesn't permit type coercion by design, which means that all type conversions are intended to be explicit. CEL will let you do custom additions with custom types, but not across types. The intent is to make it clear that operators should always use CEL semantics, but custom functions may have their own semantics.

For example, you can easily support the addition of two Decimal values and type converters from decimal(string), decimal(int), etc. If you want to add across types, then you'd need a custom function, e.g. decimal("10.0").add(1) It's not pretty, but it's also very clear that the operation is doing something other than what CEL would do.

Does that make sense?

-Tristan

Thanks for the clarification. That all makes sense, and I'll stick to using explicit conversions / custom functions. For readability of non-technical users, I was hoping to have all ints and doubles implicitly call / be replaced with decimal(int) and decimal(double), respectively, but I ran into some issues trying to wrestle a custom adapter and decorator into accomplishing this.

I can respect that adding a configuration option for this request might go against the explicit syntax philosophy of the project, so I'll work around it differently for now and can close out this issue. Thanks again for the speedy support!

[TristonianJones]

You're welcome. Thanks for reaching out!