Rule: trailing-default-rule (#716)

Simple rule to check that `default` rules are placed before non-default rules.

Fixes #699

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

README.md

@@ -254,6 +254,7 @@ The following rules are currently available:
 | style       | [rule-length](https://docs.styra.com/regal/rules/style/rule-length)                                   | Max rule length exceeded                                  |
 | style       | [rule-name-repeats-package](https://docs.styra.com/regal/rules/style/rule-name-repeats-package)       | Rule name repeats package                                 |
 | style       | [todo-comment](https://docs.styra.com/regal/rules/style/todo-comment)                                 | Avoid TODO comments                                       |
+| style       | [trailing-default-rule](https://docs.styra.com/regal/rules/style/trailing-default-rule)               | Default rule should be declared first                     |
 | style       | [unconditional-assignment](https://docs.styra.com/regal/rules/style/unconditional-assignment)         | Unconditional assignment in rule body                     |
 | style       | [unnecessary-some](https://docs.styra.com/regal/rules/style/unnecessary-some)                         | Unnecessary use of `some`                                 |
 | style       | [use-assignment-operator](https://docs.styra.com/regal/rules/style/use-assignment-operator)           | Prefer := over = for assignment                           |

bundle/regal/config/provided/data.yaml

@@ -136,6 +136,8 @@ rules:
       level: error
     todo-comment:
       level: error
+    trailing-default-rule:
+      level: error
     unconditional-assignment:
       level: error
     unnecessary-some:

bundle/regal/rules/style/trailing_default_rule.rego

@@ -0,0 +1,24 @@
+# METADATA
+# description: Default rule should be declared first
+package regal.rules.style["trailing-default-rule"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.result
+
+report contains violation if {
+	some i, rule in input.rules
+
+	rule["default"] == true
+
+	name := ast.ref_to_string(rule.head.ref)
+	name in all_names(array.slice(input.rules, 0, i))
+
+	violation := result.fail(rego.metadata.chain(), result.location(rule))
+}
+
+all_names(rules) := {name |
+	some rule in rules
+	name := ast.ref_to_string(rule.head.ref)
+}

bundle/regal/rules/style/trailing_default_rule_test.rego

@@ -0,0 +1,40 @@
+package regal.rules.style["trailing-default-rule_test"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.config
+
+import data.regal.rules.style["trailing-default-rule"] as rule
+
+test_success_default_declared_first if {
+	module := ast.with_rego_v1(`
+	default foo := true
+
+	foo if true
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_fail_default_declared_after if {
+	module := ast.with_rego_v1(`
+	foo if true
+
+	default foo := true
+	`)
+
+	r := rule.report with input as module
+	r == {{
+		"category": "style",
+		"description": "Default rule should be declared first",
+		"level": "error",
+		"location": {"col": 2, "file": "policy.rego", "row": 8, "text": "\tdefault foo := true"},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/trailing-default-rule", "style"),
+		}],
+		"title": "trailing-default-rule",
+	}}
+}

docs/rules/style/function-arg-return.md

@@ -17,7 +17,6 @@ has_email(user) if {
 ```
 
 **Prefer**
-
 ```rego
 package policy
 

docs/rules/style/trailing-default-rule.md

@@ -0,0 +1,56 @@
+# trailing-default-rule
+
+**Summary**: Default rule should be declared first
+
+**Category**: Style
+
+**Avoid**
+```rego
+package policy
+
+import rego.v1
+
+allow if {
+    # some conditions
+}
+
+default allow := false
+```
+
+**Prefer**
+```rego
+package policy
+
+import rego.v1
+
+default allow := false
+
+allow if {
+    # some conditions
+}
+```
+
+## Rationale
+
+Presenting the default value of a rule (if one is used) before the conditional rule assignments is a common practice,
+and it's often easier to to reason about conditional assignments knowing there is a default fallback value in place.
+For that reason, it's recommended to follow the convention and place the default rule declaration before rules
+conditionally assigning values.
+
+## Configuration Options
+
+This linter rule provides the following configuration options:
+
+```yaml
+rules:
+  style:
+    trailing-default-rule:
+      # one of "error", "warning", "ignore"
+      level: error
+```
+
+## 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)!

e2e/testdata/violations/most_violations.rego

@@ -228,6 +228,9 @@ foo := "bar"
 # messy rule
 abc if false
 
+# trailing default rule
+default abc := true
+
 # dubious print sprintf
 y if {
 	print(sprintf("name is: %s domain is: %s", [input.name, input.domain]))