-
Notifications
You must be signed in to change notification settings - Fork 34
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rule:
argument-always-wildcard (#883)
I did not expect that I'd have to fix any issues in Regal as a result of this, but that was a rather nice surprise! Fixes #872 Signed-off-by: Anders Eknert <[email protected]>
- Loading branch information
1 parent
a4c96a4
commit 8110bc5
Showing
7 changed files
with
246 additions
and
17 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # METADATA | ||
| # description: Argument is always a wildcard | ||
| package regal.rules.bugs["argument-always-wildcard"] | ||
|
|
||
| import rego.v1 | ||
|
|
||
| import data.regal.ast | ||
| import data.regal.result | ||
|
|
||
| function_groups[name] contains fn if { | ||
| some fn in ast.functions | ||
|
|
||
| name := ast.ref_to_string(fn.head.ref) | ||
| } | ||
|
|
||
| report contains violation if { | ||
| some functions in function_groups | ||
|
|
||
| fn := any_member(functions) | ||
|
|
||
| some pos in numbers.range(0, count(fn.head.args) - 1) | ||
|
|
||
| every function in functions { | ||
| function.head.args[pos].type == "var" | ||
| startswith(function.head.args[pos].value, "$") | ||
| } | ||
|
|
||
| violation := result.fail(rego.metadata.chain(), result.location(fn.head.args[pos])) | ||
| } | ||
|
|
||
| any_member(s) := [x | some x in s][0] |
97 changes: 97 additions & 0 deletions
97
bundle/regal/rules/bugs/argument_always_wildcard_test.rego
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package regal.rules.bugs["argument-always-wildcard_test"] | ||
|
|
||
| import rego.v1 | ||
|
|
||
| import data.regal.ast | ||
| import data.regal.config | ||
|
|
||
| import data.regal.rules.bugs["argument-always-wildcard"] as rule | ||
|
|
||
| test_fail_single_function_single_argument_always_a_wildcard if { | ||
| module := ast.with_rego_v1(` | ||
| f(_) := 1 | ||
| `) | ||
|
|
||
| r := rule.report with input as module | ||
| r == {{ | ||
| "category": "bugs", | ||
| "description": "Argument is always a wildcard", | ||
| "level": "error", | ||
| "location": {"col": 4, "file": "policy.rego", "row": 6, "text": "\tf(_) := 1"}, | ||
| "related_resources": [{ | ||
| "description": "documentation", | ||
| "ref": config.docs.resolve_url("$baseUrl/$category/argument-always-wildcard", "bugs"), | ||
| }], | ||
| "title": "argument-always-wildcard", | ||
| }} | ||
| } | ||
|
|
||
| test_fail_single_argument_always_a_wildcard if { | ||
| module := ast.with_rego_v1(` | ||
| f(_) := 1 | ||
| f(_) := 2 | ||
| `) | ||
|
|
||
| r := rule.report with input as module | ||
| r == {{ | ||
| "category": "bugs", | ||
| "description": "Argument is always a wildcard", | ||
| "level": "error", | ||
| "location": {"col": 4, "file": "policy.rego", "row": 6, "text": "\tf(_) := 1"}, | ||
| "related_resources": [{ | ||
| "description": "documentation", | ||
| "ref": config.docs.resolve_url("$baseUrl/$category/argument-always-wildcard", "bugs"), | ||
| }], | ||
| "title": "argument-always-wildcard", | ||
| }} | ||
| } | ||
|
|
||
| test_fail_single_argument_always_a_wildcard_default_function if { | ||
| module := ast.with_rego_v1(` | ||
| default f(_) := 1 | ||
| f(_) := 2 | ||
| `) | ||
|
|
||
| r := rule.report with input as module | ||
| r == {{ | ||
| "category": "bugs", | ||
| "description": "Argument is always a wildcard", | ||
| "level": "error", | ||
| "location": {"col": 12, "file": "policy.rego", "row": 6, "text": "\tdefault f(_) := 1"}, | ||
| "related_resources": [{ | ||
| "description": "documentation", | ||
| "ref": config.docs.resolve_url("$baseUrl/$category/argument-always-wildcard", "bugs"), | ||
| }], | ||
| "title": "argument-always-wildcard", | ||
| }} | ||
| } | ||
|
|
||
| test_fail_multiple_argument_always_a_wildcard if { | ||
| module := ast.with_rego_v1(` | ||
| f(x, _) := x + 1 | ||
| f(x, _) := x + 2 | ||
| `) | ||
|
|
||
| r := rule.report with input as module | ||
| r == {{ | ||
| "category": "bugs", | ||
| "description": "Argument is always a wildcard", | ||
| "level": "error", | ||
| "location": {"col": 7, "file": "policy.rego", "row": 6, "text": "\tf(x, _) := x + 1"}, | ||
| "related_resources": [{ | ||
| "description": "documentation", | ||
| "ref": config.docs.resolve_url("$baseUrl/$category/argument-always-wildcard", "bugs"), | ||
| }], | ||
| "title": "argument-always-wildcard", | ||
| }} | ||
| } | ||
|
|
||
| test_success_multiple_argument_not_always_a_wildcard if { | ||
| module := ast.with_rego_v1(` | ||
| f(x, _) := x + 1 | ||
| f(_, y) := y + 2 | ||
| `) | ||
|
|
||
| r := rule.report with input as module | ||
| r == set() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| # argument-always-wildcard | ||
|
|
||
| **Summary**: Argument is always a wildcard | ||
|
|
||
| **Category**: Bugs | ||
|
|
||
| **Avoid** | ||
| ```rego | ||
| package policy | ||
| import rego.v1 | ||
| # there's only one definition of the last_name function in | ||
| # this package, and the second argument is never used | ||
| last_name(name, _) := lname if { | ||
| parts := split(name, " ") | ||
| lname := parts[count(parts) - 1] | ||
| } | ||
| ``` | ||
|
|
||
| **Prefer** | ||
| ```rego | ||
| package policy | ||
| import rego.v1 | ||
| last_name(name) := lname if { | ||
| parts := split(name, " ") | ||
| lname := parts[count(parts) - 1] | ||
| } | ||
| ``` | ||
|
|
||
| ## Rationale | ||
|
|
||
| Function definitions may use wildcard variables as arguments to indicate that the value is not used in the body of | ||
| the function. This helps make the function definition more readable, as it's immediately clear which of the arguments | ||
| are used in that definition of the function. This is particularly useful for incrementally defined functions: | ||
|
|
||
| ```rego | ||
| package policy | ||
| import rego.v1 | ||
| default authorized(_, _) := false | ||
| authorized(user, _) if { | ||
| # some logic to determine if authorized | ||
| } | ||
| # or | ||
| authorized(user, _) if { | ||
| # some further logic to determine if authorized | ||
| } | ||
| ``` | ||
|
|
||
| In the example above, the second argument is a wildcard in all definitions, and could just as well be removed for a | ||
| cleaner definition. More likely, the argument was meant to be _used_, if only in one of the definitions: | ||
|
|
||
| ```rego | ||
| package policy | ||
| import rego.v1 | ||
| default authorized(_, _) := false | ||
| authorized(user, _) if { | ||
| # some logic to determine if authorized | ||
| } | ||
| # or | ||
| authorized(_, request) if { | ||
| # some further logic to determine if authorized | ||
| } | ||
| ``` | ||
|
|
||
| ## Configuration Options | ||
|
|
||
| This linter rule provides the following configuration options: | ||
|
|
||
| ```yaml | ||
| rules: | ||
| bugs: | ||
| argument-always-wildcard: | ||
| # 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)! |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters