Skip to content

Commit

Permalink
Improve use-assignment-operator rule
Browse files Browse the repository at this point in the history
Using missing location attribute as an indicator of a
"generated" boolean assignment, we're now able to finish
the cases where this was previously not possible to discern
from the AST alone. The `use-assignment-operator` rule now
additionally flags unification in the following cases:

- `foo = bar`
- `foo(x) = bar`

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert committed Jun 26, 2023
1 parent cafd6b7 commit e5686e4
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 27 deletions.
9 changes: 9 additions & 0 deletions bundle/regal/ast.rego
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,15 @@ function_ret_in_args(fn_name, terms) if {
count(rest) > count(all_functions[fn_name].args)
}

# METADATA
# description: |
# answers if provided rule is implicitly assigned boolean true, i.e. allow { .. } or not
implicit_boolean_assignment(rule) if {
# note the missing location attribute here, which is how we distinguish
# between implicit and explicit assignments
rule.head.value == {"type": "boolean", "value": true}
}

all_functions := object.union(opa.builtins, function_decls(input.rules))

all_function_names := object.keys(all_functions)
37 changes: 13 additions & 24 deletions bundle/regal/rules/style/use_assignment_operator.rego
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,20 @@ import future.keywords.contains
import future.keywords.if
import future.keywords.in

import data.regal.ast
import data.regal.result

# Some cases blocked by https://github.com/StyraInc/regal/issues/6 - e.g:
#
# allow = true { true }
#
# f(x) = 5

todo_report contains violation if {
report contains violation if {
# foo = "bar"
some rule in input.rules
not rule["default"]
not rule.head.assign
not possibly_implicit_assign(rule)

# print(text_for_location(rule.head.location))
not rule.head.key
not ast.implicit_boolean_assignment(rule)

violation := result.fail(rego.metadata.chain(), result.location(rule))
}

empty_body(rule) if {
rule.body == [{"index": 0, "terms": {"type": "boolean", "value": true}}]
}

possibly_implicit_assign(rule) if {
rule.head.value == {"type": "boolean", "value": true}
}

report contains violation if {
# default foo = "bar"
some rule in input.rules
Expand All @@ -53,9 +39,12 @@ report contains violation if {
violation := result.fail(rego.metadata.chain(), result.location(rule.head.ref[0]))
}

text_for_location(location) := {"location": object.union(
location,
{"text": input.regal.file.lines[location.row - 1]},
)} if {
location.row
} else := {"location": location}
report contains violation if {
# foo(bar) = "baz"
some rule in input.rules
rule.head.args
not rule.head.assign
not ast.implicit_boolean_assignment(rule)

violation := result.fail(rego.metadata.chain(), result.location(rule.head.ref[0]))
}
80 changes: 80 additions & 0 deletions bundle/regal/rules/style/use_assignment_operator_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,56 @@ import data.regal.ast
import data.regal.config
import data.regal.rules.style["use-assignment-operator"] as rule

test_fail_unification_in_regular_assignment if {
r := rule.report with input as ast.policy(`foo = false`)
r == {{
"category": "style",
"description": "Prefer := over = for assignment",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-assignment-operator", "style"),
}],
"title": "use-assignment-operator",
"location": {"col": 1, "file": "policy.rego", "row": 3, "text": "foo = false"},
"level": "error",
}}
}

test_fail_not_implicit_boolean_assignment_with_body if {
r := rule.report with input as ast.policy(`allow = true { true }`)
r == {{
"category": "style",
"description": "Prefer := over = for assignment",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-assignment-operator", "style"),
}],
"title": "use-assignment-operator",
"location": {"col": 1, "file": "policy.rego", "row": 3, "text": "allow = true { true }"},
"level": "error",
}}
}

test_fail_not_implicit_boolean_assignment if {
r := rule.report with input as ast.policy(`foo = true`)
r == {{
"category": "style",
"description": "Prefer := over = for assignment",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-assignment-operator", "style"),
}],
"title": "use-assignment-operator",
"location": {"col": 1, "file": "policy.rego", "row": 3, "text": "foo = true"},
"level": "error",
}}
}

test_success_implicit_boolean_assignment if {
r := rule.report with input as ast.policy(`allow { true }`)
r == set()
}

test_fail_unification_in_default_assignment if {
r := rule.report with input as ast.policy(`default x = false`)
r == {{
Expand Down Expand Up @@ -45,3 +95,33 @@ test_success_assignment_in_object_rule_assignment if {
r := rule.report with input as ast.policy(`x["a"] := 1`)
r == set()
}

test_fail_unification_in_function_assignment if {
r := rule.report with input as ast.policy(`foo(bar) = "baz"`)
r == {{
"category": "style",
"description": "Prefer := over = for assignment",
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-assignment-operator", "style"),
}],
"title": "use-assignment-operator",
"location": {"col": 1, "file": "policy.rego", "row": 3, "text": `foo(bar) = "baz"`},
"level": "error",
}}
}

test_success_implicit_boolean_assignment_function if {
r := rule.report with input as ast.policy(`f(x) { 1 == 1 }`)
r == set()
}

test_success_assignment_operator_function if {
r := rule.report with input as ast.policy(`f(x) := true { 1 == 1 }`)
r == set()
}

test_success_partial_rule if {
r := rule.report with input as ast.policy(`partial["works"] { 1 == 1 }`)
r == set()
}
3 changes: 0 additions & 3 deletions p.rego

This file was deleted.

0 comments on commit e5686e4

Please sign in to comment.