Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed partial variables extended bug and split PartialVarsEnvExtended test into two #955

Merged
merged 5 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
TristonianJones marked this conversation as resolved.
Show resolved Hide resolved

// 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
}

TristonianJones marked this conversation as resolved.
Show resolved Hide resolved
// 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