ast: Deprecating any() and all() built-in functions #4271
Conversation
| types.B, | ||
| ), | ||
| } | ||
|
|
There was a problem hiding this comment.
Just moving these down to the "deprecated" section.
ast/compile.go
Outdated
| @@ -4406,14 +4411,17 @@ func safetyErrorSlice(unsafe unsafeVars, rewritten map[Var]Var) (result Errors) | |||
| return | |||
| } | |||
|
|
|||
| func checkUnsafeBuiltins(unsafeBuiltinsMap map[string]struct{}, node interface{}) Errors { | |||
| func checkUnsafeBuiltins(unsafeBuiltinsMap map[string]struct{}, deprecatedBuiltinsMap map[string]struct{}, node interface{}) Errors { | |||
There was a problem hiding this comment.
Perhaps I should rename this function as "unsafe" has a special meaning here. How are we on renaming existing compiler stages?
Although "unsafe" could also mean "implicitly unsafe"/"deprecated" ..
There was a problem hiding this comment.
Renaming the internal function is fine, but renaming the stage could break callers because we have an API that lets them register custom stages before/after a stage name... I agree that "safe" is not the best term to use here. That said, the "unsafe function" support is deprecated in favor of using capabilities--so, I'd probably just leave the stage name as is (we can remove it entirely eventually.)
There was a problem hiding this comment.
This looks good to me. The only alternative I'd have thought about was wrapping it into the check for undefined functions, https://github.com/open-policy-agent/opa/blob/main/ast/compile.go#L278. It's happening a little earlier the the compiler stages.
johanfylling
force-pushed
the
2437/strict_any_all_deprecated
branch
from
e77aef9
to
171831a
Compare
johanfylling
force-pushed
the
2437/strict_any_all_deprecated
branch
2 times, most recently
from
e4a4555
to
0ae6b04
Compare
Updating compiler strict mode to produce error when these deprecated methods are used. Fixes: open-policy-agent#2437 Signed-off-by: Johan Fylling <johan.dev@fylling.se>
johanfylling
force-pushed
the
2437/strict_any_all_deprecated
branch
from
0ae6b04
to
a4dba1d
Compare
ast/compile.go
Outdated
| @@ -4406,14 +4411,17 @@ func safetyErrorSlice(unsafe unsafeVars, rewritten map[Var]Var) (result Errors) | |||
| return | |||
| } | |||
|
|
|||
| func checkUnsafeBuiltins(unsafeBuiltinsMap map[string]struct{}, node interface{}) Errors { | |||
| func checkUnsafeBuiltins(unsafeBuiltinsMap map[string]struct{}, deprecatedBuiltinsMap map[string]struct{}, node interface{}) Errors { | |||
There was a problem hiding this comment.
Renaming the internal function is fine, but renaming the stage could break callers because we have an API that lets them register custom stages before/after a stage name... I agree that "safe" is not the best term to use here. That said, the "unsafe function" support is deprecated in favor of using capabilities--so, I'd probably just leave the stage name as is (we can remove it entirely eventually.)
…re merge) Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Updating compiler strict mode to produce error when these deprecated methods are used.
Fixes: #2437
Signed-off-by: Johan Fylling johan.dev@fylling.se