Skip to content

Commit

Permalink
Fixed partial variables extended bug and split PartialVarsEnvExtended…
Browse files Browse the repository at this point in the history
… test into two (#955)

Fixed partial vars bug and split tests

---------

Co-authored-by: jduraniglesias <[email protected]>
  • Loading branch information
jduraniglesias and jduraniglesias authored Jun 7, 2024
1 parent 74de262 commit 69a23d9
Show file tree
Hide file tree
Showing 7 changed files with 278 additions and 14 deletions.
73 changes: 73 additions & 0 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,79 @@ func TestResidualAstAttributeQualifiers(t *testing.T) {
}
}

func TestPartialVarsEnv(t *testing.T) {
env := testEnv(t,
Variable("x", IntType),
Variable("y", IntType),
)

// Use env to make sure internals are all initialized.
ast, iss := env.Compile("x == y")

if iss.Err() != nil {
t.Fatalf("env.Compile() failed: %v", iss.Err())
}
prg, err := env.Program(ast, EvalOptions(OptPartialEval))
if err != nil {
t.Fatalf("env.Program() failed: %v", err)
}

act, err := env.PartialVars(map[string]any{"x": 1, "y": 1})

if err != nil {
t.Fatalf("env.PartialVars failed: %v", err)
}
val, _, err := prg.Eval(act)
if err != nil {
t.Fatalf("Eval failed: %v", err)
}

if val != types.True {
t.Fatalf("want: %v, got: %v", types.True, val)
}
}

func TestPartialVarsExtendedEnv(t *testing.T) {
env := testEnv(t,
Variable("x", IntType),
Variable("y", IntType),
)

env.Compile("x == y")
// Now test that a sub environment is correctly copied.
env2, err := env.Extend(Variable("z", IntType))
if err != nil {
t.Fatalf("env.Extend failed: %v", err)
}

ast, iss := env2.Compile("x == y && y == z")

if iss.Err() != nil {
t.Fatalf("env.Compile() failed: %v", iss.Err())
}
prg, err := env2.Program(ast, EvalOptions(OptPartialEval))
if err != nil {
t.Fatalf("env.Program() failed: %v", err)
}

act, err := env2.PartialVars(map[string]any{"z": 1, "y": 1})

if err != nil {
t.Fatalf("env.PartialVars failed: %v", err)
}
val, _, err := prg.Eval(act)
if err != nil {
t.Fatalf("Eval failed: %v", err)
}
if !types.IsUnknown(val) {
t.Fatalf("Wanted unknown, got %v", val)
}

if !reflect.DeepEqual(val, types.NewUnknown(1, types.NewAttributeTrail("x"))) {
t.Fatalf("Wanted Unknown(x (1)), got: %v", val)
}
}

func TestResidualAstModified(t *testing.T) {
env := testEnv(t,
Variable("x", MapType(StringType, IntType)),
Expand Down
8 changes: 2 additions & 6 deletions cel/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,17 +309,13 @@ func (e *Env) Extend(opts ...EnvOption) (*Env, error) {
copy(chkOptsCopy, e.chkOpts)

// Copy the declarations if needed.
varsCopy := []*decls.VariableDecl{}
if chk != nil {
// If the type-checker has already been instantiated, then the e.declarations have been
// validated within the chk instance.
chkOptsCopy = append(chkOptsCopy, checker.ValidatedDeclarations(chk))
} else {
// If the type-checker has not been instantiated, ensure the unvalidated declarations are
// provided to the extended Env instance.
varsCopy = make([]*decls.VariableDecl, len(e.variables))
copy(varsCopy, e.variables)
}
varsCopy := make([]*decls.VariableDecl, len(e.variables))
copy(varsCopy, e.variables)

// Copy macros and program options
macsCopy := make([]parser.Macro, len(e.macros))
Expand Down
43 changes: 36 additions & 7 deletions repl/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,10 @@ type Optioner interface {
// EvaluationContext context for the repl.
// Handles maintaining state for multiple let expressions.
type EvaluationContext struct {
letVars []letVariable
letFns []letFunction
options []Optioner
letVars []letVariable
letFns []letFunction
options []Optioner
enablePartialEval bool
}

func (ctx *EvaluationContext) indexLetVar(name string) int {
Expand Down Expand Up @@ -311,6 +312,7 @@ func (ctx *EvaluationContext) copy() *EvaluationContext {
copy(cpy.letVars, ctx.letVars)
cpy.letFns = make([]letFunction, len(ctx.letFns))
copy(cpy.letFns, ctx.letFns)
cpy.enablePartialEval = ctx.enablePartialEval
return &cpy
}

Expand Down Expand Up @@ -419,12 +421,16 @@ func (ctx *EvaluationContext) addOption(opt Optioner) {

// programOptions generates the program options for planning.
// Assumes context has been planned.
func (ctx *EvaluationContext) programOptions() cel.ProgramOption {
func (ctx *EvaluationContext) programOptions() []cel.ProgramOption {
var fns = make([]*functions.Overload, len(ctx.letFns))
for i, fn := range ctx.letFns {
fns[i] = fn.generateFunction()
}
return cel.Functions(fns...)
result := []cel.ProgramOption{cel.Functions(fns...)}
if ctx.enablePartialEval {
result = append(result, cel.EvalOptions(cel.OptPartialEval))
}
return result
}

// Evaluator provides basic environment for evaluating an expression with
Expand Down Expand Up @@ -489,7 +495,7 @@ func updateContextPlans(ctx *EvaluationContext, env *cel.Env) error {
el.ast = ast
el.resultType = ast.ResultType()

plan, err := env.Program(ast, ctx.programOptions())
plan, err := env.Program(ast, ctx.programOptions()...)
if err != nil {
return err
}
Expand Down Expand Up @@ -579,6 +585,18 @@ func (e *Evaluator) AddOption(opt Optioner) error {
return nil
}

// EnablePartialEval enables the option to allow partial evaluations.
func (e *Evaluator) EnablePartialEval() error {
cpy := e.ctx.copy()
cpy.enablePartialEval = true
err := updateContextPlans(cpy, e.env)
if err != nil {
return err
}
e.ctx = *cpy
return nil
}

// DelLetVar removes a variable from the evaluation context.
// If deleting the variable breaks a later expression, this function will return an error without modifying the context.
func (e *Evaluator) DelLetVar(name string) error {
Expand Down Expand Up @@ -747,6 +765,11 @@ func (e *Evaluator) setOption(args []string) error {
if err != nil {
issues = append(issues, fmt.Sprintf("extension: %v", err))
}
case "--enable_partial_eval":
err := e.EnablePartialEval()
if err != nil {
issues = append(issues, fmt.Sprintf("enable_partial_eval: %v", err))
}
default:
issues = append(issues, fmt.Sprintf("unsupported option '%s'", arg))
}
Expand Down Expand Up @@ -967,7 +990,12 @@ func (e *Evaluator) Process(cmd Cmder) (string, bool, error) {
}
if val != nil {
t := UnparseType(resultT)
unknown, ok := val.Value().(*types.Unknown)
if ok {
return fmt.Sprintf("Unknown %v", unknown), false, nil
}
v, err := ext.FormatString(val, "")

if err != nil {
// Default format if type is unsupported by ext.Strings formatter.
return fmt.Sprintf("%v : %s", val.Value(), t), false, nil
Expand Down Expand Up @@ -1039,11 +1067,12 @@ func (e *Evaluator) Evaluate(expr string) (ref.Val, *exprpb.Type, error) {
return nil, nil, iss.Err()
}

p, err := env.Program(ast, e.ctx.programOptions())
p, err := env.Program(ast, e.ctx.programOptions()...)
if err != nil {
return nil, nil, err
}

act, _ = env.PartialVars(act)
val, _, err := p.Eval(act)
// expression can be well-formed and result in an error
return val, ast.ResultType(), err
Expand Down
76 changes: 75 additions & 1 deletion repl/evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,81 @@ func TestProcess(t *testing.T) {
wantExit: false,
wantError: false,
},

{
name: "OptionEnablePartialEval",
commands: []Cmder{
&simpleCmd{
cmd: "option",
args: []string{
"--enable_partial_eval",
},
},
&letVarCmd{
identifier: "x",
typeHint: mustParseType(t, "int"),
src: "",
},
&letVarCmd{
identifier: "y",
typeHint: mustParseType(t, "int"),
src: "10",
},
&evalCmd{
expr: "x + y > 10 || y > 10",
},
},
wantText: "Unknown x (1)",
wantExit: false,
wantError: false,
},
{
name: "PartialEvalDisabled",
commands: []Cmder{
&letVarCmd{
identifier: "x",
typeHint: mustParseType(t, "int"),
src: "",
},
&letVarCmd{
identifier: "y",
typeHint: mustParseType(t, "int"),
src: "10",
},
&evalCmd{
expr: "x + y > 10 || y > 10",
},
},
wantText: "",
wantExit: false,
wantError: true,
},
{
name: "PartialEvalFiltered",
commands: []Cmder{
&simpleCmd{
cmd: "option",
args: []string{
"--enable_partial_eval",
},
},
&letVarCmd{
identifier: "x",
typeHint: mustParseType(t, "int"),
src: "",
},
&letVarCmd{
identifier: "y",
typeHint: mustParseType(t, "int"),
src: "11",
},
&evalCmd{
expr: "x + y > 10 || y > 10",
},
},
wantText: "true : bool",
wantExit: false,
wantError: false,
},
{
name: "LoadDescriptorsError",
commands: []Cmder{
Expand Down
4 changes: 4 additions & 0 deletions repl/main/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ may take string arguments.
`--extension <extensionType>` enables CEL extensions. Valid options are:
`strings`, `protos`, `math`, `encoders`, `optional`, `bindings`, and `all`.

`--enable_partial_eval` enables partial evaluations

example:

`%option --container 'google.protobuf'`
Expand All @@ -169,6 +171,8 @@ example:

`%option --extension 'all'` (Loads all extensions)

`%option --enable_partial_eval`

#### reset

`%reset` drops all options and let expressions, returning the evaluator to a
Expand Down
Loading

0 comments on commit 69a23d9

Please sign in to comment.