Skip to content

Commit

Permalink
Rule: use-object-keys
Browse files Browse the repository at this point in the history
Fixes #1044

Signed-off-by: Anders Eknert <anders@styra.com>
  • Loading branch information
anderseknert committed Feb 6, 2025
1 parent a4e3592 commit 90be3f7
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ The following rules are currently available:
| idiomatic | [use-contains](https://docs.styra.com/regal/rules/idiomatic/use-contains) | Use the `contains` keyword |
| idiomatic | [use-if](https://docs.styra.com/regal/rules/idiomatic/use-if) | Use the `if` keyword |
| idiomatic | [use-in-operator](https://docs.styra.com/regal/rules/idiomatic/use-in-operator) | Use in to check for membership |
| idiomatic | [use-object-keys](https://docs.styra.com/regal/rules/idiomatic/use-object-keys) | Prefer to use `object.keys` |
| idiomatic | [use-some-for-output-vars](https://docs.styra.com/regal/rules/idiomatic/use-some-for-output-vars) | Use `some` to declare output variables |
| idiomatic | [use-strings-count](https://docs.styra.com/regal/rules/idiomatic/use-strings-count) | Use `strings.count` where possible |
| imports | [avoid-importing-input](https://docs.styra.com/regal/rules/imports/avoid-importing-input) | Avoid importing input |
Expand Down
2 changes: 2 additions & 0 deletions bundle/regal/config/provided/data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ rules:
level: error
use-in-operator:
level: error
use-object-keys:
level: error
use-some-for-output-vars:
level: error
use-strings-count:
Expand Down
70 changes: 70 additions & 0 deletions bundle/regal/rules/idiomatic/use-object-keys/use_object_keys.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# METADATA
# description: Prefer to use `object.keys`
package regal.rules.idiomatic["use-object-keys"]

import data.regal.ast
import data.regal.result

# {k | some k, _ in object}
report contains violation if {
comprehension := ast.found.comprehensions[_][_]

comprehension.type == "setcomprehension"
comprehension.value.term.type == "var"
count(comprehension.value.body) == 1

symbol := comprehension.value.body[0].terms.symbols[0]

# some k, _ in object
symbol.type == "call"
symbol.value[0].type == "ref"
symbol.value[0].value[0].value == "internal"
symbol.value[0].value[1].value == "member_3"

# same 'k' var value as in the head
symbol.value[1].type == "var"
symbol.value[1].value == comprehension.value.term.value

# we don't care what symbol.value[2] is, as it's not assigned in the head
# but symbol.value[3] should be a ref without vars

symbol.value[3].type == "ref"

ast.static_ref(symbol.value[3].value)

violation := result.fail(rego.metadata.chain(), result.location(comprehension))
}

# {k | input.object[k]}
# {k | some k; input.object[k]}
report contains violation if {
comprehension := ast.found.comprehensions[_][_]

comprehension.type == "setcomprehension"
comprehension.value.term.type == "var"

ref := _ref(comprehension.value.body)

vars := [part |
some part in array.slice(ref, 1, 128)
part.type == "var"
]

count(vars) == 1
vars[0].value == comprehension.value.term.value

violation := result.fail(rego.metadata.chain(), result.location(comprehension))
}

# {k | input.object[k]}
_ref(exprs) := exprs[0].terms.value if {
count(exprs) == 1
exprs[0].terms.type == "ref"
}

# {k | some k; input.object[k]}
_ref(exprs) := exprs[1].terms.value if {
count(exprs) == 2
exprs[0].terms.symbols[0].type == "var"
exprs[1].terms.type == "ref"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package regal.rules.idiomatic["use-object-keys_test"]

import data.regal.ast
import data.regal.config

import data.regal.rules.idiomatic["use-object-keys"] as rule

test_fail_use_object_keys_not_some_in_comprehension if {
r := rule.report with input as ast.policy(`comp := {k | some k, _ in input.object}`)

r == {{
"category": "idiomatic",
"description": "Prefer to use `object.keys`",
"level": "error",
"location": {
"col": 9,
"end": {
"col": 40,
"row": 3,
},
"file": "policy.rego",
"row": 3,
"text": "comp := {k | some k, _ in input.object}",
},
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-object-keys", "idiomatic"),
}],
"title": "use-object-keys",
}}
}

test_fail_use_object_keys_not_some_comprehension if {
r := rule.report with input as ast.policy(`comp := {k | some k; input.object[k]}`)

r == {{
"category": "idiomatic",
"description": "Prefer to use `object.keys`",
"level": "error",
"location": {
"col": 9,
"end": {
"col": 38,
"row": 3,
},
"file": "policy.rego",
"row": 3,
"text": "comp := {k | some k; input.object[k]}",
},
"related_resources": [{
"description": "documentation",
"ref": config.docs.resolve_url("$baseUrl/$category/use-object-keys", "idiomatic"),
}],
"title": "use-object-keys",
}}
}
51 changes: 51 additions & 0 deletions docs/rules/idiomatic/use-object-keys.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# use-object-keys

**Summary**: Prefer to use `object.keys`

**Category**: Idiomatic

**Avoid**
```rego
package policy
keys := {k | some k, _ in input.object}
# or
keys := {k | some k; input.object[k]}
```

**Prefer**
```rego
package policy
keys := object.keys(input.object)
```

## Rationale

Instead of using a set comprehension to collect keys from an object, prefer to use the built-in function
[object.keys](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-object-objectkeys).
This option is both more declarative and better conveys the intent of the code.

## Configuration Options

This linter rule provides the following configuration options:

```yaml
rules:
idiomatic:
use-object-keys:
# one of "error", "warning", "ignore"
level: error
```
## Related Resources
- OPA Docs: [object.keys](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-object-objectkeys)
## 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/#)!
2 changes: 2 additions & 0 deletions e2e/testdata/violations/most_violations.rego
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ incremental if input.y

prefer_strings_count := count(indexof_n("foobarbaz", "a"))

use_object_keys := {k | some k; input.object[k]}

### Style ###

# avoid-get-and-list-prefix
Expand Down

0 comments on commit 90be3f7

Please # to comment.