opa
opa copied to clipboard
open-policy-agent
Add `strings.count(string, substring)` built-in function
For cases where you need to count the occurances of a substring in some text, it would be nice if instead of this:
n := count(indexof_n(string, substring))
One could do this:
n := strings.count(string, substring)
"Nice" in that it more clearly communicates intent, would be easier to find in documentation, and likely can be made more performant.
Just checking in, @Manish-Giri. Any progress to report? Let us know if you need any help!
Hello @anderseknert, thank you for offering help! I might actually need some here.
The only reference of indexof_n() that I could find was here - https://github.com/open-policy-agent/opa/blob/e0ee7418b07236e57f39eee4c2186a077afce29c/ast/builtins.go#L1072-L1083
which leads to https://github.com/open-policy-agent/opa/blob/5464b005e8f6bedd644c95d67e8c09eee3c7352f/topdown/strings.go#L231-L261
I couldn't locate any actual usages of indexof_n(string, substring), which, along with count(), can be replaced with strings.Count().
Would you be able to help point me towards where I need to look?
I don't think there are any usages of that in OPA's codebase.
Built-in functions are commonly tested using the "YAML test suite", which you'll find here. Once you've built an implementation of strings.count, you should add a couple of tests in that directory. Instructions for how to run those tests can be found here.
You can also search the list of closed PRs to find other built-in functions that have been added, and what is in those PRs. Here's a recent example.
Note that you don't need to write the builtin_metadata.json, capabilities.json, policy_reference.md by hand! Those are auto-generated from the built-in definition in builtins.go.
I hope that helps! And feel free to ask more questions.
@anderseknert Thank you very much for explaining the approach and sharing these resources, I found them very helpful!
A first stab at a solution is now available in the linked pull request, ready for review. There does appear to be a problem with the DCO check, although I did amend the commit message after, to match the expected format.