Fix bug causing prefer-some-in-iteration not to be reported (#902)

This was due to an unfortunate combination of trying to be lenient
about `coll[_]` used in the position of a function arg (as that's not
*directly* replaceable by `some .. in` **but** not excluding operators
like `:=` from functions (where it _should_ be replaced by `some .. in`)
**and** many tests for this rule missing to provide the built-in functions
as a mocked dependency.

Will need to think some about how to best avoid this to happen again.

Also:
- Replace `ast.policy` in tests with `ast.with_rego_v1`
- Fix docs that used `=` for assignment rather than `:=`

Fixes #901

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

bundle/regal/ast/ast.rego

@@ -6,6 +6,26 @@ import data.regal.config
 
 scalar_types := {"boolean", "null", "number", "string"}
 
+operators := {
+	"and",
+	"assign",
+	"div",
+	"eq",
+	"equal",
+	"gt",
+	"gte",
+	"internal.member_2",
+	"internal.member_3",
+	"lt",
+	"lte",
+	"minus",
+	"mul",
+	"neq",
+	"or",
+	"plus",
+	"rem",
+}
+
 # regal ignore:external-reference
 is_constant(value) if value.type in scalar_types
 

bundle/regal/rules/style/prefer_some_in_iteration.rego

@@ -82,6 +82,7 @@ invalid_some_context(rule, path) if {
 	node.terms[0].type == "ref"
 	node.terms[0].value[0].type == "var"
 	node.terms[0].value[0].value in ast.all_function_names # regal ignore:external-reference
+	not node.terms[0].value[0].value in ast.operators # regal ignore:external-reference
 }
 
 # if previous node is of type call, also don't recommend `some .. in`

bundle/regal/rules/style/prefer_some_in_iteration_test.rego

@@ -8,40 +8,45 @@ import data.regal.config
 import data.regal.rules.style["prefer-some-in-iteration"] as rule
 
 test_fail_simple_iteration if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		var := input.foo[_]
 	}`)
 
-	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
-	r == with_location({"col": 10, "file": "policy.rego", "row": 4, "text": "\t\tvar := input.foo[_]"})
+	r := rule.report with config.for_rule as allow_nesting(2)
+		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 10, "file": "policy.rego", "row": 6, "text": "\t\tvar := input.foo[_]"})
 }
 
 test_fail_simple_iteration_comprehension if {
-	policy := ast.policy(`s := {p |
+	policy := ast.with_rego_v1(`s := {p |
 		p := input.foo[_]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
-	r == with_location({"col": 8, "file": "policy.rego", "row": 4, "text": "\t\tp := input.foo[_]"})
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 8, "file": "policy.rego", "row": 6, "text": "\t\tp := input.foo[_]"})
 }
 
 test_fail_simple_iteration_output_var if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		input.foo[x]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
-	r == with_location({"col": 3, "file": "policy.rego", "row": 4, "text": "\t\tinput.foo[x]"})
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 3, "file": "policy.rego", "row": 6, "text": "\t\tinput.foo[x]"})
 }
 
 test_fail_simple_iteration_output_var_some_decl if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		some x
 		input.foo[x]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
-	r == with_location({"col": 3, "file": "policy.rego", "row": 5, "text": "\t\tinput.foo[x]"})
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 3, "file": "policy.rego", "row": 7, "text": "\t\tinput.foo[x]"})
 }
 
 test_success_some_in_var_input if {
@@ -51,61 +56,67 @@ test_success_some_in_var_input if {
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_nesting_zero if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		input.foo[_]
 		input.foo[_].bar[_]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(0) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_nesting_one if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		input.foo[_]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(1) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_nesting_two if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		input.foo[_].bar[_]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_fail_allow_nesting_two if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		input.foo[_]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
-	r == with_location({"col": 3, "file": "policy.rego", "row": 4, "text": "\t\tinput.foo[_]"})
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 3, "file": "policy.rego", "row": 6, "text": "\t\tinput.foo[_]"})
 }
 
 test_success_not_output_vars if {
-	policy := ast.policy(`
+	policy := ast.with_rego_v1(`
 	x := 5
 
-	allow {
+	allow if {
 		y := 10
 		input.foo[x].bar[y]
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_output_var_to_input_var if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		# x is an output var here
 		# iteration allowed as nesting level == 2
 		input.foo[x].bar[_]
@@ -115,21 +126,23 @@ test_success_output_var_to_input_var if {
 	}`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_complex_comprehension_term if {
-	policy := ast.policy(`
+	policy := ast.with_rego_v1(`
 
 	foo := [{"foo": bar} | input[bar]]
 	`)
 
 	r := rule.report with config.for_rule as allow_nesting(2) with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_if_subattribute if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		bar := input.foo[_].bar
 		bar == "baz"
 	}`)
@@ -140,11 +153,12 @@ test_success_allow_if_subattribute if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_fail_ignore_if_subattribute_disabled if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		bar := input.foo[_].bar
 		bar == "baz"
 	}`)
@@ -155,11 +169,12 @@ test_fail_ignore_if_subattribute_disabled if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
-	r == with_location({"col": 10, "file": "policy.rego", "row": 4, "text": "\t\tbar := input.foo[_].bar"})
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == with_location({"col": 10, "file": "policy.rego", "row": 6, "text": "\t\tbar := input.foo[_].bar"})
 }
 
 test_success_allow_if_inside_array if {
-	policy := ast.policy(`allow {
+	policy := ast.with_rego_v1(`allow if {
 		bar := [input.foo[_] == 1]
 	}`)
 
@@ -169,30 +184,33 @@ test_success_allow_if_inside_array if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_if_inside_set if {
-	policy := ast.policy(`s := {input.foo[_] == 1}`)
+	policy := ast.with_rego_v1(`s := {input.foo[_] == 1}`)
 
 	r := rule.report with config.for_rule as {
 		"level": "error",
 		"ignore-if-sub-attribute": true,
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
 test_success_allow_if_inside_object if {
-	policy := ast.policy(`s := {foo: input.foo[_] == 1}`)
+	policy := ast.with_rego_v1(`s := {foo: input.foo[_] == 1}`)
 
 	r := rule.report with config.for_rule as {
 		"level": "error",
 		"ignore-if-sub-attribute": true,
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
@@ -205,6 +223,7 @@ test_success_allow_if_inside_rule_head_key if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
@@ -218,6 +237,7 @@ test_success_allow_if_contains_check_eq if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 
@@ -231,6 +251,7 @@ test_success_allow_if_contains_check_equal if {
 		"ignore-nesting-level": 5,
 	}
 		with input as policy
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
 

docs/rules/style/prefer-some-in-iteration.md

@@ -10,7 +10,7 @@ package policy
 
 import rego.v1
 
-engineering_roles = {"engineer", "dba", "developer"}
+engineering_roles := {"engineer", "dba", "developer"}
 
 engineers contains employee if {
     employee := data.employees[_]
@@ -24,7 +24,7 @@ package policy
 
 import rego.v1
 
-engineering_roles = {"engineer", "dba", "developer"}
+engineering_roles := {"engineer", "dba", "developer"}
 
 engineers contains employee if {
     some employee in data.employees