Skip to content

Commit

Permalink
Rename unused-return-value -> unassigned-return-value (#581)
Browse files Browse the repository at this point in the history
Fixes #297

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert authored Feb 28, 2024
1 parent 21a0dc4 commit c3d3fe0
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 63 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ The following rules are currently available:
| bugs | [rule-named-if](https://docs.styra.com/regal/rules/bugs/rule-named-if) | Rule named "if" |
| bugs | [rule-shadows-builtin](https://docs.styra.com/regal/rules/bugs/rule-shadows-builtin) | Rule name shadows built-in |
| bugs | [top-level-iteration](https://docs.styra.com/regal/rules/bugs/top-level-iteration) | Iteration in top-level assignment |
| bugs | [unused-return-value](https://docs.styra.com/regal/rules/bugs/unused-return-value) | Non-boolean return value unused |
| bugs | [unassigned-return-value](https://docs.styra.com/regal/rules/bugs/unassigned-return-value) | Non-boolean return value unassigned |
| bugs | [zero-arity-function](https://docs.styra.com/regal/rules/bugs/zero-arity-function) | Avoid functions without args |
| custom | [forbidden-function-call](https://docs.styra.com/regal/rules/custom/forbidden-function-call) | Forbidden function call |
| custom | [naming-convention](https://docs.styra.com/regal/rules/custom/naming-convention) | Naming convention violation |
Expand Down
2 changes: 1 addition & 1 deletion bundle/regal/config/provided/data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ rules:
level: error
top-level-iteration:
level: error
unused-return-value:
unassigned-return-value:
level: error
zero-arity-function:
level: error
Expand Down
4 changes: 2 additions & 2 deletions bundle/regal/rules/bugs/unused_return_value.rego
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# METADATA
# description: Non-boolean return value unused
package regal.rules.bugs["unused-return-value"]
# description: Non-boolean return value unassigned
package regal.rules.bugs["unassigned-return-value"]

import rego.v1

Expand Down
10 changes: 5 additions & 5 deletions bundle/regal/rules/bugs/unused_return_value_test.rego
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package regal.rules.bugs["unused-return-value_test"]
package regal.rules.bugs["unassigned-return-value_test"]

import rego.v1

import data.regal.ast
import data.regal.capabilities
import data.regal.config
import data.regal.rules.bugs["unused-return-value"] as rule
import data.regal.rules.bugs["unassigned-return-value"] as rule

test_fail_unused_return_value if {
r := rule.report with input as ast.with_rego_v1(`allow if {
Expand All @@ -14,14 +14,14 @@ test_fail_unused_return_value if {
with data.internal.combined_config as {"capabilities": capabilities.provided}
r == {{
"category": "bugs",
"description": "Non-boolean return value unused",
"description": "Non-boolean return value unassigned",
"level": "error",
"location": {"col": 3, "file": "policy.rego", "row": 6, "text": "\t\tindexof(\"s\", \"s\")"},
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/unused-return-value", "bugs"),
"ref": config.docs.resolve_url("$baseUrl/$category/unassigned-return-value", "bugs"),
}],
"title": "unused-return-value",
"title": "unassigned-return-value",
}}
}

Expand Down
54 changes: 54 additions & 0 deletions docs/rules/bugs/unassigned-return-value.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# unassigned-return-value

**Summary**: Non-boolean return value unassigned

**Category**: Bugs

**Avoid**
```rego
package policy
import rego.v1
allow if {
# return value not assigned
lower(input.user.name)
# ...
}
```

**Prefer**
```rego
package policy
allow if {
# return value assigned
name_lower := lower(input.user.name)
# ...
}
```

## Rationale

Calling a built-in function that returns a non-boolean value without actually assigning the returned value is almost
always a mistake. Only return of `false` or undefined will cause evaluation to halt, so a function that e.g. always
returns a string will always be evaluated as "truthy". But more importantly — not handling the return value in that case
is almost certainly a mistake.

## Configuration Options

This linter rule provides the following configuration options:

```yaml
rules:
bugs:
unassigned-return-value:
# 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)!
53 changes: 2 additions & 51 deletions docs/rules/bugs/unused-return-value.md
Original file line number Diff line number Diff line change
@@ -1,54 +1,5 @@
# unused-return-value

**Summary**: Non-boolean return value unused
## Please Note

**Category**: Bugs

**Avoid**
```rego
package policy
import rego.v1
allow if {
# return value unused
lower(input.user.name)
# ...
}
```

**Prefer**
```rego
package policy
allow if {
# return value assigned
name_lower := lower(input.user.name)
# ...
}
```

## Rationale

Calling a built-in function that returns a non-boolean value without actually *using* the returned value is almost
always a mistake. Only return of `false` or undefined will cause evaluation to halt, so a function that e.g. always
returns a string will always be evaluated as "truthy". But more importantly — not handling the return value in that case
is almost certainly a mistake.

## Configuration Options

This linter rule provides the following configuration options:

```yaml
rules:
bugs:
unused-return-value:
# 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 rule has been renamed to *unassigned-return-value* and can be found [here](../unassigned-return-value.md).
4 changes: 1 addition & 3 deletions e2e/testdata/violations/most_violations.rego
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ contains := true

top_level_iteration := input[_]

unused_return_value if {
indexof("foo", "o")
}
unassigned_return_value if indexof("foo", "o")

zero_arity_function() := true

Expand Down

0 comments on commit c3d3fe0

Please sign in to comment.