Rule: boolean-assignment (#488)

Prefer boolean expression in rule body over assigning the
result in the rule head.

Also did some consistency fixes in the docs "Related Resources"
sections.

Fixes #465

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

README.md

@@ -198,6 +198,7 @@ The following rules are currently available:
 | custom    | [naming-convention](https://docs.styra.com/regal/rules/custom/naming-convention)                    | Naming convention violation                               |
 | custom    | [one-liner-rule](https://docs.styra.com/regal/rules/custom/one-liner-rule)                          | Rule body could be made a one-liner                       |
 | custom    | [prefer-value-in-head](https://docs.styra.com/regal/rules/custom/prefer-value-in-head)              | Prefer value in rule head                                 |
+| idiomatic | [boolean-assignment](https://docs.styra.com/regal/rules/idiomatic/boolean-assignment)               | Prefer `if` over boolean assignment                       |
 | idiomatic | [custom-has-key-construct](https://docs.styra.com/regal/rules/idiomatic/custom-has-key-construct)   | Custom function may be replaced by `in` and `object.keys` |
 | idiomatic | [custom-in-construct](https://docs.styra.com/regal/rules/idiomatic/custom-in-construct)             | Custom function may be replaced by `in` keyword           |
 | idiomatic | [equals-pattern-matching](https://docs.styra.com/regal/rules/idiomatic/equals-pattern-matching)     | Prefer pattern matching in function arguments             |

bundle/regal/config/provided/data.yaml

@@ -32,6 +32,8 @@ rules:
     prefer-value-in-head:
       level: ignore
   idiomatic:
+    boolean-assignment:
+      level: error
     custom-has-key-construct:
       level: error
     custom-in-construct:

bundle/regal/rules/idiomatic/boolean_assignment.rego

@@ -0,0 +1,24 @@
+# METADATA
+# description: Prefer `if` over boolean assignment
+package regal.rules.idiomatic["boolean-assignment"]
+
+import rego.v1
+
+import data.regal.config
+import data.regal.result
+
+report contains violation if {
+	some rule in input.rules
+
+	rhv := rule.head.value
+
+	rhv.type == "call"
+	rhv.value[0].type == "ref"
+	rhv.value[0].value[0].type == "var"
+
+	ref_name := rhv.value[0].value[0].value
+
+	config.capabilities.builtins[ref_name].decl.result == "boolean"
+
+	violation := result.fail(rego.metadata.chain(), result.location(rule))
+}

bundle/regal/rules/idiomatic/boolean_assignment_test.rego

@@ -0,0 +1,43 @@
+package regal.rules.idiomatic["boolean-assignment_test"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.capabilities
+import data.regal.config
+
+import data.regal.rules.idiomatic["boolean-assignment"] as rule
+
+test_boolean_assignment_in_rule_head if {
+	module := ast.policy("more_than_one := input.count > 1")
+
+	r := rule.report with input as module
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == {{
+		"category": "idiomatic",
+		"description": "Prefer `if` over boolean assignment",
+		"level": "error",
+		"location": {"col": 1, "file": "policy.rego", "row": 3, "text": "more_than_one := input.count > 1"},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/boolean-assignment", "idiomatic"),
+		}],
+		"title": "boolean-assignment",
+	}}
+}
+
+test_success_uses_if_instead_of_boolean_assignment_in_rule_head if {
+	module := ast.with_rego_v1("more_than_one if input.count > 1")
+
+	r := rule.report with input as module
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == set()
+}
+
+test_success_non_boolean_assignment_in_rule_head if {
+	module := ast.with_rego_v1(`foo := lower("FOO")`)
+
+	r := rule.report with input as module
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == set()
+}

docs/rules/idiomatic/boolean-assignment.md

@@ -0,0 +1,65 @@
+# boolean-assignment
+
+**Summary**: Prefer `if` over boolean assignment
+
+**Category**: Idiomatic
+
+**Avoid**
+```rego
+package policy
+
+import rego.v1
+
+more_than_one_member := count(input.members) > 1
+```
+
+**Prefer**
+```rego
+package policy
+
+import rego.v1
+
+more_than_one_member if count(input.members) > 1
+```
+
+## Rationale
+
+Assigning the result of a boolean function is almost always redundant, as the boolean value returned by the expression
+rarely is used for anything but to determine whether to continue evaluation. Moving the condition to the body following
+an `if` will have the rule either evaluate to `true` or be undefined. For the few cases where `false` should be
+returned, using a `default` rule assignment is preferable, as it is guaranteed to be assigned a value even on undefined
+input:
+
+```rego
+package policy
+
+import rego.v1
+
+default more_than_one_member := false
+
+# will be assigned `false` even if input.members is undefined
+more_than_one_member if count(input.members) > 1
+```
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  idiomatic:
+    boolean-assignment:
+      # one of "error", "warning", "ignore"
+      level: error
+```
+
+## Related Resources
+
+- Styra Blog: [How to express OR in Rego](https://www.styra.com/blog/how-to-express-or-in-rego/)
+- Regal Docs: [default-over-else](https://docs.styra.com/regal/rules/style/default-over-else)
+
+## 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)!

docs/rules/idiomatic/equals-pattern-matching.md

@@ -98,6 +98,10 @@ rules:
       level: error
 ```
 
+## Related Resources
+
+- Styra Blog: [How to express OR in Rego](https://www.styra.com/blog/how-to-express-or-in-rego/)
+
 ## Community
 
 If you think you've found a problem with this rule or its documentation, would like to suggest improvements, new rules,

docs/rules/idiomatic/no-defined-entrypoint.md

@@ -89,8 +89,8 @@ rules:
 
 ## Related Resources
 
-- OPA Docs [Metadata](https://www.openpolicyagent.org/docs/latest/policy-language/#metadata)
-- OPA Docs [Entrypoint](https://www.openpolicyagent.org/docs/latest/policy-language/#entrypoint)
+- OPA Docs: [Metadata](https://www.openpolicyagent.org/docs/latest/policy-language/#metadata)
+- OPA Docs: [Entrypoint](https://www.openpolicyagent.org/docs/latest/policy-language/#entrypoint)
 
 ## Community
 

docs/rules/idiomatic/non-raw-regex-pattern.md

@@ -56,7 +56,7 @@ rules:
 
 ## Related Resources
 
-- Rego Style Guide [Use raw strings for regex patterns](https://github.com/StyraInc/rego-style-guide#use-raw-strings-for-regex-patterns)
+- Rego Style Guide: [Use raw strings for regex patterns](https://github.com/StyraInc/rego-style-guide#use-raw-strings-for-regex-patterns)
 - OPA Docs: [Regex Functions Reference](https://www.openpolicyagent.org/docs/latest/policy-reference/#regex)
 
 ## Community

docs/rules/idiomatic/use-if.md

@@ -63,7 +63,7 @@ rules:
 ## Related Resources
 
 - Regal Docs: [use-contains](https://docs.styra.com/regal/rules/idiomatic/use-contains)
-- OPA Docs [Future Keywords](https://www.openpolicyagent.org/docs/latest/policy-language/#future-keywords)
+- OPA Docs: [Future Keywords](https://www.openpolicyagent.org/docs/latest/policy-language/#future-keywords)
 
 ## Community
 

docs/rules/idiomatic/use-in-operator.md

@@ -47,7 +47,7 @@ rules:
 
 ## Related Resources
 
-- Rego Style Guide [Use `in` to check for membership](https://github.com/StyraInc/rego-style-guide#use-in-to-check-for-membership)
+- Rego Style Guide: [Use `in` to check for membership](https://github.com/StyraInc/rego-style-guide#use-in-to-check-for-membership)
 
 ## Community
 

docs/rules/idiomatic/use-some-for-output-vars.md

@@ -103,9 +103,9 @@ rules:
 
 ## Related Resources
 
-- Rego Style Guide [Don't use undeclared variables](https://github.com/StyraInc/rego-style-guide#dont-use-undeclared-variables)
+- Rego Style Guide: [Don't use undeclared variables](https://github.com/StyraInc/rego-style-guide#dont-use-undeclared-variables)
 - OPA Docs: [The `some` keyword](https://www.openpolicyagent.org/docs/latest/policy-language/#some-keyword)
-- Wikipedia [Unification](https://en.wikipedia.org/wiki/Unification_(computer_science))
+- Wikipedia: [Unification](https://en.wikipedia.org/wiki/Unification_(computer_science))
 
 ## Community
 

docs/rules/style/opa-fmt.md

@@ -42,7 +42,7 @@ rules:
 
 ## Related Resources
 
-- OPA Docs [CLI Reference `opa fmt`](https://www.openpolicyagent.org/docs/latest/cli/#opa-fmt)
+- OPA Docs: [CLI Reference `opa fmt`](https://www.openpolicyagent.org/docs/latest/cli/#opa-fmt)
 
 ## Community
 

docs/rules/style/prefer-snake-case.md

@@ -52,7 +52,7 @@ rules:
 
 ## Related Resources
 
-- Rego Style Guide [Prefer snake_case for rule names and variables](https://github.com/StyraInc/rego-style-guide#prefer-snake_case-for-rule-names-and-variables)
+- Rego Style Guide: [Prefer snake_case for rule names and variables](https://github.com/StyraInc/rego-style-guide#prefer-snake_case-for-rule-names-and-variables)
 
 ## Community
 

docs/rules/style/todo-comment.md

@@ -48,7 +48,7 @@ rules:
 
 ## Related Resources
 
-- [DEV Community: //TODO: Write a better comment](https://dev.to/adammc331/todo-write-a-better-comment-4c8c)
+- DEV Community: [//TODO: Write a better comment](https://dev.to/adammc331/todo-write-a-better-comment-4c8c)
 
 ## Community
 

docs/rules/style/unnecessary-some.md

@@ -62,7 +62,7 @@ rules:
 
 ## Related Resources
 
-- OPA Docs [Membership and iteration: `in`](https://www.openpolicyagent.org/docs/latest/policy-language/#membership-and-iteration-in)
+- OPA Docs: [Membership and iteration: `in`](https://www.openpolicyagent.org/docs/latest/policy-language/#membership-and-iteration-in)
 
 ## Community
 

e2e/testdata/violations/most_violations.rego

@@ -91,6 +91,8 @@ prefer_set_or_object_rule := {x | some x in input; x == "violation"}
 
 equals_pattern_matching(x) := x == "x"
 
+boolean_assignment := 1 < input.two
+
 ### Style ###
 
 # avoid-get-and-list-prefix