Skip to content

Commit

Permalink
Include function args in ast.vars (and prefer-snake-case rule)
Browse files Browse the repository at this point in the history
Also add end location to `prefer-snake-case` violations

Fixes #942

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert committed Jul 30, 2024
1 parent 99fb078 commit 83d1132
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 10 deletions.
11 changes: 11 additions & 0 deletions bundle/regal/ast/search.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions bundle/regal/rules/style/prefer_snake_case.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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
59 changes: 54 additions & 5 deletions bundle/regal/rules/style/prefer_snake_case_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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},
}])
}

Expand All @@ -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 {
Expand All @@ -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},
}])
}

Expand All @@ -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},
}])
}

Expand All @@ -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},
}])
}

Expand All @@ -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},
}])
}

Expand All @@ -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},
}])
}

Expand All @@ -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 |
Expand Down
4 changes: 2 additions & 2 deletions bundle/regal/util/util.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 83d1132

Please sign in to comment.