NativeTypes doesn't use baseAdapter when it should

[gudvinr]

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Check which components this affects:

• parser
• checker
• interpreter

Sample expression and input that reproduces the issue:

x == "abc"

Test setup:

//
// setup [ref.Val] wrapper
//

type bogus struct {
    bogus string
}

var BogusType = cel.OpaqueType("bogus.Wrapper")

type wrapper bogus

var _ ref.Val = wrapper{}

func (w wrapper) ConvertToNative(reflect.Type) (any, error)
func (w wrapper) ConvertToType(ref.Type) ref.Val
func (w wrapper) Equal(ref.Val) ref.Val

func (w wrapper) Type() ref.Type { return BogusType }
func (w wrapper) Value() any { return bogus(w) }

//
// setup [types.Adapter]
//

type bogusAdapter struct {
    baseAdapter types.Adapter
}

func (ba *bogusAdapter) NativeToValue(value any) ref.Val {
    switch v := value.(type) {
    case wrapper: return v
    case bogus: return wrapper(v)
    }

    return ba.baseAdapter.NativeToValue(value)
}

func bogusAdapterOption(env *cel.Env) (*cel.Env, error) {
    adapter := bogusAdapter{baseAdapter: env.CELTypeAdapter()},
    return env.Extend(cel.CustomTypeAdapter(&adapter))
}

//
// setup CEL environment
//

env := cel.Env(
    bogusAdapterOption,
    ext.NativeTypes(...),
)

Expected behavior

I am not sure what'd be the solution though. This very well may be the intended behaviour but took me a while to get to the bottom of it. But it doesn't seem like it. ext.NativeTypes accepts reflect.Type and proto descriptors which are getting registered explicitly to nativeTypeProvider. So my assumption was that only registered types will be converted through NativeToValue

Additional context

nativeTypeProvider treats all structures as native types and uses newNativeObject. Thus bogusAdapter.NativeTypes not being called at all.

If you define type as type wrapper string then it should (I think) fall to baseAdapter which makes this even more confusing.

When you swap EnvOption order, it does work.

[TristonianJones]

Hi @gudvinr, NativeTypeProvider attempts to compose over prior adapters. If you change the order, the new adapter just clears the NativeTypeProvider completely.

The solution is likely to copy a similar check from NewValue into the NativeToValue method:

https://github.com/google/cel-go/blob/14f6746a286f198c2a976749331b7bf4aa2d4bd2/ext/native.go#L332-L336

The NativeToValue was assuming that all Go-native types would be managed through the NativeTypeProvider and any primitive or protobuf-types would fallback to the default types.Registry implementation. As CEL and its applications have become more diverse, these aren't the only scenarios any more.

[gudvinr]

If you change the order, the new adapter just clears the NativeTypeProvider completely.

Not really, bogusAdapterOption from the example gets previous adapter from cel.Env and passes everything unhandled there. This is similar to what ext.NativeTypes itself does.

In this case it'll be nativeTypeProvider.

Well at least it should as far as I can see.