Skip to content

Commit

Permalink
Update the presence testing fix to be opt-out (#939)
Browse files Browse the repository at this point in the history
* Update the presence testing fix to be opt-out
* Update functional option comments, enable opt-out during conformance runs
* Update workspace to include new optional conformance tests
  • Loading branch information
TristonianJones authored Apr 30, 2024
1 parent 315a5cc commit e2d7340
Show file tree
Hide file tree
Showing 8 changed files with 201 additions and 96 deletions.
2 changes: 1 addition & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ go_repository(
# CEL Spec deps
go_repository(
name = "com_google_cel_spec",
commit = "91f3cb1a7590f594a392b607ae9cab5e969b4cf3",
commit = "8958834564cfc6a4764f76238d03cdbe772665bb",
importpath = "github.com/google/cel-spec",
)

Expand Down
36 changes: 32 additions & 4 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2253,6 +2253,34 @@ func TestOptionalValuesEval(t *testing.T) {
in map[string]any
out any
}{
{
expr: `{}.?invalid`,
out: types.OptionalNone,
},
{
expr: `{'null_field': dyn(null)}.?null_field`,
out: types.OptionalOf(types.NullValue),
},
{
expr: `{'null_field': dyn(null)}.?null_field.?nested`,
out: types.OptionalNone,
},
{
expr: `{'zero_field': dyn(0)}.?zero_field.?invalid`,
out: types.OptionalNone,
},
{
expr: `{0: dyn(0)}[?0].?invalid`,
out: types.OptionalNone,
},
{
expr: `{true: dyn(0)}[?false].?invalid`,
out: types.OptionalNone,
},
{
expr: `{true: dyn(0)}[?true].?invalid`,
out: types.OptionalNone,
},
{
expr: `x.or(y).orValue(z)`,
in: map[string]any{
Expand Down Expand Up @@ -2659,10 +2687,10 @@ func TestOptionalValuesEval(t *testing.T) {
}
}

func TestOptionalValuesEvalNoneIfNull(t *testing.T) {
func TestEnableErrorOnBadPresenceTest(t *testing.T) {
env := testEnv(t,
OptionalTypes(),
OptionalFieldSelectionNoneIfNull(true),
EnableErrorOnBadPresenceTest(true),
)
adapter := env.TypeAdapter()
tests := []struct {
Expand All @@ -2676,11 +2704,11 @@ func TestOptionalValuesEvalNoneIfNull(t *testing.T) {
},
{
expr: `{'null_field': dyn(null)}.?null_field`,
out: types.OptionalNone,
out: types.OptionalOf(types.NullValue),
},
{
expr: `{'null_field': dyn(null)}.?null_field.?nested`,
out: types.OptionalNone,
out: "no such key: nested",
},
{
expr: `{'zero_field': dyn(0)}.?zero_field.?invalid`,
Expand Down
6 changes: 4 additions & 2 deletions cel/library.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,10 @@ func enableOptionalSyntax() EnvOption {
}
}

func OptionalFieldSelectionNoneIfNull(value bool) EnvOption {
return features(featureOptionalFieldSelectionNoneIfNull, value)
// EnableErrorOnBadPresenceTest enables error generation when a presence test or optional field
// selection is performed on a primitive type.
func EnableErrorOnBadPresenceTest(value bool) EnvOption {
return features(featureEnableErrorOnBadPresenceTest, value)
}

func decorateOptionalOr(i interpreter.Interpretable) (interpreter.Interpretable, error) {
Expand Down
5 changes: 3 additions & 2 deletions cel/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ const (
// expressions occur: e.g. a && b && c && d -> call(_&&_, [a, b, c, d])
featureVariadicLogicalASTs

// Enable optional field selection to treat null values as optional.none()
featureOptionalFieldSelectionNoneIfNull
// Enable error generation when a presence test or optional field selection is
// performed on a primitive type.
featureEnableErrorOnBadPresenceTest
)

// EnvOption is a functional interface for configuring the environment.
Expand Down
2 changes: 1 addition & 1 deletion cel/program.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func newProgram(e *Env, a *Ast, opts []ProgramOption) (Program, error) {
// Set the attribute factory after the options have been set.
var attrFactory interpreter.AttributeFactory
attrFactorOpts := []interpreter.AttrFactoryOption{
interpreter.OptionalFieldSelectionNoneIfNull(p.HasFeature(featureOptionalFieldSelectionNoneIfNull)),
interpreter.EnableErrorOnBadPresenceTest(p.HasFeature(featureEnableErrorOnBadPresenceTest)),
}
if p.evalOpts&OptPartialEval == OptPartialEval {
attrFactory = interpreter.NewPartialAttributeFactory(e.Container, e.adapter, e.provider, attrFactorOpts...)
Expand Down
Loading

0 comments on commit e2d7340

Please sign in to comment.