diff --git a/ast/builtins.go b/ast/builtins.go index 86847ec73a..cbfe54783a 100644 --- a/ast/builtins.go +++ b/ast/builtins.go @@ -663,36 +663,6 @@ var Min = &Builtin{ ), } -// All takes a list and returns true if all of the items -// are true. A collection of length 0 returns true. -var All = &Builtin{ - Name: "all", - Decl: types.NewFunction( - types.Args( - types.NewAny( - types.NewSet(types.A), - types.NewArray(nil, types.A), - ), - ), - types.B, - ), -} - -// Any takes a collection and returns true if any of the items -// is true. A collection of length 0 returns false. -var Any = &Builtin{ - Name: "any", - Decl: types.NewFunction( - types.Args( - types.NewAny( - types.NewSet(types.A), - types.NewArray(nil, types.A), - ), - ), - types.B, - ), -} - /** * Arrays */ @@ -2516,13 +2486,51 @@ var RegexMatchDeprecated = &Builtin{ ), } +// All takes a list and returns true if all of the items +// are true. A collection of length 0 returns true. +var All = &Builtin{ + Name: "all", + Decl: types.NewFunction( + types.Args( + types.NewAny( + types.NewSet(types.A), + types.NewArray(nil, types.A), + ), + ), + types.B, + ), + deprecated: true, +} + +// Any takes a collection and returns true if any of the items +// is true. A collection of length 0 returns false. +var Any = &Builtin{ + Name: "any", + Decl: types.NewFunction( + types.Args( + types.NewAny( + types.NewSet(types.A), + types.NewArray(nil, types.A), + ), + ), + types.B, + ), + deprecated: true, +} + // Builtin represents a built-in function supported by OPA. Every built-in // function is uniquely identified by a name. type Builtin struct { - Name string `json:"name"` // Unique name of built-in function, e.g., (arg1,arg2,...,argN) - Decl *types.Function `json:"decl"` // Built-in function type declaration. - Infix string `json:"infix,omitempty"` // Unique name of infix operator. Default should be unset. - Relation bool `json:"relation,omitempty"` // Indicates if the built-in acts as a relation. + Name string `json:"name"` // Unique name of built-in function, e.g., (arg1,arg2,...,argN) + Decl *types.Function `json:"decl"` // Built-in function type declaration. + Infix string `json:"infix,omitempty"` // Unique name of infix operator. Default should be unset. + Relation bool `json:"relation,omitempty"` // Indicates if the built-in acts as a relation. + deprecated bool // Indicates if the built-in has been deprecated. +} + +// IsDeprecated returns true if the Builtin function is deprecated and will be removed in a future release. +func (b *Builtin) IsDeprecated() bool { + return b.deprecated } // Expr creates a new expression for the built-in with the given operands. diff --git a/ast/compile.go b/ast/compile.go index 86cb55d7e9..f05991e108 100644 --- a/ast/compile.go +++ b/ast/compile.go @@ -103,6 +103,7 @@ type Compiler struct { builtins map[string]*Builtin // universe of built-in functions customBuiltins map[string]*Builtin // user-supplied custom built-in functions (deprecated: use capabilities) unsafeBuiltinsMap map[string]struct{} // user-supplied set of unsafe built-ins functions to block (deprecated: use capabilities) + deprecatedBuiltinsMap map[string]struct{} // set of deprecated, but not removed, built-in functions enablePrintStatements bool // indicates if print statements should be elided (default) comprehensionIndices map[*Term]*ComprehensionIndex // comprehension key index initialized bool // indicates if init() has been called @@ -240,11 +241,12 @@ func NewCompiler() *Compiler { }, func(x util.T) int { return x.(Ref).Hash() }), - maxErrs: CompileErrorLimitDefault, - after: map[string][]CompilerStageDefinition{}, - unsafeBuiltinsMap: map[string]struct{}{}, - comprehensionIndices: map[*Term]*ComprehensionIndex{}, - debug: debug.Discard(), + maxErrs: CompileErrorLimitDefault, + after: map[string][]CompilerStageDefinition{}, + unsafeBuiltinsMap: map[string]struct{}{}, + deprecatedBuiltinsMap: map[string]struct{}{}, + comprehensionIndices: map[*Term]*ComprehensionIndex{}, + debug: debug.Discard(), } c.ModuleTree = NewModuleTree(nil) @@ -284,6 +286,7 @@ func NewCompiler() *Compiler { {"CheckRecursion", "compile_stage_check_recursion", c.checkRecursion}, {"CheckTypes", "compile_stage_check_types", c.checkTypes}, {"CheckUnsafeBuiltins", "compile_state_check_unsafe_builtins", c.checkUnsafeBuiltins}, + {"CheckDeprecatedBuiltins", "compile_state_check_deprecated_builtins", c.checkDeprecatedBuiltins}, {"BuildRuleIndices", "compile_stage_rebuild_indices", c.buildRuleIndices}, {"BuildComprehensionIndices", "compile_stage_rebuild_comprehension_indices", c.buildComprehensionIndices}, } @@ -1173,6 +1176,15 @@ func (c *Compiler) checkUnsafeBuiltins() { } } +func (c *Compiler) checkDeprecatedBuiltins() { + for _, name := range c.sorted { + errs := checkDeprecatedBuiltins(c.deprecatedBuiltinsMap, c.Modules[name], c.strict) + for _, err := range errs { + c.err(err) + } + } +} + func (c *Compiler) runStage(metricName string, f func()) { if c.metrics != nil { c.metrics.Timer(metricName).Start() @@ -1225,6 +1237,9 @@ func (c *Compiler) init() { for _, bi := range c.capabilities.Builtins { c.builtins[bi.Name] = bi + if c.strict && bi.IsDeprecated() { + c.deprecatedBuiltinsMap[bi.Name] = struct{}{} + } } for name, bi := range c.customBuiltins { @@ -2015,6 +2030,7 @@ func (qc *queryCompiler) Compile(query Body) (Body, error) { {"RewriteDynamicTerms", "query_compile_stage_rewrite_dynamic_terms", qc.rewriteDynamicTerms}, {"CheckTypes", "query_compile_stage_check_types", qc.checkTypes}, {"CheckUnsafeBuiltins", "query_compile_stage_check_unsafe_builtins", qc.checkUnsafeBuiltins}, + {"CheckDeprecatedBuiltins", "query_compile_stage_check_deprecated_builtins", qc.checkDeprecatedBuiltins}, {"BuildComprehensionIndex", "query_compile_stage_build_comprehension_index", qc.buildComprehensionIndices}, } @@ -2185,6 +2201,14 @@ func (qc *queryCompiler) checkUnsafeBuiltins(_ *QueryContext, body Body) (Body, return body, nil } +func (qc *queryCompiler) checkDeprecatedBuiltins(_ *QueryContext, body Body) (Body, error) { + errs := checkDeprecatedBuiltins(qc.compiler.deprecatedBuiltinsMap, body, qc.compiler.strict) + if len(errs) > 0 { + return nil, errs + } + return body, nil +} + func (qc *queryCompiler) rewriteWithModifiers(_ *QueryContext, body Body) (Body, error) { f := newEqualityFactory(newLocalVarGenerator("q", body)) body, err := rewriteWithModifiersInBody(qc.compiler, f, body) @@ -4634,6 +4658,25 @@ func checkUnsafeBuiltins(unsafeBuiltinsMap map[string]struct{}, node interface{} return errs } +func checkDeprecatedBuiltins(deprecatedBuiltinsMap map[string]struct{}, node interface{}, strict bool) Errors { + // Early out; deprecatedBuiltinsMap is only populated in strict-mode. + if !strict { + return nil + } + + errs := make(Errors, 0) + WalkExprs(node, func(x *Expr) bool { + if x.IsCall() { + operator := x.Operator().String() + if _, ok := deprecatedBuiltinsMap[operator]; ok { + errs = append(errs, NewError(TypeErr, x.Loc(), "deprecated built-in function calls in expression: %v", operator)) + } + } + return false + }) + return errs +} + func rewriteVarsInRef(vars ...map[Var]Var) varRewriter { return func(node Ref) Ref { i, _ := TransformVars(node, func(v Var) (Value, error) { diff --git a/ast/compile_test.go b/ast/compile_test.go index 51512b383d..12fd2140a0 100644 --- a/ast/compile_test.go +++ b/ast/compile_test.go @@ -1692,6 +1692,53 @@ func TestCompilerCheckKeywordOverrides(t *testing.T) { runStrictnessTestCase(t, cases, true) } +func TestCompilerCheckDeprecatedMethods(t *testing.T) { + cases := []strictnessTestCase{ + { + note: "all() built-in", + module: `package test + p := all([true, false]) + `, + expectedErrors: Errors{ + &Error{ + Location: NewLocation([]byte("all([true, false])"), "", 2, 10), + Message: "deprecated built-in function calls in expression: all", + }, + }, + }, + { + note: "user-defined all()", + module: `package test + import future.keywords.in + all(arr) = {x | some x in arr} == {true} + p := all([true, false]) + `, + }, + { + note: "any() built-in", + module: `package test + p := any([true, false]) + `, + expectedErrors: Errors{ + &Error{ + Location: NewLocation([]byte("any([true, false])"), "", 2, 10), + Message: "deprecated built-in function calls in expression: any", + }, + }, + }, + { + note: "user-defined any()", + module: `package test + import future.keywords.in + any(arr) = true in arr + p := any([true, false]) + `, + }, + } + + runStrictnessTestCase(t, cases, true) +} + type strictnessTestCase struct { note string module string @@ -5647,6 +5694,23 @@ func TestQueryCompilerWithUnsafeBuiltins(t *testing.T) { } } +func TestQueryCompilerWithDeprecatedBuiltins(t *testing.T) { + cases := []strictnessQueryTestCase{ + { + note: "all() built-in", + query: "all([true, false])", + expectedErrors: fmt.Errorf("1 error occurred: 1:1: rego_type_error: deprecated built-in function calls in expression: all"), + }, + { + note: "any() built-in", + query: "any([true, false])", + expectedErrors: fmt.Errorf("1 error occurred: 1:1: rego_type_error: deprecated built-in function calls in expression: any"), + }, + } + + runStrictnessQueryTestCase(t, cases) +} + func TestQueryCompilerWithUnusedAssignedVar(t *testing.T) { cases := []strictnessQueryTestCase{ { diff --git a/docs/content/strict.md b/docs/content/strict.md index 99b5bd3643..013a0cbcbe 100644 --- a/docs/content/strict.md +++ b/docs/content/strict.md @@ -20,4 +20,5 @@ Name | Description | Enforced by default in OPA version --- | --- | --- Duplicate imports | Duplicate [imports](../policy-language/#imports), where one import shadows another, are prohibited. | 1.0 Unused local assignments | Unused [assignments](../policy-reference/#assignment-and-equality) local to a rule, function or comprehension are prohibited | 1.0 -`input` and `data` reserved keywords | `input` and `data` are reserved keywords, and may not be used as names for rules and variable assignment. | 1.0 \ No newline at end of file +`input` and `data` reserved keywords | `input` and `data` are reserved keywords, and may not be used as names for rules and variable assignment. | 1.0 +`any()` and `all()` removed | The `any()` and `all()` built-in functions have been deprecated, and will be removed in OPA 1.0. Use of these functions is prohibited. | 1.0 \ No newline at end of file