Rule: prefer-strings-count (#948)

Fixes #940

Signed-off-by: Anders Eknert <[email protected]>

README.md

@@ -217,6 +217,7 @@ The following rules are currently available:
 | idiomatic   | [use-if](https://docs.styra.com/regal/rules/idiomatic/use-if)                                         | Use the `if` keyword                                      |
 | idiomatic   | [use-in-operator](https://docs.styra.com/regal/rules/idiomatic/use-in-operator)                       | Use in to check for membership                            |
 | idiomatic   | [use-some-for-output-vars](https://docs.styra.com/regal/rules/idiomatic/use-some-for-output-vars)     | Use `some` to declare output variables                    |
+| idiomatic   | [use-strings-count](https://docs.styra.com/regal/rules/idiomatic/use-strings-count)                   | Use `strings.count` where possible                        |
 | imports     | [avoid-importing-input](https://docs.styra.com/regal/rules/imports/avoid-importing-input)             | Avoid importing input                                     |
 | imports     | [circular-import](https://docs.styra.com/regal/rules/imports/circular-import)                         | Circular import                                           |
 | imports     | [ignored-import](https://docs.styra.com/regal/rules/imports/ignored-import)                           | Reference ignores import                                  |

bundle/regal/capabilities.rego

@@ -16,6 +16,8 @@ provided := data.internal.capabilities
 
 has_object_keys if "object.keys" in object.keys(config.capabilities.builtins)
 
+has_strings_count if "strings.count" in object.keys(config.capabilities.builtins)
+
 # if if if!
 has_if if "if" in config.capabilities.future_keywords
 

bundle/regal/config/provided/data.yaml

@@ -6,8 +6,8 @@ rules:
     annotation-without-metadata:
       level: error
     argument-always-wildcard:
+      except-function-name-pattern: '^mock_'
       level: error
-      except-function-name-pattern: "^mock_"
     constant-condition:
       level: error
     deprecated-builtin:
@@ -83,6 +83,8 @@ rules:
       level: error
     use-some-for-output-vars:
       level: error
+    use-strings-count:
+      level: error
   imports:
     avoid-importing-input:
       level: error

bundle/regal/rules/idiomatic/use_strings_count.rego

@@ -0,0 +1,35 @@
+# METADATA
+# description: Use `strings.count` where possible
+package regal.rules.idiomatic["use-strings-count"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.capabilities
+import data.regal.result
+
+# METADATA
+# description: Missing capability for built-in function `strings.count`
+# custom:
+#   severity: warning
+notices contains result.notice(rego.metadata.chain()) if not capabilities.has_object_keys
+
+# METADATA
+# description: flag calls to `count` where the first argument is a call to `indexof_n`
+report contains violation if {
+	some rule in input.rules
+
+	ref := ast.refs[_][_]
+
+	ref[0].value[0].type == "var"
+	ref[0].value[0].value == "count"
+
+	ref[1].type == "call"
+	ref[1].value[0].value[0].type == "var"
+	ref[1].value[0].value[0].value == "indexof_n"
+
+	loc1 := result.location(ref[0])
+	loc2 := result.ranged_location_from_text(ref[1])
+
+	violation := result.fail(rego.metadata.chain(), object.union(loc1, {"location": {"end": loc2.location.end}}))
+}

bundle/regal/rules/idiomatic/use_strings_count_test.rego

@@ -0,0 +1,42 @@
+package regal.rules.idiomatic["use-strings-count_test"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.config
+
+import data.regal.rules.idiomatic["use-strings-count"] as rule
+
+test_fail_can_use_strings_count if {
+	module := ast.with_rego_v1(`x := count(indexof_n("foo", "o"))`)
+
+	r := rule.report with input as module
+	r == {{
+		"category": "idiomatic",
+		"description": "Use `strings.count` where possible",
+		"level": "error",
+		"location": {
+			"col": 6,
+			"file": "policy.rego",
+			"row": 5,
+			"text": `x := count(indexof_n("foo", "o"))`,
+			"end": {"col": 34, "row": 5},
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/use-strings-count", "idiomatic"),
+		}],
+		"title": "use-strings-count",
+	}}
+}
+
+test_has_notice_if_unmet_capability if {
+	r := rule.notices with config.capabilities as {}
+	r == {{
+		"category": "idiomatic",
+		"description": "Missing capability for built-in function `strings.count`",
+		"level": "notice",
+		"severity": "warning",
+		"title": "use-strings-count",
+	}}
+}

docs/rules/idiomatic/use-strings-count.md

@@ -0,0 +1,50 @@
+# use-strings-count
+
+**Summary**: Use `strings.count` where possible
+
+**Category**: Idiomatic
+
+**Avoid**
+```rego
+package policy
+
+import rego.v1
+
+num_as := count(indexof_n("foobarbaz", "a"))
+```
+
+**Prefer**
+```rego
+package policy
+
+import rego.v1
+
+num_as := strings.count("foobarbaz", "a")
+```
+
+## Rationale
+
+The `strings.count` function added in [OPA v0.67.0](https://github.com/open-policy-agent/opa/releases/tag/v0.67.0)
+is both more readable and efficient compared to using `count(indexof_n(...))` and should therefore be preferred.
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  idiomatic:
+    use-strings-count:
+      # one of "error", "warning", "ignore"
+      level: error
+```
+
+## Related Resources
+
+- OPA Docs: [strings.count](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringscount)
+
+## Community
+
+If you think you've found a problem with this rule or its documentation, would like to suggest improvements, new rules,
+or just talk about Regal in general, please join us in the `#regal` channel in the Styra Community
+[Slack](https://communityinviter.com/apps/styracommunity/signup)!

e2e/cli_test.go

@@ -710,8 +710,9 @@ func TestLintWithCustomCapabilitiesAndUnmetRequirement(t *testing.T) {
 	// This is only an informative warning — command should not fail
 	expectExitCode(t, err, 0, &stdout, &stderr)
 
-	expectOut := "1 file linted. No violations found. 2 rules skipped:\n" +
+	expectOut := "1 file linted. No violations found. 3 rules skipped:\n" +
 		"- custom-has-key-construct: Missing capability for built-in function `object.keys`\n" +
+		"- use-strings-count: Missing capability for built-in function `strings.count`\n" +
 		"- use-rego-v1: Missing capability for `import rego.v1`\n\n"
 
 	if stdout.String() != expectOut {
@@ -736,8 +737,9 @@ func TestLintWithCustomCapabilitiesAndUnmetRequirementMultipleFiles(t *testing.T
 	// This is only an informative warning — command should not fail
 	expectExitCode(t, err, 0, &stdout, &stderr)
 
-	expectOut := "2 files linted. No violations found. 2 rules skipped:\n" +
+	expectOut := "2 files linted. No violations found. 3 rules skipped:\n" +
 		"- custom-has-key-construct: Missing capability for built-in function `object.keys`\n" +
+		"- use-strings-count: Missing capability for built-in function `strings.count`\n" +
 		"- use-rego-v1: Missing capability for `import rego.v1`\n\n"
 
 	if stdout.String() != expectOut {

e2e/testdata/violations/most_violations.rego

@@ -137,6 +137,8 @@ incremental if input.x
 
 incremental if input.y
 
+prefer_strings_count := count(indexof_n("foobarbaz", "a"))
+
 ### Style ###
 
 # avoid-get-and-list-prefix