Include function args in ast.vars (and prefer-snake-case rule) (S…

…tyraInc#947)

Also add end location to `prefer-snake-case` violations

Fixes StyraInc#942

Signed-off-by: Anders Eknert <anders@styra.com>

bundle/regal/ast/search.rego

@@ -137,6 +137,17 @@ _find_vars(value, last) := {"every": _find_every_vars(value)} if {
 	value.domain
 }
 
+_find_vars(value, last) := {"args": arg_vars} if {
+	last == "args"
+
+	arg_vars := [arg |
+		some arg in value
+		arg.type == "var"
+	]
+
+	count(arg_vars) > 0
+}
+
 _rule_index(rule) := sprintf("%d", [i]) if {
 	some i, r in _rules # regal ignore:external-reference
 	r == rule

bundle/regal/rules/style/prefer_snake_case.rego

@@ -20,11 +20,11 @@ report contains violation if {
 	var := ast.vars[_][_][_]
 	not util.is_snake_case(var.value)
 
-	violation := result.fail(rego.metadata.chain(), result.location(var))
+	violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(var))
 }
 
-_location(_, part) := result.location(part) if part.location
+_location(_, part) := result.ranged_location_from_text(part) if part.location
 
 # workaround until https://github.com/open-policy-agent/opa/issues/6860
 # is fixed and we can trust that location is included for all ref parts
-_location(rule, part) := result.location(rule.head) if not part.location
+_location(rule, part) := result.ranged_location_from_text(rule.head) if not part.location

bundle/regal/rules/style/prefer_snake_case_test.rego

@@ -8,7 +8,13 @@ import data.regal.rules.style["prefer-snake-case"] as rule
 
 test_fail_camel_cased_rule_name if {
 	r := rule.report with input as ast.policy(`camelCase := 5`)
-	r == expected_with_locations([{"col": 1, "file": "policy.rego", "row": 3, "text": `camelCase := 5`}])
+	r == expected_with_locations([{
+		"col": 1,
+		"file": "policy.rego",
+		"row": 3,
+		"text": `camelCase := 5`,
+		"end": {"col": 10, "row": 3},
+	}])
 }
 
 test_success_snake_cased_rule_name if {
@@ -18,7 +24,13 @@ test_success_snake_cased_rule_name if {
 
 test_fail_camel_cased_some_declaration if {
 	r := rule.report with input as ast.policy(`p {some fooBar; input[_]}`)
-	r == expected_with_locations([{"col": 9, "file": "policy.rego", "row": 3, "text": `p {some fooBar; input[_]}`}])
+	r == expected_with_locations([{
+		"col": 9,
+		"file": "policy.rego",
+		"row": 3,
+		"text": `p {some fooBar; input[_]}`,
+		"end": {"col": 15, "row": 3},
+	}])
 }
 
 test_success_snake_cased_some_declaration if {
@@ -35,6 +47,7 @@ test_fail_camel_cased_multiple_some_declaration if {
 		"file": "policy.rego",
 		"row": 6,
 		"text": "\t\tsome x, foo_bar, fooBar; x = 1; foo_bar = 2; input[_]",
+		"end": {"col": 26, "row": 6},
 	}])
 }
 
@@ -43,9 +56,31 @@ test_success_snake_cased_multiple_some_declaration if {
 	r == set()
 }
 
+test_fail_camel_cased_function_argument if {
+	r := rule.report with input as ast.with_rego_v1(`f(fooBar) := fooBar`)
+	r == expected_with_locations([{
+		"col": 3,
+		"file": "policy.rego",
+		"row": 5,
+		"text": "f(fooBar) := fooBar",
+		"end": {"col": 9, "row": 5},
+	}])
+}
+
+test_success_not_camel_cased_function_argument if {
+	r := rule.report with input as ast.with_rego_v1(`f(foo) := foo`)
+	r == set()
+}
+
 test_fail_camel_cased_var_assignment if {
 	r := rule.report with input as ast.policy(`allow { camelCase := 5 }`)
-	r == expected_with_locations([{"col": 9, "file": "policy.rego", "row": 3, "text": `allow { camelCase := 5 }`}])
+	r == expected_with_locations([{
+		"col": 9,
+		"file": "policy.rego",
+		"row": 3,
+		"text": `allow { camelCase := 5 }`,
+		"end": {"col": 18, "row": 3},
+	}])
 }
 
 test_fail_camel_cased_multiple_var_assignment if {
@@ -55,6 +90,7 @@ test_fail_camel_cased_multiple_var_assignment if {
 		"file": "policy.rego",
 		"row": 3,
 		"text": `allow { snake_case := "foo"; camelCase := 5 }`,
+		"end": {"col": 39, "row": 3},
 	}])
 }
 
@@ -70,6 +106,7 @@ test_fail_camel_cased_some_in_value if {
 		"file": "policy.rego",
 		"row": 5,
 		"text": `allow if { some cC in input }`,
+		"end": {"col": 19, "row": 5},
 	}])
 }
 
@@ -80,6 +117,7 @@ test_fail_camel_cased_some_in_key_value if {
 		"file": "policy.rego",
 		"row": 5,
 		"text": `allow if { some cC, sc in input }`,
+		"end": {"col": 19, "row": 5},
 	}])
 }
 
@@ -90,6 +128,7 @@ test_fail_camel_cased_some_in_key_value_2 if {
 		"file": "policy.rego",
 		"row": 5,
 		"text": `allow if { some sc, cC in input }`,
+		"end": {"col": 23, "row": 5},
 	}])
 }
 
@@ -105,14 +144,18 @@ test_fail_camel_cased_every_value if {
 		"file": "policy.rego",
 		"row": 5,
 		"text": `allow if { every cC in input { cC == 1 } }`,
+		"end": {"col": 20, "row": 5},
 	}])
 }
 
 test_fail_camel_cased_every_key if {
 	r := rule.report with input as ast.with_rego_v1(`allow if { every cC, sc in input { cC == 1; sc == 2 } }`)
 	r == expected_with_locations([{
-		"col": 18, "file": "policy.rego", "row": 5,
+		"col": 18,
+		"file": "policy.rego",
+		"row": 5,
 		"text": `allow if { every cC, sc in input { cC == 1; sc == 2 } }`,
+		"end": {"col": 20, "row": 5},
 	}])
 }
 
@@ -124,7 +167,13 @@ test_success_snake_cased_every if {
 # https://github.com/open-policy-agent/opa/issues/6860
 test_fail_location_provided_even_when_not_in_ref if {
 	r := rule.report with input as ast.with_rego_v1(`foo.Bar := true`)
-	r == expected_with_locations([{"col": 5, "file": "policy.rego", "row": 5, "text": "foo.Bar := true"}])
+	r == expected_with_locations([{
+		"col": 5,
+		"file": "policy.rego",
+		"row": 5,
+		"text": "foo.Bar := true",
+		"end": {"col": 8, "row": 5},
+	}])
 }
 
 expected_with_locations(locations) := {with_location |

bundle/regal/util/util.rego

@@ -25,8 +25,8 @@ find_duplicates(arr) := {indices |
 
 # METADATA
 # description: returns true if array has duplicates of item
-has_duplicates(array, item) if count([x |
-	some x in array
+has_duplicates(arr, item) if count([x |
+	some x in arr
 	x == item
 ]) > 1