Rule: sprintf-arguments-mismatch

README.md

@@ -196,6 +196,7 @@ The following rules are currently available:
 | bugs        | [redundant-existence-check](https://docs.styra.com/regal/rules/bugs/redundant-existence-check)        | Redundant existence check                                 |
 | bugs        | [rule-named-if](https://docs.styra.com/regal/rules/bugs/rule-named-if)                                | Rule named "if"                                           |
 | bugs        | [rule-shadows-builtin](https://docs.styra.com/regal/rules/bugs/rule-shadows-builtin)                  | Rule name shadows built-in                                |
+| bugs        | [sprintf-arguments-mismatch](https://docs.styra.com/regal/rules/bugs/sprintf-arguments-mismatch)      | Mismatch in `sprintf` arguments count                     |
 | bugs        | [top-level-iteration](https://docs.styra.com/regal/rules/bugs/top-level-iteration)                    | Iteration in top-level assignment                         |
 | bugs        | [unassigned-return-value](https://docs.styra.com/regal/rules/bugs/unassigned-return-value)            | Non-boolean return value unassigned                       |
 | bugs        | [unused-output-variable](https://docs.styra.com/regal/rules/bugs/unused-output-variable)              | Unused output variable                                    |

bundle/regal/ast/ast.rego

@@ -120,16 +120,21 @@ is_ref(value) if value.type == "ref"
 
 is_ref(value) if value[0].type == "ref"
 
+# METADATA
+# description: |
+#   returns an array of all rule indices, as strings. this will be needed until
+#   https://github.com/open-policy-agent/opa/issues/6736 is fixed
+rule_index_strings := [s |
+	some i, _ in _rules
+	s := sprintf("%d", [i])
+]
+
 # METADATA
 # description: |
 #   a map containing all function calls (built-in and custom) in the input AST
 #   keyed by rule index
 function_calls[rule_index] contains call if {
-	some i, rule in _rules
-
-	# converting to string until https://github.com/open-policy-agent/opa/issues/6736 is fixed
-	rule_index := sprintf("%d", [i])
-
+	some rule_index in rule_index_strings
 	some ref in found.refs[rule_index]
 
 	name := ref_to_string(ref[0].value)
@@ -141,7 +146,7 @@ function_calls[rule_index] contains call if {
 
 	call := {
 		"name": ref_to_string(ref[0].value),
-		"location": util.to_location_object(ref[0].location),
+		"location": ref[0].location,
 		"args": args,
 	}
 }
@@ -206,8 +211,6 @@ builtin_functions_called contains name if {
 		some part in value[0].value
 		value := part.value
 	])
-
-	name in builtin_names
 }
 
 # METADATA

bundle/regal/config/provided/data.yaml

@@ -37,6 +37,8 @@ rules:
       level: error
     rule-shadows-builtin:
       level: error
+    sprintf-arguments-mismatch:
+      level: error
     top-level-iteration:
       level: error
     unassigned-return-value:

bundle/regal/result.rego

@@ -222,3 +222,10 @@ ranged_location_from_text(x) := end if {
 		"col": loc.location.col + count(regal.last(lines)),
 	}}})
 }
+
+# METADATA
+# description: creates a location where x is the start, and y is the end (calculated from `text`)
+ranged_location_between(x, y) := object.union(
+	location(x),
+	{"location": {"end": ranged_location_from_text(y).location.end}},
+)

bundle/regal/rules/bugs/sprintf_argument_mismatch.rego

@@ -0,0 +1,48 @@
+# METADATA
+# description: Mismatch in `sprintf` arguments count
+package regal.rules.bugs["sprintf-arguments-mismatch"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.config
+import data.regal.result
+
+# METADATA
+# description: Missing capability for built-in `sprintf`
+# custom:
+#   severity: none
+notices contains result.notice(rego.metadata.chain()) if not "sprintf" in object.keys(config.capabilities.builtins)
+
+# METADATA
+# description: |
+#   Count the number of distinct arguments ("verbs", denoted by %) in the string argument,
+#   compare it to the number of items in the array (if known), and flag when the numbers
+#   don't match
+report contains violation if {
+	some fn
+	ast.function_calls[_][fn].name == "sprintf"
+
+	fn.args[1].type == "array" # can only check static arrays, not vars
+
+	values_in_arr := count(fn.args[1].value)
+	str_no_escape := replace(fn.args[0].value, "%%", "") # don't include '%%' as it's used to "escape" %
+	values_in_str := strings.count(str_no_escape, "%") - _repeated_explicit_argument_indexes(str_no_escape)
+
+	values_in_str != values_in_arr
+
+	violation := result.fail(rego.metadata.chain(), result.ranged_location_between(fn.args[0], regal.last(fn.args)))
+}
+
+default _repeated_explicit_argument_indexes(_) := 0
+
+# see: https://pkg.go.dev/fmt#hdr-Explicit_argument_indexes
+# each distinct explicit argument index should only contribute one value to the
+# values array. this calculates the number to subtract from the total expected
+# number of values based on the number of eai's occurring more than once
+_repeated_explicit_argument_indexes(str) := sum([n |
+	some eai in _unique_explicit_arguments(str)
+	n := strings.count(str, eai) - 1
+])
+
+_unique_explicit_arguments(str) := {eai | some eai in regex.find_n(`%\[\d\]`, str, -1)}

bundle/regal/rules/bugs/sprintf_argument_mismatch_test.rego

@@ -0,0 +1,89 @@
+package regal.rules.bugs["sprintf-arguments-mismatch_test"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.config
+
+import data.regal.rules.bugs["sprintf-arguments-mismatch"] as rule
+
+test_fail_too_many_values_in_array if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%s", [1, 2])`)
+	r == {{
+		"category": "bugs",
+		"description": "Mismatch in `sprintf` arguments count",
+		"level": "error",
+		"location": {
+			"row": 5,
+			"col": 14,
+			"end": {"col": 26, "row": 5},
+			"file": "policy.rego",
+			"text": "x := sprintf(\"%s\", [1, 2])",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/sprintf-arguments-mismatch", "bugs"),
+		}],
+		"title": "sprintf-arguments-mismatch",
+	}}
+}
+
+test_fail_too_few_values_in_array if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%s%v", [1])`)
+	r == {{
+		"category": "bugs",
+		"description": "Mismatch in `sprintf` arguments count",
+		"level": "error",
+		"location": {
+			"row": 5,
+			"col": 14,
+			"end": {"col": 25, "row": 5},
+			"file": "policy.rego",
+			"text": `x := sprintf("%s%v", [1])`,
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/sprintf-arguments-mismatch", "bugs"),
+		}],
+		"title": "sprintf-arguments-mismatch",
+	}}
+}
+
+test_success_same_number_of_values if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%s%d", [1, 2])`)
+	r == set()
+}
+
+test_fail_different_number_of_values_with_explicit_index if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%[1]s %[1]s %[2]d", [1, 2, 3])`)
+	r == {{
+		"category": "bugs",
+		"description": "Mismatch in `sprintf` arguments count",
+		"level": "error",
+		"location": {
+			"row": 5,
+			"col": 14,
+			"end": {
+				"col": 44,
+				"row": 5,
+			},
+			"file": "policy.rego",
+			"text": "x := sprintf(\"%[1]s %[1]s %[2]d\", [1, 2, 3])",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/sprintf-arguments-mismatch", "bugs"),
+		}],
+		"title": "sprintf-arguments-mismatch",
+	}}
+}
+
+test_success_same_number_of_values_with_explicit_index if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%[1]s %[1]s %[2]d", [1, 2])`)
+	r == set()
+}
+
+test_success_escaped_verbs_are_ignored if {
+	r := rule.report with input as ast.with_rego_v1(`x := sprintf("%d %% %% %s", [1, "f"])`)
+	r == set()
+}

bundle/regal/util/util.rego

@@ -49,3 +49,10 @@ to_location_object(loc) := {"row": to_number(row), "col": to_number(col), "text"
 }
 
 to_location_object(loc) := loc if is_object(loc)
+
+# METADATA
+# description: short-hand helper to prepare values for pretty-printing
+json_pretty(value) := json.marshal_with_options(value, {
+	"indent": "  ",
+	"pretty": true,
+})

docs/rules/bugs/sprintf-arguments-mismatch.md

@@ -0,0 +1,73 @@
+# sprintf-arguments-mismatch
+
+**Summary**: Mismatch in `sprintf` arguments count
+
+**Category**: Bugs
+
+**Avoid**
+```rego
+package policy
+
+import rego.v1
+
+max_issues := 1
+
+report contains warning if {
+    count(issues) > max_issues
+
+    # two placeholders found in the string, but only one value in the array
+    warning := sprintf("number of issues (%d) must not be higher than %d", [count(issues)])
+}
+```
+
+**Prefer**
+```rego
+package policy
+
+import rego.v1
+
+max_issues := 1
+
+report contains warning if {
+    count(issues) > max_issues
+
+    # two placeholders found in the string, and two values in the array
+    warning := sprintf("number of issues (%d) must not be higher than %d", [count(issues), max_issues])
+}
+```
+
+## Rationale
+
+While the built-in `sprintf` function itself reports argument mismatches, it'll do so by returning a string containing
+the error message rather than actually failing.
+
+```shell
+> opa eval -f pretty 'sprintf("%v %d", [1])'
+"1 %!d(MISSING)"
+```
+
+While this is normally caught in development and testing, having this issue reported at "compile time", which ideally
+is [directly in your editor](https://docs.styra.com/regal/language-server) as you work on your policy. This means less
+time spent chasing down issues later, and a happier development experience.
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  bugs:
+    sprintf-arguments-mismatch:
+      # one of "error", "warning", "ignore"
+      level: error
+```
+
+## Related Resources
+
+- OPA Docs: [Built-in Functions: `sprintf`](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-sprintf)
+
+## 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/testdata/violations/most_violations.rego

@@ -59,6 +59,8 @@ not_equals_in_loop if {
 	"foo" != input.bar[_]
 }
 
+sprintf_arguments_mismatch := sprintf("%v", [1, 2])
+
 # rule-shadows-builtin
 abs := true