From 0ce82ae0afed1b28a12dd201aa68f9265c5f66d9 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Wed, 23 Oct 2024 13:14:38 +0200 Subject: [PATCH] Rule: `walk-no-path` (#1219) Suggest replacing the path variable with a wildcard where possible, as the walk built-in is able to optimize such calls. More info in the docs included in the PR. Also: - Add `ast.is_wildcard` convenience function and use it consistently Fixes #1154 Signed-off-by: Anders Eknert --- README.md | 1 + bundle/regal/ast/ast.rego | 29 ++++- bundle/regal/ast/ast_test.rego | 6 +- bundle/regal/ast/search.rego | 3 +- bundle/regal/config/provided/data.yaml | 3 +- .../completion/location/location_test.rego | 6 ++ .../argument_always_wildcard.rego | 2 +- .../inconsistent-args/inconsistent_args.rego | 2 +- .../not_equals_in_loop.rego | 3 +- .../unused_output_variable.rego | 22 +--- .../naming_convention_test.rego | 5 +- .../custom_has_key_construct.rego | 6 +- .../custom_in_construct.rego | 3 +- .../equals_pattern_matching.rego | 6 +- .../prefer_set_or_object_rule.rego | 3 +- .../use-in-operator/use_in_operator.rego | 5 +- .../use_some_for_output_vars.rego | 2 +- .../walk-no-path/walk_no_path.rego | 61 +++++++++++ .../walk-no-path/walk_no_path_test.rego | 101 ++++++++++++++++++ .../external_reference.rego | 2 +- docs/rules/performance/walk-no-path.md | 101 ++++++++++++++++++ e2e/testdata/violations/most_violations.rego | 5 + internal/update/update.rego | 3 +- 23 files changed, 331 insertions(+), 49 deletions(-) create mode 100644 bundle/regal/rules/performance/walk-no-path/walk_no_path.rego create mode 100644 bundle/regal/rules/performance/walk-no-path/walk_no_path_test.rego create mode 100644 docs/rules/performance/walk-no-path.md diff --git a/README.md b/README.md index 80fb0b91..18a2d416 100644 --- a/README.md +++ b/README.md @@ -254,6 +254,7 @@ The following rules are currently available: | imports | [unresolved-import](https://docs.styra.com/regal/rules/imports/unresolved-import) | Unresolved import | | imports | [use-rego-v1](https://docs.styra.com/regal/rules/imports/use-rego-v1) | Use `import rego.v1` | | performance | [defer-assignment](https://docs.styra.com/regal/rules/performance/defer-assignment) | Assignment can be deferred | +| performance | [walk-no-path](https://docs.styra.com/regal/rules/performance/walk-no-path) | Call to `walk` can be optimized | | performance | [with-outside-test-context](https://docs.styra.com/regal/rules/performance/with-outside-test-context) | `with` used outside test context | | 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 | diff --git a/bundle/regal/ast/ast.rego b/bundle/regal/ast/ast.rego index 2c9a0b3a..01d40b00 100644 --- a/bundle/regal/ast/ast.rego +++ b/bundle/regal/ast/ast.rego @@ -46,6 +46,13 @@ is_constant(term) if { not has_term_var(term.value) } +# METADATA +# description: true if provided term represents a wildcard (`_`) variable +is_wildcard(term) if { + term.type == "var" + startswith(term.value, "$") +} + default builtin_names := set() # METADATA @@ -160,7 +167,7 @@ rule_names contains ref_to_string(rule.head.ref) if some rule in rules # scope: document is_output_var(rule, var) if { # test the cheap and common case first, and 'else' only when it's not - startswith(var.value, "$") + is_wildcard(var) } else if { not var.value in (rule_names | imported_identifiers) # regal ignore:external-reference @@ -381,6 +388,26 @@ var_in_head(head, name) if { var.value == name } +# METADATA +# description: | +# true if var of `name` is referenced in any `calls` (likely, +# `ast.function_calls`) in the rule of given `rule_index` +# scope: document +var_in_call(calls, rule_index, name) if _var_in_arg(calls[rule_index][_].args[_], name) + +_var_in_arg(arg, name) if { + arg.type == "var" + arg.value == name +} + +_var_in_arg(arg, name) if { + arg.type in {"array", "object", "set"} + + some var in find_term_vars(arg) + + var.value == name +} + # METADATA # description: answers wether provided expression is an assignment (using `:=`) is_assignment(expr) if { diff --git a/bundle/regal/ast/ast_test.rego b/bundle/regal/ast/ast_test.rego index a6060a85..51ae762a 100644 --- a/bundle/regal/ast/ast_test.rego +++ b/bundle/regal/ast/ast_test.rego @@ -45,7 +45,7 @@ test_find_vars if { module := regal.parse_module("p.rego", policy) - vars := ast.find_vars(module.rules) + vars := ast.find_vars(module.rules) with data.internal.combined_config as {"capabilities": capabilities.provided} names := {var.value | some var in vars @@ -70,7 +70,7 @@ test_find_vars_comprehension_lhs if { module := regal.parse_module("p.rego", policy) - vars := ast.find_vars(module.rules) + vars := ast.find_vars(module.rules) with data.internal.combined_config as {"capabilities": capabilities.provided} names := {var.value | some var in vars @@ -236,6 +236,8 @@ test_find_names_in_scope if { allow_rule := module.rules[3] in_scope := ast.find_names_in_scope(allow_rule, {"col": 1, "row": 30}) with input as module + with data.internal.combined_config as {"capabilities": capabilities.provided} + in_scope == {"bar", "global", "comp", "allow", "a", "b", "c", "d", "e"} } diff --git a/bundle/regal/ast/search.rego b/bundle/regal/ast/search.rego index d6cd593f..0c4e537c 100644 --- a/bundle/regal/ast/search.rego +++ b/bundle/regal/ast/search.rego @@ -59,7 +59,6 @@ _find_some_in_decl_vars(value) := vars if { find_ref_vars(value) := [var | some i, var in value.value - # ignore first element, as it is the base ref (like input or data) i > 0 var.type == "var" ] @@ -249,7 +248,7 @@ found.comprehensions[rule_index] contains value if { find_vars_in_local_scope(rule, location) := [var | var := found.vars[_rule_index(rule)][_][_] # regal ignore:external-reference - not startswith(var.value, "$") + not is_wildcard(var) _before_location(rule, var, util.to_location_object(location)) ] diff --git a/bundle/regal/config/provided/data.yaml b/bundle/regal/config/provided/data.yaml index 9edd8dc0..23a5cd81 100644 --- a/bundle/regal/config/provided/data.yaml +++ b/bundle/regal/config/provided/data.yaml @@ -15,7 +15,6 @@ rules: duplicate-rule: level: error if-empty-object: - # deprecated with the introduction of if-object-literal level: ignore if-object-literal: level: error @@ -120,6 +119,8 @@ rules: performance: defer-assignment: level: error + walk-no-path: + level: error with-outside-test-context: level: error style: diff --git a/bundle/regal/lsp/completion/location/location_test.rego b/bundle/regal/lsp/completion/location/location_test.rego index 3fd6cc97..0e95f302 100644 --- a/bundle/regal/lsp/completion/location/location_test.rego +++ b/bundle/regal/lsp/completion/location/location_test.rego @@ -3,6 +3,7 @@ package regal.lsp.completion.location_test import rego.v1 import data.regal.ast +import data.regal.capabilities import data.regal.lsp.completion.location @@ -69,21 +70,26 @@ another if { r2 := location.find_locals(module.rules, {"row": 6, "col": 10}) with input as module with input.regal.file.lines as lines + with data.internal.combined_config as {"capabilities": capabilities.provided} r2 == {"x"} r3 := location.find_locals(module.rules, {"row": 10, "col": 1}) with input as module with input.regal.file.lines as lines + with data.internal.combined_config as {"capabilities": capabilities.provided} r3 == {"a", "b"} r4 := location.find_locals(module.rules, {"row": 10, "col": 6}) with input as module with input.regal.file.lines as lines + with data.internal.combined_config as {"capabilities": capabilities.provided} r4 == {"a", "b", "c"} r5 := location.find_locals(module.rules, {"row": 15, "col": 1}) with input as module with input.regal.file.lines as lines + with data.internal.combined_config as {"capabilities": capabilities.provided} r5 == {"x", "y"} r6 := location.find_locals(module.rules, {"row": 16, "col": 1}) with input as module with input.regal.file.lines as lines + with data.internal.combined_config as {"capabilities": capabilities.provided} r6 == {"x", "y", "z"} } diff --git a/bundle/regal/rules/bugs/argument-always-wildcard/argument_always_wildcard.rego b/bundle/regal/rules/bugs/argument-always-wildcard/argument_always_wildcard.rego index 13e685b0..e0d5ffa4 100644 --- a/bundle/regal/rules/bugs/argument-always-wildcard/argument_always_wildcard.rego +++ b/bundle/regal/rules/bugs/argument-always-wildcard/argument_always_wildcard.rego @@ -17,7 +17,7 @@ report contains violation if { every function in functions { function.head.args[pos].type == "var" - startswith(function.head.args[pos].value, "$") + ast.is_wildcard(function.head.args[pos]) } not _function_name_excepted(config.for_rule("bugs", "argument-always-wildcard"), name) diff --git a/bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego b/bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego index a1bbe226..5102a3dc 100644 --- a/bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego +++ b/bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego @@ -43,7 +43,7 @@ _inconsistent_args(position) if { named_vars := {arg.value | some arg in position arg.type == "var" - not startswith(arg.value, "$") + not ast.is_wildcard(arg) } count(named_vars) > 1 } diff --git a/bundle/regal/rules/bugs/not-equals-in-loop/not_equals_in_loop.rego b/bundle/regal/rules/bugs/not-equals-in-loop/not_equals_in_loop.rego index bb14fc46..2eccac57 100644 --- a/bundle/regal/rules/bugs/not-equals-in-loop/not_equals_in_loop.rego +++ b/bundle/regal/rules/bugs/not-equals-in-loop/not_equals_in_loop.rego @@ -18,8 +18,7 @@ report contains violation if { neq_term.type == "ref" some value in neq_term.value - value.type == "var" - startswith(value.value, "$") + ast.is_wildcard(value) violation := result.fail(rego.metadata.chain(), result.location(expr.terms[0])) } diff --git a/bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego b/bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego index a25f628c..a96345e0 100644 --- a/bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego +++ b/bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego @@ -30,7 +30,7 @@ report contains violation if { some var in var_refs not ast.var_in_head(input.rules[to_number(rule_index)].head, var.value) - not _var_in_call(ast.function_calls, rule_index, var.value) + not ast.var_in_call(ast.function_calls, rule_index, var.value) not _ref_base_vars[rule_index][var.value] # this is by far the most expensive condition to check, so only do @@ -44,7 +44,7 @@ _ref_vars[rule_index][var.value] contains var if { some rule_index var := ast.found.vars[rule_index].ref[_] - not startswith(var.value, "$") + not ast.is_wildcard(var) } # "a" in "a[foo]", and not foo @@ -52,21 +52,5 @@ _ref_base_vars[rule_index][term.value] contains term if { some rule_index term := ast.found.refs[rule_index][_].value[0] - term.type == "var" - not startswith(term.value, "$") -} - -_var_in_call(calls, rule_index, name) if _var_in_arg(calls[rule_index][_].args[_], name) - -_var_in_arg(arg, name) if { - arg.type == "var" - arg.value == name -} - -_var_in_arg(arg, name) if { - arg.type in {"array", "object", "set"} - - some var in ast.find_term_vars(arg) - - var.value == name + not ast.is_wildcard(term) } diff --git a/bundle/regal/rules/custom/naming-convention/naming_convention_test.rego b/bundle/regal/rules/custom/naming-convention/naming_convention_test.rego index d1c357c8..aa84635d 100644 --- a/bundle/regal/rules/custom/naming-convention/naming_convention_test.rego +++ b/bundle/regal/rules/custom/naming-convention/naming_convention_test.rego @@ -3,6 +3,7 @@ package regal.rules.custom["naming-convention_test"] import rego.v1 import data.regal.ast +import data.regal.capabilities import data.regal.config import data.regal.rules.custom["naming-convention"] as rule @@ -162,7 +163,9 @@ test_fail_multiple_conventions if { {"targets": ["package"], "pattern": `^acmecorp\.[a-z_\.]+$`}, {"targets": ["rule", "variable"], "pattern": "^bar$|^foo_bar$"}, ]} - r := rule.report with input as policy with config.for_rule as cfg + r := rule.report with input as policy + with config.for_rule as cfg + with data.internal.combined_config as {"capabilities": capabilities.provided} r == { expected( diff --git a/bundle/regal/rules/idiomatic/custom-has-key-construct/custom_has_key_construct.rego b/bundle/regal/rules/idiomatic/custom-has-key-construct/custom_has_key_construct.rego index 05d5bd56..00915f44 100644 --- a/bundle/regal/rules/idiomatic/custom-has-key-construct/custom_has_key_construct.rego +++ b/bundle/regal/rules/idiomatic/custom-has-key-construct/custom_has_key_construct.rego @@ -39,13 +39,11 @@ report contains violation if { # normalize var to always always be on the left hand side _normalize_eq_terms(terms) := [terms[1], terms[2]] if { - terms[1].type == "var" - startswith(terms[1].value, "$") + ast.is_wildcard(terms[1]) terms[2].type == "ref" } _normalize_eq_terms(terms) := [terms[2], terms[1]] if { terms[1].type == "ref" - terms[2].type == "var" - startswith(terms[2].value, "$") + ast.is_wildcard(terms[2]) } diff --git a/bundle/regal/rules/idiomatic/custom-in-construct/custom_in_construct.rego b/bundle/regal/rules/idiomatic/custom-in-construct/custom_in_construct.rego index 2ccb4f5a..cc11f9da 100644 --- a/bundle/regal/rules/idiomatic/custom-in-construct/custom_in_construct.rego +++ b/bundle/regal/rules/idiomatic/custom-in-construct/custom_in_construct.rego @@ -25,8 +25,7 @@ report contains violation if { var.value in arg_names ref.value[0].value in arg_names - ref.value[1].type == "var" - startswith(ref.value[1].value, "$") + ast.is_wildcard(ref.value[1]) violation := result.fail(rego.metadata.chain(), result.location(rule.head)) } diff --git a/bundle/regal/rules/idiomatic/equals-pattern-matching/equals_pattern_matching.rego b/bundle/regal/rules/idiomatic/equals-pattern-matching/equals_pattern_matching.rego index 5166dda0..7617bf7b 100644 --- a/bundle/regal/rules/idiomatic/equals-pattern-matching/equals_pattern_matching.rego +++ b/bundle/regal/rules/idiomatic/equals-pattern-matching/equals_pattern_matching.rego @@ -71,13 +71,11 @@ report contains violation if { # normalize var to always always be on the left hand side _normalize_eq_terms(terms, scalar_types) := [terms[1], terms[2]] if { - terms[1].type == "var" - not startswith(terms[1].value, "$") + not ast.is_wildcard(terms[1]) terms[2].type in scalar_types } _normalize_eq_terms(terms, scalar_types) := [terms[2], terms[1]] if { terms[1].type in scalar_types - terms[2].type == "var" - not startswith(terms[2].value, "$") + not ast.is_wildcard(terms[2]) } diff --git a/bundle/regal/rules/idiomatic/prefer-set-or-object-rule/prefer_set_or_object_rule.rego b/bundle/regal/rules/idiomatic/prefer-set-or-object-rule/prefer_set_or_object_rule.rego index 828a2f6d..23177559 100644 --- a/bundle/regal/rules/idiomatic/prefer-set-or-object-rule/prefer_set_or_object_rule.rego +++ b/bundle/regal/rules/idiomatic/prefer-set-or-object-rule/prefer_set_or_object_rule.rego @@ -68,6 +68,5 @@ _is_array_conversion(value) if { some ref_val in rhs.value - ref_val.type == "var" - startswith(ref_val.value, "$") + ast.is_wildcard(ref_val) } diff --git a/bundle/regal/rules/idiomatic/use-in-operator/use_in_operator.rego b/bundle/regal/rules/idiomatic/use-in-operator/use_in_operator.rego index ba0f6463..9b440e81 100644 --- a/bundle/regal/rules/idiomatic/use-in-operator/use_in_operator.rego +++ b/bundle/regal/rules/idiomatic/use-in-operator/use_in_operator.rego @@ -37,9 +37,8 @@ _non_loop_term(terms) := [{"pos": i + 1, "term": term} | _loop_term(term) if { term.type == "ref" term.value[0].type == "var" - last := regal.last(term.value) - last.type == "var" - startswith(last.value, "$") + + ast.is_wildcard(regal.last(term.value)) } _static_term(term) if term.type in {"array", "boolean", "object", "null", "number", "set", "string", "var"} diff --git a/bundle/regal/rules/idiomatic/use-some-for-output-vars/use_some_for_output_vars.rego b/bundle/regal/rules/idiomatic/use-some-for-output-vars/use_some_for_output_vars.rego index 614c4695..cab86a44 100644 --- a/bundle/regal/rules/idiomatic/use-some-for-output-vars/use_some_for_output_vars.rego +++ b/bundle/regal/rules/idiomatic/use-some-for-output-vars/use_some_for_output_vars.rego @@ -15,7 +15,7 @@ report contains violation if { # first item can't be a loop or key ref i != 0 elem.type == "var" - not startswith(elem.value, "$") + not ast.is_wildcard(elem) rule := input.rules[to_number(rule_index)] diff --git a/bundle/regal/rules/performance/walk-no-path/walk_no_path.rego b/bundle/regal/rules/performance/walk-no-path/walk_no_path.rego new file mode 100644 index 00000000..80fb22a1 --- /dev/null +++ b/bundle/regal/rules/performance/walk-no-path/walk_no_path.rego @@ -0,0 +1,61 @@ +# METADATA +# description: Call to `walk` can be optimized +package regal.rules.performance["walk-no-path"] + +import rego.v1 + +import data.regal.ast +import data.regal.result + +report contains violation if { + some rule_index + call := ast.function_calls[rule_index][_] + + call.name == "walk" + call.args[1].type == "array" + call.args[1].value[0].type == "var" + + path_var := call.args[1].value[0] + + not ast.is_wildcard(path_var) + not ast.var_in_head(input.rules[to_number(rule_index)].head, path_var.value) + + not _var_in_other_call(ast.function_calls, rule_index, path_var) + not _var_in_ref(rule_index, path_var) + + violation := result.fail(rego.metadata.chain(), result.location(call)) +} + +# similar to ast.var_in_call, but here we need to discern that +# the var is not the same var declared in the walk itself +_var_in_other_call(calls, rule_index, var) if _var_in_arg(calls[rule_index][_].args[_], var) + +_var_in_arg(arg, var) if { + arg.type == "var" + arg.value == var.value + + arg.location != var.location +} + +_var_in_arg(arg, var) if { + arg.type in {"array", "object", "set"} + + some term_var in ast.find_term_vars(arg) + + term_var.value == var.value + term_var.location != var.location +} + +_var_in_ref(rule_index, var) if { + some ref_var in ast.found.vars[rule_index].ref + + var.value == ref_var.value +} + +_var_in_ref(rule_index, var) if { + term := ast.found.refs[rule_index][_].value[0] # regal ignore:external-reference + + not ast.is_wildcard(term) + + var.value == term.value +} diff --git a/bundle/regal/rules/performance/walk-no-path/walk_no_path_test.rego b/bundle/regal/rules/performance/walk-no-path/walk_no_path_test.rego new file mode 100644 index 00000000..f8c89ac9 --- /dev/null +++ b/bundle/regal/rules/performance/walk-no-path/walk_no_path_test.rego @@ -0,0 +1,101 @@ +package regal.rules.performance["walk-no-path_test"] + +import rego.v1 + +import data.regal.ast +import data.regal.config + +import data.regal.rules.performance["walk-no-path"] as rule + +test_fail_walk_can_have_path_omitted if { + module := ast.with_rego_v1(` + allow if { + walk(input, [path, value]) + value == 1 + } + `) + r := rule.report with input as module + + r == {with_location({ + "col": 3, + "end": { + "col": 7, + "row": 7, + }, + "file": "policy.rego", + "row": 7, + "text": "\t\twalk(input, [path, value])", + })} +} + +test_success_path_is_wildcard if { + module := ast.with_rego_v1(` + allow if { + walk(input, [_, value]) + value == 1 + } + `) + r := rule.report with input as module + + r == set() +} + +test_success_path_in_head if { + module := ast.with_rego_v1(` + rule[path] := "foo" if { + walk(input, [path, value]) + value == 1 + } + `) + r := rule.report with input as module + + r == set() +} + +test_success_path_in_call if { + module := ast.with_rego_v1(` + allow if { + walk(input, [path, value]) + path == 1 + } + `) + r := rule.report with input as module + + r == set() +} + +test_success_path_in_ref if { + module := ast.with_rego_v1(` + allow if { + walk(input, [path, value]) + input[path] == 1 + } + `) + r := rule.report with input as module + + r == set() +} + +test_success_path_in_ref_in_ref if { + module := ast.with_rego_v1(` + allow if { + walk(input, [path, value]) + input[path[0]] == 1 + } + `) + r := rule.report with input as module + + r == set() +} + +with_location(location) := { + "category": "performance", + "description": "Call to `walk` can be optimized", + "level": "error", + "location": location, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/walk-no-path", "performance"), + }], + "title": "walk-no-path", +} diff --git a/bundle/regal/rules/style/external-reference/external_reference.rego b/bundle/regal/rules/style/external-reference/external_reference.rego index b149ea3f..b3285820 100644 --- a/bundle/regal/rules/style/external-reference/external_reference.rego +++ b/bundle/regal/rules/style/external-reference/external_reference.rego @@ -27,7 +27,7 @@ report contains violation if { value.type == "var" not value.value in allowed_refs - not startswith(value.value, "$") + not ast.is_wildcard(value) not _function_call_ctx(fn, path) violation := result.fail(rego.metadata.chain(), result.location(value)) diff --git a/docs/rules/performance/walk-no-path.md b/docs/rules/performance/walk-no-path.md new file mode 100644 index 00000000..0f96c438 --- /dev/null +++ b/docs/rules/performance/walk-no-path.md @@ -0,0 +1,101 @@ +# walk-no-path + +**Summary**: Call to `walk` can be optimized + +**Category**: Performance + +**Avoid** +```rego +package policy + +import rego.v1 + +allow if { + # traverse potentially nested permissions structure looking + # for an admin role, but notice how the path is never referenced + # later + walk(user.permissions, [path, value]) + + value.type == "role" + value.name == "admin" +} +``` + +**Prefer** +```rego +package policy + +import rego.v1 + +allow if { + # replacing `path` with a wildcard variable tells the evaluator that it won't + # have to build the path array for each node `walk` traverses, thereby avoiding + # unnecessary allocations + walk(user.permissions, [_, value]) + + value.type == "role" + value.name == "admin" +} +``` + +## Rationale + +The primary purpose of the `walk` function is to traverse nested data structures, and often at an arbitrary depth. +Each node traversed "produces" a path/value pair, where the path is an array of keys that lead to the current node, +and the value is the current node itself. Most often, rules only need to account for the value of the node and not the +path, and when that is the case, using a wildcard variable (`_`) in place of path tells the evaluation engine that +there's no need to build the path array for each node traversed, thereby avoiding unnecessary allocations. This can +have a big impact on performance when huge data structures are traversed! + +More concretely, `walk`ing without generating the path array cuts down evaluation time by about 33%, and reduces the +number of allocations by about 40%. + +**Trivia**: this optimization was originally made in OPA to improve the performance of Regal, where `walk` is used +extensively to traverse the AST of the policy being linted. + +## Exceptions + +This rule can only optimize `walk` calls where the path/value array is provided as a second argument to `walk`, and +**not** when assigned using `:=`: + +```rego +package policy + +import rego.v1 + +allow if { + # this can't be optimized, as the `walk` function can't + # "see" the array assignment on the left hand side + [path, value] := walk(user.permissions) + + value.type == "role" + value.name == "admin" +} +``` + +For this reason, and a few historic ones, using the second argument for the return value is the preferred way to use +`walk`, which is [unique](https://docs.styra.com/regal/rules/style/function-arg-return#exceptions) for the walk built-in +function. + +## Configuration Options + +This linter rule provides the following configuration options: + +```yaml +rules: + performance: + walk-no-path: + # one of "error", "warning", "ignore" + level: error +``` + +## Related Resources + +- Regal Docs: [function-arg-return](https://docs.styra.com/regal/rules/style/function-arg-return) +- GitHub: [Source Code](https://github.com/StyraInc/regal/blob/main/bundle/regal/rules/performance/walk-no-path/walk_no_path.rego) + +## 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)! diff --git a/e2e/testdata/violations/most_violations.rego b/e2e/testdata/violations/most_violations.rego index 87155a42..7f217ba0 100644 --- a/e2e/testdata/violations/most_violations.rego +++ b/e2e/testdata/violations/most_violations.rego @@ -287,3 +287,8 @@ defer_assignment if { input.foo == "bar" x == 1 } + +walk_no_path if { + walk(input, [path, value]) + value == "violation" +} diff --git a/internal/update/update.rego b/internal/update/update.rego index 5fd8e6b0..fecb4669 100644 --- a/internal/update/update.rego +++ b/internal/update/update.rego @@ -11,8 +11,7 @@ default needs_update := false # scope: document needs_update if { current_version := trim(input.current_version, "v") - latest_version := trim(input.latest_version, "v") semver.is_valid(current_version) - semver.compare(current_version, latest_version) == -1 + semver.compare(current_version, trim(input.latest_version, "v")) == -1 }