Rule: use-rego-v1 (#538)

Recommend using `import rego.v1`. Also: * Ignore `import future` rules when `rego.v1` rule in capabilities * Update the documentation for all rules to use the `rego.v1` import * Update all Rego used in tests to use `rego.v1` import Signed-off-by: Anders Eknert <[email protected]>
StyraInc · Feb 5, 2024 · bc27c76 · bc27c76
1 parent c911cb8
commit bc27c76
Show file tree

Hide file tree

Showing 51 changed files with 309 additions and 165 deletions.
diff --git a/README.md b/README.md
@@ -97,15 +97,16 @@ First, author some Rego!
 ```rego
 package authz
 
-import future.keywords
+import rego.v1
 
 default allow = false
 
-deny if {
-    "admin" != input.user.roles[_]
+allow if {
+    isEmployee
+    "developer" in input.user.roles
 }
 
-allow if not deny
+isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
 ```
 
 Next, run `regal lint` pointed at one or more files or directories to have them linted.
@@ -116,19 +117,12 @@ regal lint policy/
 <!-- markdownlint-capture -->
 <!-- markdownlint-disable MD010 -->
 ```text
-Rule:         	not-equals-in-loop
-Description:  	Use of != in loop
-Category:     	bugs
-Location:     	policy/authz.rego:8:10
-Text:         	"admin" != input.user.roles[_]
-Documentation:	https://docs.styra.com/regal/rules/bugs/not-equals-in-loop
-
-Rule:         	implicit-future-keywords
-Description:  	Use explicit future keyword imports
-Category:     	imports
-Location:     	policy/authz.rego:3:8
-Text:         	import future.keywords
-Documentation:	https://docs.styra.com/regal/rules/imports/implicit-future-keywords
+Rule:         	non-raw-regex-pattern
+Description:  	Use raw strings for regex patterns
+Category:     	idiomatic
+Location:     	policy/authz.rego:12:27
+Text:         	isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
+Documentation:	https://docs.styra.com/regal/rules/idiomatic/non-raw-regex-pattern
 
 Rule:         	use-assignment-operator
 Description:  	Prefer := over = for assignment
@@ -137,6 +131,13 @@ Location:     	policy/authz.rego:5:1
 Text:         	default allow = false
 Documentation:	https://docs.styra.com/regal/rules/style/use-assignment-operator
 
+Rule:         	prefer-snake-case
+Description:  	Prefer snake_case for names
+Category:     	style
+Location:     	policy/authz.rego:12:1
+Text:         	isEmployee if regex.match("@acmecorp\\.com$", input.user.email)
+Documentation:	https://docs.styra.com/regal/rules/style/prefer-snake-case
+
 1 file linted. 3 violations found.
 ```
 <!-- markdownlint-restore -->
@@ -224,6 +225,7 @@ The following rules are currently available:
 | imports   | [prefer-package-imports](https://docs.styra.com/regal/rules/imports/prefer-package-imports)         | Prefer importing packages over rules                      |
 | imports   | [redundant-alias](https://docs.styra.com/regal/rules/imports/redundant-alias)                       | Redundant alias                                           |
 | imports   | [redundant-data-import](https://docs.styra.com/regal/rules/imports/redundant-data-import)           | Redundant import of data                                  |
+| imports   | [use-rego-v1](https://docs.styra.com/regal/rules/imports/use-rego-v1)                               | Use `import rego.v1`                                      |
 | style     | [avoid-get-and-list-prefix](https://docs.styra.com/regal/rules/style/avoid-get-and-list-prefix)     | Avoid `get_` and `list_` prefix for rules and functions   |
 | style     | [chained-rule-body](https://docs.styra.com/regal/rules/style/chained-rule-body)                     | Avoid chaining rule bodies                                |
 | style     | [default-over-else](https://docs.styra.com/regal/rules/style/default-over-else)                     | Prefer default assignment over fallback else              |

diff --git a/bundle/regal/config/provided/data.yaml b/bundle/regal/config/provided/data.yaml
@@ -77,6 +77,8 @@ rules:
       level: error
     redundant-data-import:
       level: error
+    use-rego-v1:
+      level: error
   style:
     avoid-get-and-list-prefix:
       level: error

diff --git a/bundle/regal/regal.rego b/bundle/regal/regal.rego
@@ -7,3 +7,5 @@
 # schemas:
 # - input: schema.regal.ast
 package regal
+
+import rego.v1
diff --git a/bundle/regal/rules/idiomatic/use_if.rego b/bundle/regal/rules/idiomatic/use_if.rego
@@ -8,10 +8,6 @@ import data.regal.ast
 import data.regal.capabilities
 import data.regal.result
 
-# Note: think more about what UX we want when import_rego_v1
-# capbility is available. Should we simply just recommend that
-# and silence this rule in that case? I'm inclined to say yes.
-
 # METADATA
 # description: Missing capability for keyword `if`
 # custom:

diff --git a/bundle/regal/rules/imports/use_rego_v1.rego b/bundle/regal/rules/imports/use_rego_v1.rego
@@ -0,0 +1,21 @@
+# METADATA
+# description: Use `import rego.v1`
+package regal.rules.imports["use-rego-v1"]
+
+import rego.v1
+
+import data.regal.ast
+import data.regal.capabilities
+import data.regal.result
+
+# METADATA
+# description: Missing capability for `import rego.v1`
+# custom:
+#   severity: warning
+notices contains result.notice(rego.metadata.chain()) if not capabilities.has_rego_v1_feature
+
+report contains violation if {
+	not ast.imports_has_path(ast.imports, ["rego", "v1"])
+
+	violation := result.fail(rego.metadata.chain(), result.location(input["package"]))
+}
diff --git a/bundle/regal/rules/imports/use_rego_v1_test.rego b/bundle/regal/rules/imports/use_rego_v1_test.rego
@@ -0,0 +1,37 @@
+package regal.rules.imports["use-rego-v1_test"]
+
+import rego.v1
+
+import data.regal.capabilities
+import data.regal.config
+import data.regal.rules.imports["use-rego-v1"] as rule
+
+test_fail_missing_rego_v1_import if {
+	r := rule.report with input as regal.parse_module("policy.rego", `package policy
+	import future.keywords
+
+	foo if not bar
+	`)
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == {{
+		"category": "imports",
+		"description": "Use `import rego.v1`",
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/use-rego-v1", "imports"),
+		}],
+		"title": "use-rego-v1",
+		"location": {"col": 1, "file": "policy.rego", "row": 1, "text": "package policy"},
+		"level": "error",
+	}}
+}
+
+test_success_rego_v1_import if {
+	r := rule.report with input as regal.parse_module("policy.rego", `package policy
+	import rego.v1
+
+	foo if not bar
+	`)
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == set()
+}
diff --git a/docs/custom-rules.md b/docs/custom-rules.md
@@ -118,8 +118,7 @@ An example policy to implement this requirement might look something like this:
 # - input: schema.regal.ast
 package custom.regal.rules.naming["acme-corp-package"]
 
-import future.keywords.contains
-import future.keywords.if
+import rego.v1
 
 import data.regal.result
 
@@ -186,25 +185,23 @@ from files, and one that actually lints and reports violations using that data.
 # - input: schema.regal.ast
 package custom.regal.rules.organizational["at-least-one-allow"]
 
-import future.keywords.contains
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 import data.regal.ast
 import data.regal.result
 
 aggregate contains entry if {
     # ast.rules is input.rules with functions filtered out
     some rule in ast.rules
-    
+
     # search for rule named alllow
     ast.name(rule) == "allow"
-    
+
     # make sure it's a default assignemnt
     # ideally we'll want more than that, but the *requiremnt* is only
     # that such a rule exists...
     rule["default"] == true
-    
+
     # ...and that it defaults to false
     rule.head.value.type == "boolean"
     rule.head.value.value == false

diff --git a/docs/rules/bugs/constant-condition.md b/docs/rules/bugs/constant-condition.md
@@ -8,7 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow if {
     1 == 1
@@ -32,7 +32,7 @@ harmless, it has no place in production policy, and should be replaced or remove
 This linter rule provides the following configuration options:
 
 ```yaml
-rules: 
+rules:
   bugs:
     constant-condition:
       # one of "error", "warning", "ignore"

diff --git a/docs/rules/bugs/duplicate-rule.md b/docs/rules/bugs/duplicate-rule.md
@@ -8,7 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow if user.is_admin
 
@@ -22,7 +22,7 @@ allow if user.is_admin
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow if user.is_admin
 

diff --git a/docs/rules/bugs/if-empty-object.md b/docs/rules/bugs/if-empty-object.md
@@ -8,7 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow if {}
 ```

diff --git a/docs/rules/bugs/inconsistent-args.md b/docs/rules/bugs/inconsistent-args.md
@@ -8,8 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 find_vars(rule, node) if node in rule
 
@@ -24,8 +23,7 @@ find_vars(node, rule) if {
 ```rego
 package policy
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 find_vars(rule, node) if node in rule
 
@@ -48,8 +46,7 @@ Using wildcards (`_`) in place of unused arguments is always allowed, and in fac
 ```rego
 package policy
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 find_vars(rule, node) if node in rule
 
@@ -68,7 +65,7 @@ also allowed.
 This linter rule provides the following configuration options:
 
 ```yaml
-rules: 
+rules:
   bugs:
     inconsistent-args:
       # one of "error", "warning", "ignore"

diff --git a/docs/rules/bugs/not-equals-in-loop.md b/docs/rules/bugs/not-equals-in-loop.md
@@ -8,7 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 deny if {
     "admin" != input.user.roles[_]
@@ -19,8 +19,7 @@ deny if {
 ```rego
 package policy
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 deny if {
     not "admin" in input.user.roles
@@ -39,7 +38,7 @@ day. If it doesn't mean "not in", what does it mean?
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 deny if {
     "admin" != input.user.roles[_]

diff --git a/docs/rules/bugs/rule-named-if.md b/docs/rules/bugs/rule-named-if.md
@@ -17,7 +17,7 @@ allow := true if {
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow := true if {
     authorized
@@ -26,10 +26,10 @@ allow := true if {
 
 ## Rationale
 
-Forgetting to import the `if` keyword (using `import future.keywords.if`) is a common mistake. While this often results
-in a parse error, there are some situations where the parser can't tell if the `if` is intended to be used as the
-imported keyword, or a new rule named `if`. This is almost always a mistake, and if it isn't — consider using a better
-name for your rule!
+Forgetting to import the `if` keyword (using `import future.keywords.if`, or from OPA v0.59.0+ `import rego.v1`) is a
+common mistake. While this often results in a parse error, there are some situations where the parser can't tell if the
+`if` is intended to be used as the imported keyword, or a new rule named `if`. This is almost always a mistake, and if
+it isn't — consider using a better name for your rule!
 
 ## Configuration Options
 

diff --git a/docs/rules/bugs/unused-return-value.md b/docs/rules/bugs/unused-return-value.md
@@ -8,7 +8,7 @@
 ```rego
 package policy
 
-import future.keywords.if
+import rego.v1
 
 allow if {
     # return value unused
@@ -40,7 +40,7 @@ is almost certainly a mistake.
 This linter rule provides the following configuration options:
 
 ```yaml
-rules: 
+rules:
   bugs:
     unused-return-value:
       # one of "error", "warning", "ignore"

diff --git a/docs/rules/community/circular-import.md b/docs/rules/community/circular-import.md
@@ -23,8 +23,7 @@ graph LR
 # authz.rego
 package authz
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 import data.shared
 
@@ -71,8 +70,7 @@ graph LR
 # authz.rego
 package authz
 
-import future.keywords.if
-import future.keywords.in
+import rego.v1
 
 import data.shared
 

diff --git a/docs/rules/community/double-negative.md b/docs/rules/community/double-negative.md
@@ -15,7 +15,7 @@
 ```rego
 package negative
 
-import future.keywords.if
+import rego.v1
 
 fine if not not_fine
 
@@ -30,7 +30,7 @@ without_friends if count(input.friends) == 0
 ```rego
 package negative
 
-import future.keywords.if
+import rego.v1
 
 fine if input.fine == true