diff --git a/.regal/config.yaml b/.regal/config.yaml index 45bb4081..406999be 100644 --- a/.regal/config.yaml +++ b/.regal/config.yaml @@ -17,6 +17,7 @@ rules: level: error exclude-args: - cfg + - metadata - rule style: line-length: diff --git a/bundle/regal/ast/ast.rego b/bundle/regal/ast/ast.rego index 75abd0b7..da0b1aea 100644 --- a/bundle/regal/ast/ast.rego +++ b/bundle/regal/ast/ast.rego @@ -203,7 +203,7 @@ rule_index_strings := [sprintf("%d", [i]) | some i, _ in _rules] # keyed by rule index function_calls[rule_index] contains call if { some rule_index in rule_index_strings - some ref in found.refs[rule_index] + some ref in found.calls[rule_index] name := ref_to_string(ref[0].value) args := [arg | @@ -344,21 +344,6 @@ all_functions := object.union(config.capabilities.builtins, function_decls(input # scope of the input AST all_function_names := object.keys(all_functions) -# METADATA -# description: set containing all negated expressions in input AST -negated_expressions[rule_index] contains value if { - some i, rule in _rules - - # converting to string until https://github.com/open-policy-agent/opa/issues/6736 is fixed - rule_index := rule_index_strings[i] - - some node in ["head", "body", "else"] - - walk(rule[node], [_, value]) - - value.negated == true -} - # METADATA # description: | # true if rule head contains no identifier, but is a chained rule body immediately following the previous one: @@ -398,15 +383,15 @@ var_in_call(calls, rule_index, name) if has_named_var(calls[rule_index][_].args[ # METADATA # description: answers wether provided expression is an assignment (using `:=`) -is_assignment(expr) if { - expr.terms[0].type == "ref" - expr.terms[0].value[0].type == "var" - expr.terms[0].value[0].value == "assign" +is_assignment(term) if { + term.type == "ref" + term.value[0].type == "var" + term.value[0].value == "assign" } # METADATA # description: returns the terms in an assignment (`:=`) expression, or undefined if not assignment -assignment_terms(expr) := [expr.terms[1], expr.terms[2]] if is_assignment(expr) +assignment_terms(terms) := [terms[1], terms[2]] if is_assignment(terms[0]) # METADATA # description: | diff --git a/bundle/regal/ast/ast_test.rego b/bundle/regal/ast/ast_test.rego index 974a5f9c..ba71636e 100644 --- a/bundle/regal/ast/ast_test.rego +++ b/bundle/regal/ast/ast_test.rego @@ -4,89 +4,6 @@ import data.regal.ast import data.regal.capabilities import data.regal.config -test_find_vars if { - policy := ` - package p - - global := "foo" - - allow if { - a := global - b := [c | c := input[d]] - - every e in input { - e == "foo" - } - - every f, g in input.bar { - f == g - } - - some h, i - input.bar[h][i] - some j in input - some k, l in input - - [m, n, o] := [1, 2, 3] - - [p, [q, _]] := [1, [2, 1]] - - some _, [r, s] in [["foo", "bar"], [1, 2]] - - {"x": t} := {"x": 1} - - some [u] in [[1]] - } - ` - - vars := ast.find_vars(regal.parse_module("p.rego", policy).rules) with config.capabilities as capabilities.provided - names := {var.value | - some var in vars - var.type == "var" - } - - names == {"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u"} -} - -test_find_vars_comprehension_lhs if { - policy := ` - package p - - allow if { - a := [b | input[b]] - c := {d | input[d]} - e := {f: g | g := input[f]} - } - ` - - vars := ast.find_vars(regal.parse_module("p.rego", policy).rules) with config.capabilities as capabilities.provided - names := {var.value | - some var in vars - var.type == "var" - } - - names == {"a", "b", "c", "d", "e", "f", "g"} -} - -test_find_vars_function_ret_return_args if { - policy := ` - package p - - allow if { - walk(input, [path, value]) - } - ` - - module := regal.parse_module("p.rego", policy) - vars := ast.find_vars(module.rules) with config.capabilities as capabilities.provided with input.rules as [] - names := {var.value | - some var in vars - var.type == "var" - } - - names == {"path", "value"} -} - # https://github.com/StyraInc/regal/issues/168 test_function_decls_multiple_same_name if { policy := `package p @@ -132,62 +49,6 @@ allow := true ] } -test_find_vars_in_local_scope if { - policy := ` - package p - - global := "foo" - - allow if { - a := global - b := [c | c := input[d]] - - every e in input { - f == "foo" - g := "bar" - h == "foo" - } - }` - - module := regal.parse_module("p.rego", policy) - - allow_rule := module.rules[1] - - var_locations := { - "a": {"col": 3, "row": 9}, - "b": {"col": 3, "row": 10}, - "c": {"col": 13, "row": 10}, - "d": {"col": 9, "row": 12}, - "e": {"col": 4, "row": 14}, - } - - var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.a)) with input as module == set() - var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.b)) with input as module == {"a"} - var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.c)) with input as module == {"a", "b", "c"} - var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.d)) with input as module == {"a", "b", "c", "d"} - var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.e)) with input as module == {"a", "b", "c", "d", "e"} -} - -test_find_vars_in_local_scope_complex_comprehension_term if { - policy := ` - package p - - allow if { - a := [{"b": b} | c := input[b]] - }` - - module := regal.parse_module("p.rego", policy) - - allow_rule := module.rules[0] - - ast.find_vars_in_local_scope(allow_rule, {"col": 10, "row": 10}) with input as module == [ - {"location": {"col": 3, "row": 7, "text": "YQ=="}, "type": "var", "value": "a"}, - {"location": {"col": 15, "row": 7, "text": "Yg=="}, "type": "var", "value": "b"}, - {"location": {"col": 20, "row": 7, "text": "Yw=="}, "type": "var", "value": "c"}, - {"location": {"col": 31, "row": 7, "text": "Yg=="}, "type": "var", "value": "b"}, - ] -} - test_find_names_in_scope if { policy := ` package p @@ -218,23 +79,6 @@ test_find_names_in_scope if { in_scope == {"bar", "global", "comp", "allow", "a", "b", "c", "d", "e"} } -test_find_some_decl_names_in_scope if { - policy := `package p - - allow if { - foo := 1 - some x - input[x] - some y, z - input[y][z] == x - }` - - module := regal.parse_module("p.rego", policy) - - {"x"} == ast.find_some_decl_names_in_scope(module.rules[0], {"col": 1, "row": 6}) with input as module - {"x", "y", "z"} == ast.find_some_decl_names_in_scope(module.rules[0], {"col": 1, "row": 8}) with input as module -} - var_names(vars) := {var.value | some var in vars} test_provided_capabilities_never_undefined if capabilities.provided == {} with data.internal as {} diff --git a/bundle/regal/ast/search.rego b/bundle/regal/ast/search.rego index fcc773ca..a37965a0 100644 --- a/bundle/regal/ast/search.rego +++ b/bundle/regal/ast/search.rego @@ -10,19 +10,13 @@ _find_nested_vars(obj) := [value | # simple assignment, i.e. `x := 100` returns `x` # always returns a single var, but wrapped in an -# array for consistency -_find_assign_vars(value) := var if { - value[1].type == "var" - var := [value[1]] -} - +# array for consistency or # 'destructuring' array assignment, i.e. -# [a, b, c] := [1, 2, 3] -# or -# {a: b} := {"foo": "bar"} -_find_assign_vars(value) := vars if { - value[1].type in {"array", "object"} - vars := _find_nested_vars(value[1]) +# [a, b, c] := [1, 2, 3] or {a: b} := {"foo": "bar"} +_find_assign_vars(value) := [value] if { + value.type == "var" +} else := _find_nested_vars(value) if { + value.type in {"array", "object"} } # var declared via `some`, i.e. `some x` or `some x, y` @@ -32,16 +26,10 @@ _find_some_decl_vars(value) := [v | ] # single var declared via `some in`, i.e. `some x in y` -_find_some_in_decl_vars(value) := vars if { - arr := value[0].value - count(arr) == 3 - - vars := _find_nested_vars(arr[1]) -} +_find_some_in_decl_vars(arr) := _find_nested_vars(arr[1]) if count(arr) == 3 # two vars declared via `some in`, i.e. `some x, y in z` -_find_some_in_decl_vars(value) := vars if { - arr := value[0].value +_find_some_in_decl_vars(arr) := vars if { count(arr) == 4 vars := [v | @@ -128,14 +116,14 @@ _find_vars(value, last) := {"term": find_term_vars(function_ret_args(fn_name, va # left-hand side is equally dubious, but we'll treat `x = 1` as `x := 1` for # the purpose of this function until we have a more robust way of dealing with # unification -_find_vars(value, last) := {"assign": _find_assign_vars(value)} if { +_find_vars(value, last) := {"assign": _find_assign_vars(value[1])} if { last == "terms" value[0].type == "ref" value[0].value[0].type == "var" value[0].value[0].value in {"assign", "eq"} } -_find_vars(value, last) := {"somein": _find_some_in_decl_vars(value)} if { +_find_vars(value, last) := {"somein": _find_some_in_decl_vars(value[0].value)} if { last == "symbols" value[0].type == "call" } @@ -208,10 +196,13 @@ _rules := data.workspace.parsed[input.regal.file.uri].rules if not input.rules # - somein # - ref found.vars[rule_index][context] contains var if { - some i, rule_index in rule_index_strings + some i, rule in _rules + + rule_index := rule_index_strings[i] + some node in ["head", "body", "else"] - walk(_rules[i][node], [path, value]) + walk(rule[node], [path, value]) last := regal.last(path) last in {"terms", "symbols", "args"} @@ -233,7 +224,6 @@ found.vars[rule_index].ref contains var if { # METADATA # description: all refs found in module -# scope: document found.refs[rule_index] contains value if { some i, rule in _rules @@ -245,7 +235,9 @@ found.refs[rule_index] contains value if { value.type == "ref" } -found.refs[rule_index] contains value if { +# METADATA +# description: all calls found in module +found.calls[rule_index] contains value if { some i, rule in _rules # converting to string until https://github.com/open-policy-agent/opa/issues/6736 is fixed @@ -280,6 +272,21 @@ found.comprehensions[rule_index] contains value if { value.type in {"arraycomprehension", "objectcomprehension", "setcomprehension"} } +# METADATA +# description: set containing all negated expressions in input AST +found.expressions[rule_index] contains value if { + some i, rule in _rules + + # converting to string until https://github.com/open-policy-agent/opa/issues/6736 is fixed + rule_index := rule_index_strings[i] + + some node in ["head", "body", "else"] + + walk(rule[node], [_, value]) + + value.terms +} + # METADATA # description: | # finds all vars declared in `rule` *before* the `location` provided @@ -363,10 +370,3 @@ find_some_decl_names_in_scope(rule, location) := {some_var.value | some some_var in found.vars[_rule_index(rule)]["some"] _before_location(rule.head, some_var, location) } - -# METADATA -# description: all expressions in module -exprs[rule_index][expr_index] := expr if { - some rule_index, rule in input.rules - some expr_index, expr in rule.body -} diff --git a/bundle/regal/ast/search_test.rego b/bundle/regal/ast/search_test.rego index f64a65ad..4e8a306e 100644 --- a/bundle/regal/ast/search_test.rego +++ b/bundle/regal/ast/search_test.rego @@ -1,28 +1,161 @@ package regal.ast_test import data.regal.ast +import data.regal.capabilities +import data.regal.config -test_exprs if { - inp := regal.parse_module("foo.rego", `package example +test_find_vars if { + policy := ` + package p -import rego.v1 + global := "foo" -allow if input.x == 1 + allow if { + a := global + b := [c | c := input[d]] -allow if { - input.y == 2 - input.z == 3 + every e in input { + e == "foo" + } + + every f, g in input.bar { + f == g + } + + some h, i + input.bar[h][i] + some j in input + some k, l in input + + [m, n, o] := [1, 2, 3] + + [p, [q, _]] := [1, [2, 1]] + + some _, [r, s] in [["foo", "bar"], [1, 2]] + + {"x": t} := {"x": 1} + + some [u] in [[1]] + } + ` + + vars := ast.find_vars(regal.parse_module("p.rego", policy).rules) with config.capabilities as capabilities.provided + names := {var.value | + some var in vars + var.type == "var" + } + + names == {"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u"} +} + +test_find_vars_comprehension_lhs if { + policy := ` + package p + + allow if { + a := [b | input[b]] + c := {d | input[d]} + e := {f: g | g := input[f]} + } + ` + + vars := ast.find_vars(regal.parse_module("p.rego", policy).rules) with config.capabilities as capabilities.provided + names := {var.value | + some var in vars + var.type == "var" + } + + names == {"a", "b", "c", "d", "e", "f", "g"} +} + +test_find_vars_function_ret_return_args if { + policy := ` + package p + + allow if { + walk(input, [path, value]) + } + ` + + module := regal.parse_module("p.rego", policy) + vars := ast.find_vars(module.rules) with config.capabilities as capabilities.provided with input.rules as [] + names := {var.value | + some var in vars + var.type == "var" + } + + names == {"path", "value"} } -`) - result := ast.exprs with input as inp +test_find_some_decl_names_in_scope if { + policy := `package p + + allow if { + foo := 1 + some x + input[x] + some y, z + input[y][z] == x + }` + + module := regal.parse_module("p.rego", policy) + + {"x"} == ast.find_some_decl_names_in_scope(module.rules[0], {"col": 1, "row": 6}) with input as module + {"x", "y", "z"} == ast.find_some_decl_names_in_scope(module.rules[0], {"col": 1, "row": 8}) with input as module +} + +test_find_vars_in_local_scope if { + policy := ` + package p + + global := "foo" + + allow if { + a := global + b := [c | c := input[d]] + + every e in input { + f == "foo" + g := "bar" + h == "foo" + } + }` + + module := regal.parse_module("p.rego", policy) + + allow_rule := module.rules[1] + + var_locations := { + "a": {"col": 3, "row": 9}, + "b": {"col": 3, "row": 10}, + "c": {"col": 13, "row": 10}, + "d": {"col": 9, "row": 12}, + "e": {"col": 4, "row": 14}, + } + + var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.a)) with input as module == set() + var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.b)) with input as module == {"a"} + var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.c)) with input as module == {"a", "b", "c"} + var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.d)) with input as module == {"a", "b", "c", "d"} + var_names(ast.find_vars_in_local_scope(allow_rule, var_locations.e)) with input as module == {"a", "b", "c", "d", "e"} +} + +test_find_vars_in_local_scope_complex_comprehension_term if { + policy := ` + package p + + allow if { + a := [{"b": b} | c := input[b]] + }` - count(result) == 2 # rules + module := regal.parse_module("p.rego", policy) - count(result[0]) == 1 - result[0][0].terms[0].value[0].value == "equal" + allow_rule := module.rules[0] - count(result[1]) == 2 - result[1][0].terms[0].value[0].value == "equal" - result[1][1].terms[0].value[0].value == "equal" + ast.find_vars_in_local_scope(allow_rule, {"col": 10, "row": 10}) with input as module == [ + {"location": {"col": 3, "row": 7, "text": "YQ=="}, "type": "var", "value": "a"}, + {"location": {"col": 15, "row": 7, "text": "Yg=="}, "type": "var", "value": "b"}, + {"location": {"col": 20, "row": 7, "text": "Yw=="}, "type": "var", "value": "c"}, + {"location": {"col": 31, "row": 7, "text": "Yg=="}, "type": "var", "value": "b"}, + ] } diff --git a/bundle/regal/lsp/completion/ref_names.rego b/bundle/regal/lsp/completion/ref_names.rego index d0754698..bdbbd548 100644 --- a/bundle/regal/lsp/completion/ref_names.rego +++ b/bundle/regal/lsp/completion/ref_names.rego @@ -7,6 +7,12 @@ import data.regal.ast # returns a list of ref names that are used in the module # built-in functions are not included as they are provided by another completions provider # scope: document +ref_names contains name if { + name := ast.ref_static_to_string(ast.found.calls[_][_].value) + + not name in ast.builtin_names +} + ref_names contains name if { name := ast.ref_static_to_string(ast.found.refs[_][_].value) diff --git a/bundle/regal/rules/bugs/constant-condition/constant_condition.rego b/bundle/regal/rules/bugs/constant-condition/constant_condition.rego index 42bba7ba..5e667f28 100644 --- a/bundle/regal/rules/bugs/constant-condition/constant_condition.rego +++ b/bundle/regal/rules/bugs/constant-condition/constant_condition.rego @@ -5,20 +5,13 @@ package regal.rules.bugs["constant-condition"] import data.regal.ast import data.regal.result -# NOTE: The constant condition checks currently don't do nesting! -# Additionally, there are several other conditions that could be considered -# constant, or if not, redundant... so this rule should be expanded in time - # METADATA # description: single scalar value, like a lone `true` inside a rule body # scope: rule report contains violation if { - terms := input.rules[_].body[_].terms + terms := ast.found.expressions[_][_].terms - # We could probably include arrays and objects too, as a single compound value - # is not very useful, but it's not as clear cut as scalars, as you could have - # something like {"a": foo(input.x) == "bar"} which is not a constant condition, - # however meaningless it may be. Maybe consider for another rule? + # We could include composite types too, but less comomon and more expensive to check terms.type in ast.scalar_types violation := result.fail(rego.metadata.chain(), result.location(terms)) @@ -30,7 +23,7 @@ report contains violation if { report contains violation if { operators := {"equal", "gt", "gte", "lt", "lte", "neq"} - expr := input.rules[_].body[_] + expr := ast.found.expressions[_][_] expr.terms[0].value[0].type == "var" expr.terms[0].value[0].value in operators diff --git a/bundle/regal/rules/bugs/constant-condition/constant_condition_test.rego b/bundle/regal/rules/bugs/constant-condition/constant_condition_test.rego index f09e8a1c..52c4ba94 100644 --- a/bundle/regal/rules/bugs/constant-condition/constant_condition_test.rego +++ b/bundle/regal/rules/bugs/constant-condition/constant_condition_test.rego @@ -12,7 +12,45 @@ test_fail_simple_constant_condition if { r == {{ "category": "bugs", "description": "Constant condition", - "location": {"col": 2, "file": "policy.rego", "row": 4, "text": "\t1", "end": {"row": 4, "col": 3}}, + "location": { + "col": 2, + "file": "policy.rego", + "row": 4, + "text": "\t1", + "end": { + "row": 4, + "col": 3, + }, + }, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/constant-condition", "bugs"), + }], + "title": "constant-condition", + "level": "error", + }} +} + +test_fail_simple_constant_condition_nested if { + r := rule.report with input as ast.policy(`allow if { + every x in [1, 2] { + 1 + x == 2 + } + }`) + + r == {{ + "category": "bugs", + "description": "Constant condition", + "location": { + "col": 4, + "end": { + "col": 5, + "row": 5, + }, + "file": "policy.rego", + "row": 5, "text": "\t\t\t1", + }, "related_resources": [{ "description": "documentation", "ref": config.docs.resolve_url("$baseUrl/$category/constant-condition", "bugs"), @@ -55,7 +93,45 @@ test_fail_operator_constant_condition if { r == {{ "category": "bugs", "description": "Constant condition", - "location": {"col": 2, "file": "policy.rego", "row": 4, "text": "\t1 == 1", "end": {"col": 8, "row": 4}}, + "location": { + "col": 2, + "file": "policy.rego", + "row": 4, + "text": "\t1 == 1", + "end": { + "col": 8, + "row": 4, + }, + }, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/constant-condition", "bugs"), + }], + "title": "constant-condition", + "level": "error", + }} +} + +test_fail_operator_constant_condition_nested if { + r := rule.report with input as ast.policy(`nested := [1 | + c := [2 | + 1 == 1 + ] + ]`) + + r == {{ + "category": "bugs", + "description": "Constant condition", + "location": { + "col": 4, + "end": { + "col": 10, + "row": 5, + }, + "file": "policy.rego", + "row": 5, + "text": "\t\t\t1 == 1", + }, "related_resources": [{ "description": "documentation", "ref": config.docs.resolve_url("$baseUrl/$category/constant-condition", "bugs"), diff --git a/bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego b/bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego index 415b0e42..0461f47e 100644 --- a/bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego +++ b/bundle/regal/rules/bugs/deprecated-builtin/deprecated_builtin.rego @@ -29,8 +29,7 @@ report contains violation if { # bail out early if no the deprecated built-ins are in capabilities util.intersects(object.keys(config.capabilities.builtins), deprecated_builtins) - ref := ast.found.refs[_][_] - call := ref[0] + call := ast.found.calls[_][_][0] ast.ref_to_string(call.value) in deprecated_builtins diff --git a/bundle/regal/rules/bugs/if-object-literal/if_object_literal_test.rego b/bundle/regal/rules/bugs/if-object-literal/if_object_literal_test.rego index 57aeb663..012f2881 100644 --- a/bundle/regal/rules/bugs/if-object-literal/if_object_literal_test.rego +++ b/bundle/regal/rules/bugs/if-object-literal/if_object_literal_test.rego @@ -9,11 +9,27 @@ test_fail[name] if { some name, [policy, location] in { "empty_object": [ `rule if {}`, - {"col": 9, "row": 5, "text": "rule if {}", "end": {"col": 11, "row": 5}}, + { + "col": 9, + "row": 5, + "text": "rule if {}", + "end": { + "col": 11, + "row": 5, + }, + }, ], "non_empty_object": [ `rule if {"x": input.x}`, - {"col": 9, "row": 5, "text": `rule if {"x": input.x}`, "end": {"col": 23, "row": 5}}, + { + "col": 9, + "row": 5, + "text": `rule if {"x": input.x}`, + "end": { + "col": 23, + "row": 5, + }, + }, ], } diff --git a/bundle/regal/rules/bugs/impossible-not/impossible_not.rego b/bundle/regal/rules/bugs/impossible-not/impossible_not.rego index f8e9ef26..0eb6a31f 100644 --- a/bundle/regal/rules/bugs/impossible-not/impossible_not.rego +++ b/bundle/regal/rules/bugs/impossible-not/impossible_not.rego @@ -25,7 +25,7 @@ _multivalue_rules contains path if { _negated_refs contains negated_ref if { some rule_index, value - ast.negated_expressions[rule_index][value] + ast.found.expressions[rule_index][value].negated # if terms is an array, it's a function call, and most likely not "impossible" is_object(value.terms) diff --git a/bundle/regal/rules/bugs/impossible-not/impossible_not_test.rego b/bundle/regal/rules/bugs/impossible-not/impossible_not_test.rego index 04620420..8ceb78df 100644 --- a/bundle/regal/rules/bugs/impossible-not/impossible_not_test.rego +++ b/bundle/regal/rules/bugs/impossible-not/impossible_not_test.rego @@ -34,6 +34,31 @@ test_fail_multivalue_not_reference_same_package if { }) } +test_fail_multivalue_not_reference_same_package_nested_expression if { + agg1 := rule.aggregate with input as regal.parse_module("p1.rego", `package foo + + partial contains "foo"`) + + agg2 := rule.aggregate with input as regal.parse_module("p2.rego", `package foo + + test_foo if { + comprehension := [1 | not partial] + }`) + + r := rule.aggregate_report with input as {"aggregate": (agg1 | agg2)} + + r == expected_with_location({ + "col": 29, + "end": { + "col": 36, + "row": 4, + }, + "file": "p2.rego", + "row": 4, + "text": "not partial", + }) +} + test_fail_multivalue_not_reference_different_package_using_direct_reference if { agg1 := rule.aggregate with input as regal.parse_module("p1.rego", `package foo 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 26ab665e..9dea6385 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 @@ -6,7 +6,10 @@ import data.regal.ast import data.regal.result report contains violation if { - terms := ast.exprs[_][_].terms + some rule_index, i + ast.found.expressions[rule_index][i].terms[0].type == "ref" + + terms := ast.found.expressions[rule_index][i].terms terms[0].type == "ref" terms[0].value[0].type == "var" diff --git a/bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego b/bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego index 61a09532..52c31a03 100644 --- a/bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego +++ b/bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego @@ -9,7 +9,7 @@ import data.regal.result # description: check rule bodies for redundant existence checks report contains violation if { some rule_index, rule in input.rules - some expr_index, expr in ast.exprs[rule_index] + some expr_index, expr in _exprs[rule_index] expr.terms.type == "ref" @@ -52,10 +52,16 @@ report contains violation if { rule.head.value.type == "ref" - some expr in ast.exprs[rule_index] + some expr in _exprs[rule_index] expr.terms.type == "ref" ast.ref_value_equal(expr.terms.value, rule.head.value.value) violation := result.fail(rego.metadata.chain(), result.ranged_from_ref(expr.terms.value)) } + +# all top-level expressions in module +_exprs[rule_index][expr_index] := expr if { + some rule_index, rule in input.rules + some expr_index, expr in rule.body +} diff --git a/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch.rego b/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch.rego index 0f50baec..08f513de 100644 --- a/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch.rego +++ b/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch.rego @@ -63,11 +63,11 @@ _first_arg_value(rule_index, term) := found if { trow := util.to_location_object(term.location).row found := [rhs | - some expr in ast.exprs[to_number(rule_index)] + some expr in ast.found.expressions[rule_index] util.to_location_object(expr.location).row < trow - [lhs, rhs] := ast.assignment_terms(expr) + [lhs, rhs] := ast.assignment_terms(expr.terms) lhs.type == "var" lhs.value == term.value rhs.type == "string" diff --git a/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch_test.rego b/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch_test.rego index b064693d..3a5db38f 100644 --- a/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch_test.rego +++ b/bundle/regal/rules/bugs/sprintf-arguments-mismatch/sprintf_arguments_mismatch_test.rego @@ -14,7 +14,10 @@ test_fail_too_many_values_in_array if { "location": { "row": 5, "col": 14, - "end": {"col": 26, "row": 5}, + "end": { + "col": 26, + "row": 5, + }, "file": "policy.rego", "text": "x := sprintf(\"%s\", [1, 2])", }, @@ -26,6 +29,33 @@ test_fail_too_many_values_in_array if { }} } +test_fail_too_many_values_in_array_nested if { + r := rule.report with input as ast.with_rego_v1(`x := [1 | + y := [s | s := sprintf("%s", [1, 2])] + ]`) + + r == {{ + "category": "bugs", + "description": "Mismatch in `sprintf` arguments count", + "level": "error", + "location": { + "col": 26, + "end": { + "col": 38, + "row": 6, + }, + "file": "policy.rego", + "row": 6, + "text": "\t\ty := [s | s := sprintf(\"%s\", [1, 2])]", + }, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/sprintf-arguments-mismatch", "bugs"), + }], + "title": "sprintf-arguments-mismatch", + }} +} + test_fail_too_few_values_in_array if { r := rule.report with input as ast.with_rego_v1(`x := sprintf("%s%v", [1])`) r == {{ @@ -35,7 +65,10 @@ test_fail_too_few_values_in_array if { "location": { "row": 5, "col": 14, - "end": {"col": 25, "row": 5}, + "end": { + "col": 25, + "row": 5, + }, "file": "policy.rego", "text": `x := sprintf("%s%v", [1])`, }, @@ -87,7 +120,10 @@ test_fail_first_arg_is_variable_with_nonmatching_pattern if { "level": "error", "location": { "col": 11, - "end": {"col": 21, "row": 7}, + "end": { + "col": 21, + "row": 7, + }, "file": "policy.rego", "row": 7, "text": "\t\tsprintf(s, [\"foo\"])", diff --git a/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value.rego b/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value.rego index 493f31bf..682e42e6 100644 --- a/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value.rego +++ b/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value.rego @@ -7,7 +7,7 @@ import data.regal.config import data.regal.result report contains violation if { - terms := ast.exprs[_][_].terms + terms := ast.found.expressions[_][_].terms terms[0].type == "ref" terms[0].value[0].type == "var" diff --git a/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value_test.rego b/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value_test.rego index 2d35055f..c3ec2873 100644 --- a/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value_test.rego +++ b/bundle/regal/rules/bugs/unassigned-return-value/unassigned_return_value_test.rego @@ -33,6 +33,37 @@ test_fail_unused_return_value if { }} } +test_fail_unused_return_value_nested if { + r := rule.report with input as ast.with_rego_v1(`allow if { + comprehension := [x | + indexof("s", "s") + x := 1 + ] + }`) + with config.capabilities as capabilities.provided + + r == {{ + "category": "bugs", + "description": "Non-boolean return value unassigned", + "level": "error", + "location": { + "col": 4, + "end": { + "col": 11, + "row": 7, + }, + "file": "policy.rego", + "row": 7, + "text": "\t\t\tindexof(\"s\", \"s\")", + }, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/unassigned-return-value", "bugs"), + }], + "title": "unassigned-return-value", + }} +} + test_success_unused_boolean_return_value if { r := rule.report with input as ast.policy(`allow if { startswith("s", "s") }`) with config.capabilities as capabilities.provided 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 9a35b7da..5d1a7f24 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 @@ -21,12 +21,13 @@ import data.regal.result # ``` report contains violation if { some rule_index - var_refs := _ref_vars[rule_index][_] + var_refs := ast.found.vars[rule_index].ref count(var_refs) == 1 some var in var_refs + not ast.is_wildcard(var) not ast.var_in_head(input.rules[to_number(rule_index)].head, var.value) not ast.var_in_call(ast.function_calls, rule_index, var.value) not _ref_base_vars[rule_index][var.value] @@ -39,13 +40,6 @@ report contains violation if { violation := result.fail(rego.metadata.chain(), result.location(var)) } -_ref_vars[rule_index][var.value] contains var if { - some rule_index - var := ast.found.vars[rule_index].ref[_] - - not ast.is_wildcard(var) -} - # "a" in "a[foo]", and not foo _ref_base_vars[rule_index][term.value] contains term if { some rule_index diff --git a/bundle/regal/rules/custom/forbidden-function-call/forbidden_function_call.rego b/bundle/regal/rules/custom/forbidden-function-call/forbidden_function_call.rego index bd89642a..3a6786b6 100644 --- a/bundle/regal/rules/custom/forbidden-function-call/forbidden_function_call.rego +++ b/bundle/regal/rules/custom/forbidden-function-call/forbidden_function_call.rego @@ -13,7 +13,7 @@ report contains violation if { # avoid traversal if no forbidden function is called util.intersects(forbidden, ast.builtin_functions_called) - ref := ast.found.refs[_][_] + ref := ast.found.calls[_][_] name := ast.ref_to_string(ref[0].value) name in forbidden diff --git a/bundle/regal/rules/custom/narrow-argument/narrow_argument.rego b/bundle/regal/rules/custom/narrow-argument/narrow_argument.rego index 4d643864..fd48ddf9 100644 --- a/bundle/regal/rules/custom/narrow-argument/narrow_argument.rego +++ b/bundle/regal/rules/custom/narrow-argument/narrow_argument.rego @@ -17,7 +17,6 @@ report contains violation if { not _arg_used_in_call(indices, arg) location := result.location(_first_named_arg_location(indices, arg)) - violation := result.fail(rego.metadata.chain(), object.union( location, {"description": _message(count(refs), arg, narrowed)}, @@ -42,8 +41,9 @@ _narrow(refs) := narrowed if { arr := util.any_set_item(refs) count(arr) > 1 + not _nested(arr) - narrowed := concat(".", arr) + narrowed := ast.ref_to_string(_to_terms(arr)) } _narrow(refs) := narrowed if { @@ -52,8 +52,9 @@ _narrow(refs) := narrowed if { prefix := util.longest_prefix(refs) count(prefix) > 1 + not _nested(prefix) - narrowed := concat(".", prefix) + narrowed := ast.ref_to_string(_to_terms(prefix)) } _first_named_arg_location(indices, name) := [arg.location | @@ -126,6 +127,14 @@ _first_var_pos(ref) := pos if { ][0] } else := count(ref) + 1 -_exclude_arg(cfg, name) if { - name in cfg["exclude-args"] +_exclude_arg(cfg, name) if name in cfg["exclude-args"] + +_to_terms(arr) := [_to_term(item) | some item in arr] + +_to_term(value) := {"type": "number", "value": value} if is_number(value) +_to_term(value) := {"type": "string", "value": value} if is_string(value) + +_nested(arr) if { + some item in arr + not type_name(item) in ast.scalar_types } diff --git a/bundle/regal/rules/custom/narrow-argument/narrow_argument_test.rego b/bundle/regal/rules/custom/narrow-argument/narrow_argument_test.rego index 31deb4be..53d8d8a1 100644 --- a/bundle/regal/rules/custom/narrow-argument/narrow_argument_test.rego +++ b/bundle/regal/rules/custom/narrow-argument/narrow_argument_test.rego @@ -63,11 +63,48 @@ test_fail_can_be_narrowed_prefixed_ref if { }} } +test_fail_can_be_narrowed_prefixed_array_ref if { + r := rule.report with input as ast.policy(` + fun(arr) if arr[0].y.number == 1 + fun(arr) if arr[0].y.string == "1" + `) + with config.rules as {"custom": {"narrow-argument": {"level": "error"}}} + + r == {{ + "category": "custom", + "description": "Argument arr always referenced by a common prefix, value passed can be narrowed to arr[0].y", + "level": "error", + "location": { + "col": 7, + "end": { + "col": 10, + "row": 4, + }, + "file": "policy.rego", + "row": 4, + "text": "\t\tfun(arr) if arr[0].y.number == 1", + }, + "related_resources": [{ + "description": "documentation", + "ref": config.docs.resolve_url("$baseUrl/$category/narrow-argument", "custom"), + }], + "title": "narrow-argument", + }} +} + test_success_can_not_be_narrowed_arg_is_least_common_denominator if { r := rule.report with input as ast.policy(` fun(obj) if obj.typ == "string" fun(obj) if obj.val == "string" `) + with config.rules as {"custom": {"narrow-argument": {"level": "error"}}} + + r == set() +} + +test_success_nested_or_variable_path_not_narrowed if { + r := rule.report with input as ast.policy(`foo(lines) := lines[bar - 1]`) + with config.rules as {"custom": {"narrow-argument": {"level": "error"}}} r == set() } diff --git a/bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego b/bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego index 25ab0e59..dce8fa8a 100644 --- a/bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego +++ b/bundle/regal/rules/idiomatic/non-raw-regex-pattern/non_raw_regex_pattern.rego @@ -10,7 +10,7 @@ report contains violation if { # skip traversing refs if no builtin regex function calls are registered util.intersects(_re_pattern_function_names, ast.builtin_functions_called) - value := ast.found.refs[_][_] + value := ast.found.calls[_][_] value[0].value[0].type == "var" value[0].value[0].value == "regex" 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 a41ada28..ed41b225 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 @@ -54,7 +54,7 @@ _is_array_conversion(value) if { count(body) == 1 - [lhs, rhs] := ast.assignment_terms(body[0]) + [lhs, rhs] := ast.assignment_terms(body[0].terms) # Assignment to comprehension variable lhs.type == "var" diff --git a/bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego b/bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego index 50d38523..3026ac83 100644 --- a/bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego +++ b/bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego @@ -15,7 +15,7 @@ notices contains result.notice(rego.metadata.chain()) if not capabilities.has_st # METADATA # description: flag calls to `count` where the first argument is a call to `indexof_n` report contains violation if { - ref := ast.found.refs[_][_] + ref := ast.found.calls[_][_] ref[0].value[0].type == "var" ref[0].value[0].value == "count" diff --git a/bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego b/bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego index 4109093f..bb436d7c 100644 --- a/bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego +++ b/bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego @@ -19,7 +19,7 @@ aggregate contains entry if { # Special case for custom rules, where we don't want to flag e.g. `import data.regal.ast` # as unknown, even though it's not a package included in evaluation. - not _custom_regal_package_and_import(ast.package_path, path) + not _custom_regal_package_and_import(ast.package_path, path[0]) imp := object.union(result.location(_import), {"path": path}) ] @@ -30,10 +30,9 @@ aggregate contains entry if { }) } -_custom_regal_package_and_import(pkg_path, path) if { +_custom_regal_package_and_import(pkg_path, "regal") if { pkg_path[0] == "custom" pkg_path[1] == "regal" - path[0] == "regal" } # METADATA diff --git a/bundle/regal/rules/imports/unresolved-import/unresolved_import.rego b/bundle/regal/rules/imports/unresolved-import/unresolved_import.rego index dc74dba8..57c8c2f9 100644 --- a/bundle/regal/rules/imports/unresolved-import/unresolved_import.rego +++ b/bundle/regal/rules/imports/unresolved-import/unresolved_import.rego @@ -20,7 +20,7 @@ aggregate contains entry if { # Special case for custom rules, where we don't want to flag e.g. `import data.regal.ast` # as unknown, even though it's not a package included in evaluation. - not _custom_regal_package_and_import(ast.package_path, path) + not _custom_regal_package_and_import(ast.package_path, path[0]) imp := object.union(result.location(_import), {"path": path}) ] @@ -63,25 +63,16 @@ aggregate_report contains violation if { violation := result.fail(rego.metadata.chain(), result.location(imp)) } -_custom_regal_package_and_import(pkg_path, path) if { +_custom_regal_package_and_import(pkg_path, "regal") if { pkg_path[0] == "custom" pkg_path[1] == "regal" - path[0] == "regal" } # the package part will always be included exported refs # but if we have a rule like foo.bar.baz # we'll want to include both foo.bar and foo.bar.baz _to_paths(pkg_path, ref) := util.all_paths(_to_path(pkg_path, ref)) if count(ref) < 3 - -_to_paths(pkg_path, ref) := paths if { - count(ref) > 2 - - paths := [path | - some p in util.all_paths(ref) - path := _to_path(pkg_path, p) - ] -} +_to_paths(pkg_path, ref) := [_to_path(pkg_path, p) | some p in util.all_paths(ref)] if count(ref) > 2 _to_path(pkg_path, ref) := array.concat(pkg_path, [str | some i, part in ref diff --git a/bundle/regal/rules/performance/defer-assignment/defer_assignment.rego b/bundle/regal/rules/performance/defer-assignment/defer_assignment.rego index 3822258f..666555aa 100644 --- a/bundle/regal/rules/performance/defer-assignment/defer_assignment.rego +++ b/bundle/regal/rules/performance/defer-assignment/defer_assignment.rego @@ -9,7 +9,7 @@ report contains violation if { some i, rule in input.rules some j, expr in rule.body - [var, rhs] := ast.assignment_terms(expr) + [var, rhs] := ast.assignment_terms(expr.terms) not _ref_with_vars(rhs) @@ -19,7 +19,7 @@ report contains violation if { next := rule.body[j + 1] - not ast.is_assignment(next) + not ast.is_assignment(next.terms[0]) not ast.var_in_head(rule.head, var.value) not _var_value_used_in_expression(var.value, next) not _iteration_expression(next.terms) @@ -91,6 +91,7 @@ _iteration_expression(terms) if { terms[0].value[0].value == "walk" } +# regal ignore:narrow-argument _print_call(terms) if { terms[0].value[0].type == "var" terms[0].value[0].value == "print" diff --git a/bundle/regal/rules/performance/with-outside-test-context/with_outside_test_context.rego b/bundle/regal/rules/performance/with-outside-test-context/with_outside_test_context.rego index d9792224..ce9262c4 100644 --- a/bundle/regal/rules/performance/with-outside-test-context/with_outside_test_context.rego +++ b/bundle/regal/rules/performance/with-outside-test-context/with_outside_test_context.rego @@ -6,8 +6,8 @@ import data.regal.ast import data.regal.result report contains violation if { - some rule_index, rule in input.rules - some expr_index, expr in ast.exprs[rule_index] + some i, rule in input.rules + some expr in ast.found.expressions[ast.rule_index_strings[i]] expr["with"] not strings.any_prefix_match(ast.ref_to_string(rule.head.ref), {"test_", "todo_test"}) diff --git a/bundle/regal/rules/style/comprehension-term-assignment/comprehension_term_assignment.rego b/bundle/regal/rules/style/comprehension-term-assignment/comprehension_term_assignment.rego index 12949556..2a5fc0f2 100644 --- a/bundle/regal/rules/style/comprehension-term-assignment/comprehension_term_assignment.rego +++ b/bundle/regal/rules/style/comprehension-term-assignment/comprehension_term_assignment.rego @@ -25,7 +25,7 @@ report contains violation if { some expr in comp.body - [lhs, rhs] := ast.assignment_terms(expr) + [lhs, rhs] := ast.assignment_terms(expr.terms) lhs.type == comp.term.type lhs.value == comp.term.value @@ -55,7 +55,7 @@ report contains violation if { some expr in comp.body - [lhs, rhs] := ast.assignment_terms(expr) + [lhs, rhs] := ast.assignment_terms(expr.terms) some kind in ["key", "value"] diff --git a/bundle/regal/rules/style/double-negative/double_negative.rego b/bundle/regal/rules/style/double-negative/double_negative.rego index 99d8f793..cd723706 100644 --- a/bundle/regal/rules/style/double-negative/double_negative.rego +++ b/bundle/regal/rules/style/double-negative/double_negative.rego @@ -11,8 +11,9 @@ import data.regal.ast import data.regal.result report contains violation if { - some node - ast.negated_expressions[_][node].terms.type == "var" + some node, i + ast.found.expressions[i][node].negated + ast.found.expressions[i][node].terms.type == "var" strings.any_prefix_match(node.terms.value, { "cannot_", diff --git a/bundle/regal/rules/style/pointless-reassignment/pointless_reassignment.rego b/bundle/regal/rules/style/pointless-reassignment/pointless_reassignment.rego index b8853932..b15fc83d 100644 --- a/bundle/regal/rules/style/pointless-reassignment/pointless_reassignment.rego +++ b/bundle/regal/rules/style/pointless-reassignment/pointless_reassignment.rego @@ -23,7 +23,7 @@ report contains violation if { not expr["with"] - [lhs, rhs] := ast.assignment_terms(expr) + [lhs, rhs] := ast.assignment_terms(expr.terms) lhs.type == "var" rhs.type == "var" diff --git a/bundle/regal/rules/style/rule-length/rule_length.rego b/bundle/regal/rules/style/rule-length/rule_length.rego index 493e4c76..d0dd876d 100644 --- a/bundle/regal/rules/style/rule-length/rule_length.rego +++ b/bundle/regal/rules/style/rule-length/rule_length.rego @@ -14,7 +14,7 @@ report contains violation if { rule_location := util.to_location_object(rule.location) lines := split(rule_location.text, "\n") - _line_count(cfg, rule_location.row, lines) > cfg[_max_length_property(rule.head)] + _line_count(cfg, rule_location.row, lines) > cfg[_max_length_property(rule.head.ref[0].value)] not _no_body_exception(cfg, rule) @@ -28,7 +28,7 @@ _no_body_exception(cfg, rule) if { default _max_length_property(_) := "max-rule-length" -_max_length_property(head) := "max-test-rule-length" if startswith(head.ref[0].value, "test_") +_max_length_property(value) := "max-test-rule-length" if startswith(value, "test_") _line_count(cfg, _, lines) := count(lines) if cfg["count-comments"] == true diff --git a/bundle/regal/rules/style/unconditional-assignment/unconditional_assignment.rego b/bundle/regal/rules/style/unconditional-assignment/unconditional_assignment.rego index 13170f58..65f1d9ec 100644 --- a/bundle/regal/rules/style/unconditional-assignment/unconditional_assignment.rego +++ b/bundle/regal/rules/style/unconditional-assignment/unconditional_assignment.rego @@ -47,17 +47,20 @@ report contains violation if { # Multi-value rule rule.head.key.type == "var" - not rule.body[0]["with"] + expr := rule.body[0] - _assignment_expr(rule.body[0].terms) + not expr["with"] - rule.body[0].terms[1].type == "var" - rule.body[0].terms[1].value == rule.head.key.value + _assignment_expr(expr.terms) - violation := result.fail(rego.metadata.chain(), result.infix_expr_location(rule.body[0].terms)) + expr.terms[1].type == "var" + expr.terms[1].value == rule.head.key.value + + violation := result.fail(rego.metadata.chain(), result.infix_expr_location(expr.terms)) } # Assignment using either = or := +# regal ignore:narrow-argument _assignment_expr(terms) if { terms[0].type == "ref" terms[0].value[0].type == "var" diff --git a/bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego b/bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego index 0b22ff27..c95fbd25 100644 --- a/bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego +++ b/bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego @@ -14,24 +14,20 @@ report contains violation if { symbols[0].type == "call" symbols[0].value[0].type == "ref" - _some_is_unnecessary(symbols, ast.scalar_types) + _some_is_unnecessary(symbols[0].value, ast.scalar_types) violation := result.fail(rego.metadata.chain(), result.location(symbols)) } -_some_is_unnecessary(value, scalar_types) if { - ref := value[0].value[0].value - - [ref[0].value, ref[1].value] == ["internal", "member_2"] - - value[0].value[1].type in scalar_types +_some_is_unnecessary(symbol, scalar_types) if { + symbol[0].value[0].value == "internal" + symbol[0].value[1].value == "member_2" + symbol[1].type in scalar_types } -_some_is_unnecessary(value, scalar_types) if { - ref := value[0].value[0].value - - [ref[0].value, ref[1].value] == ["internal", "member_3"] - - value[0].value[1].type in scalar_types - value[0].value[2].type in scalar_types +_some_is_unnecessary(symbol, scalar_types) if { + symbol[0].value[0].value == "internal" + symbol[0].value[1].value == "member_3" + symbol[1].type in scalar_types + symbol[2].type in scalar_types } diff --git a/bundle/regal/rules/style/yoda-condition/yoda_condition.rego b/bundle/regal/rules/style/yoda-condition/yoda_condition.rego index 0e4f8cad..b87de5fc 100644 --- a/bundle/regal/rules/style/yoda-condition/yoda_condition.rego +++ b/bundle/regal/rules/style/yoda-condition/yoda_condition.rego @@ -6,7 +6,7 @@ import data.regal.ast import data.regal.result report contains violation if { - value := ast.found.refs[_][_] + value := ast.found.calls[_][_] value[0].value[0].type == "var" value[0].value[0].value in {"equal", "neq", "gt", "lt", "gte", "lte"} diff --git a/bundle/regal/rules/testing/dubious-print-sprintf/dubious_print_sprintf.rego b/bundle/regal/rules/testing/dubious-print-sprintf/dubious_print_sprintf.rego index 421c39f9..4b23e026 100644 --- a/bundle/regal/rules/testing/dubious-print-sprintf/dubious_print_sprintf.rego +++ b/bundle/regal/rules/testing/dubious-print-sprintf/dubious_print_sprintf.rego @@ -9,7 +9,7 @@ report contains violation if { # skip traversing refs if no print calls are registered "print" in ast.builtin_functions_called - value := ast.found.refs[_][_] + value := ast.found.calls[_][_] value[0].value[0].type == "var" value[0].value[0].value == "print" diff --git a/bundle/regal/rules/testing/print-or-trace-call/print_or_trace_call.rego b/bundle/regal/rules/testing/print-or-trace-call/print_or_trace_call.rego index b36b558a..2ea01dbc 100644 --- a/bundle/regal/rules/testing/print-or-trace-call/print_or_trace_call.rego +++ b/bundle/regal/rules/testing/print-or-trace-call/print_or_trace_call.rego @@ -10,7 +10,7 @@ report contains violation if { # skip iteration of refs if no print or trace calls are registered util.intersects(ast.builtin_functions_called, {"print", "trace"}) - ref := ast.found.refs[_][_][0] + ref := ast.found.calls[_][_][0] ref.value[0].type == "var" ref.value[0].value in {"print", "trace"} diff --git a/internal/embeds/schemas/regal-ast.json b/internal/embeds/schemas/regal-ast.json index 98b52e85..aabe858a 100644 --- a/internal/embeds/schemas/regal-ast.json +++ b/internal/embeds/schemas/regal-ast.json @@ -232,9 +232,6 @@ }, "head": { "properties": { - "name": { - "type": "string" - }, "ref": { "$ref": "#/$defs/ref" }, diff --git a/internal/lsp/config/watcher.go b/internal/lsp/config/watcher.go index 47fec7cb..3ed8485d 100644 --- a/internal/lsp/config/watcher.go +++ b/internal/lsp/config/watcher.go @@ -64,6 +64,33 @@ func (w *Watcher) Start(ctx context.Context) error { return nil } +func (w *Watcher) Watch(configFilePath string) { + w.pathUpdates <- configFilePath +} + +func (w *Watcher) Stop() error { + if w.fsWatcher != nil { + if err := w.fsWatcher.Close(); err != nil { + return fmt.Errorf("failed to close fsnotify watcher: %w", err) + } + + return nil + } + + return nil +} + +func (w *Watcher) IsWatching() bool { + w.fsWatcherLock.Lock() + defer w.fsWatcherLock.Unlock() + + if w.fsWatcher == nil { + return false + } + + return len(w.fsWatcher.WatchList()) > 0 +} + func (w *Watcher) loop(ctx context.Context) { for { select { @@ -109,30 +136,3 @@ func (w *Watcher) loop(ctx context.Context) { } } } - -func (w *Watcher) Watch(configFilePath string) { - w.pathUpdates <- configFilePath -} - -func (w *Watcher) Stop() error { - if w.fsWatcher != nil { - if err := w.fsWatcher.Close(); err != nil { - return fmt.Errorf("failed to close fsnotify watcher: %w", err) - } - - return nil - } - - return nil -} - -func (w *Watcher) IsWatching() bool { - w.fsWatcherLock.Lock() - defer w.fsWatcherLock.Unlock() - - if w.fsWatcher == nil { - return false - } - - return len(w.fsWatcher.WatchList()) > 0 -} diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 605f72f6..889fa3ed 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -102,32 +102,6 @@ type LanguageServerOptions struct { WorkspaceDiagnosticsPoll time.Duration } -func NewLanguageServer(ctx context.Context, opts *LanguageServerOptions) *LanguageServer { - c := cache.NewCache() - store := NewRegalStore() - - ls := &LanguageServer{ - cache: c, - regoStore: store, - logWriter: opts.LogWriter, - logLevel: opts.LogLevel, - lintFileJobs: make(chan lintFileJob, 10), - lintWorkspaceJobs: make(chan lintWorkspaceJob, 10), - builtinsPositionJobs: make(chan lintFileJob, 10), - commandRequest: make(chan types.ExecuteCommandParams, 10), - templateFileJobs: make(chan lintFileJob, 10), - completionsManager: completions.NewDefaultManager(ctx, c, store), - webServer: web.NewServer(c, opts.LogWriter, opts.LogLevel), - loadedBuiltins: concurrent.MapOf(make(map[string]map[string]*ast.Builtin)), - workspaceDiagnosticsPoll: opts.WorkspaceDiagnosticsPoll, - loadedConfigAllRegoVersions: concurrent.MapOf(make(map[string]ast.RegoVersion)), - } - - ls.configWatcher = lsconfig.NewWatcher(&lsconfig.WatcherOpts{LogFunc: ls.logf}) - - return ls -} - type LanguageServer struct { logWriter io.Writer logLevel log.Level @@ -182,6 +156,32 @@ type lintWorkspaceJob struct { AggregateReportOnly bool } +func NewLanguageServer(ctx context.Context, opts *LanguageServerOptions) *LanguageServer { + c := cache.NewCache() + store := NewRegalStore() + + ls := &LanguageServer{ + cache: c, + regoStore: store, + logWriter: opts.LogWriter, + logLevel: opts.LogLevel, + lintFileJobs: make(chan lintFileJob, 10), + lintWorkspaceJobs: make(chan lintWorkspaceJob, 10), + builtinsPositionJobs: make(chan lintFileJob, 10), + commandRequest: make(chan types.ExecuteCommandParams, 10), + templateFileJobs: make(chan lintFileJob, 10), + completionsManager: completions.NewDefaultManager(ctx, c, store), + webServer: web.NewServer(c, opts.LogWriter, opts.LogLevel), + loadedBuiltins: concurrent.MapOf(make(map[string]map[string]*ast.Builtin)), + workspaceDiagnosticsPoll: opts.WorkspaceDiagnosticsPoll, + loadedConfigAllRegoVersions: concurrent.MapOf(make(map[string]ast.RegoVersion)), + } + + ls.configWatcher = lsconfig.NewWatcher(&lsconfig.WatcherOpts{LogFunc: ls.logf}) + + return ls +} + //nolint:wrapcheck func (l *LanguageServer) Handle(ctx context.Context, _ *jsonrpc2.Conn, req *jsonrpc2.Request) (any, error) { l.logf(log.LevelDebug, "received request: %s", req.Method) @@ -479,59 +479,6 @@ func (l *LanguageServer) StartHoverWorker(ctx context.Context) { } } -func (l *LanguageServer) getLoadedConfig() *config.Config { - l.loadedConfigLock.RLock() - defer l.loadedConfigLock.RUnlock() - - return l.loadedConfig -} - -func (l *LanguageServer) getEnabledNonAggregateRules() []string { - l.loadedConfigLock.RLock() - defer l.loadedConfigLock.RUnlock() - - return l.loadedConfigEnabledNonAggregateRules -} - -func (l *LanguageServer) getEnabledAggregateRules() []string { - l.loadedConfigLock.RLock() - defer l.loadedConfigLock.RUnlock() - - return l.loadedConfigEnabledAggregateRules -} - -// loadEnabledRulesFromConfig is used to cache the enabled rules for the current -// config. These take some time to compute and only change when config changes, -// so we can store them on the server to speed up diagnostic runs. -func (l *LanguageServer) loadEnabledRulesFromConfig(ctx context.Context, cfg config.Config) error { - lint := linter.NewLinter().WithUserConfig(cfg) - - enabledRules, err := lint.DetermineEnabledRules(ctx) - if err != nil { - return fmt.Errorf("failed to determine enabled rules: %w", err) - } - - enabledAggregateRules, err := lint.DetermineEnabledAggregateRules(ctx) - if err != nil { - return fmt.Errorf("failed to determine enabled aggregate rules: %w", err) - } - - l.loadedConfigLock.Lock() - defer l.loadedConfigLock.Unlock() - - l.loadedConfigEnabledNonAggregateRules = []string{} - - for _, r := range enabledRules { - if !slices.Contains(enabledAggregateRules, r) { - l.loadedConfigEnabledNonAggregateRules = append(l.loadedConfigEnabledNonAggregateRules, r) - } - } - - l.loadedConfigEnabledAggregateRules = enabledAggregateRules - - return nil -} - func (l *LanguageServer) StartConfigWorker(ctx context.Context) { if err := l.configWatcher.Start(ctx); err != nil { l.logf(log.LevelMessage, "failed to start config watcher: %s", err) @@ -1075,6 +1022,59 @@ func (l *LanguageServer) StartWebServer(ctx context.Context) { l.webServer.Start(ctx) } +func (l *LanguageServer) getLoadedConfig() *config.Config { + l.loadedConfigLock.RLock() + defer l.loadedConfigLock.RUnlock() + + return l.loadedConfig +} + +func (l *LanguageServer) getEnabledNonAggregateRules() []string { + l.loadedConfigLock.RLock() + defer l.loadedConfigLock.RUnlock() + + return l.loadedConfigEnabledNonAggregateRules +} + +func (l *LanguageServer) getEnabledAggregateRules() []string { + l.loadedConfigLock.RLock() + defer l.loadedConfigLock.RUnlock() + + return l.loadedConfigEnabledAggregateRules +} + +// loadEnabledRulesFromConfig is used to cache the enabled rules for the current +// config. These take some time to compute and only change when config changes, +// so we can store them on the server to speed up diagnostic runs. +func (l *LanguageServer) loadEnabledRulesFromConfig(ctx context.Context, cfg config.Config) error { + lint := linter.NewLinter().WithUserConfig(cfg) + + enabledRules, err := lint.DetermineEnabledRules(ctx) + if err != nil { + return fmt.Errorf("failed to determine enabled rules: %w", err) + } + + enabledAggregateRules, err := lint.DetermineEnabledAggregateRules(ctx) + if err != nil { + return fmt.Errorf("failed to determine enabled aggregate rules: %w", err) + } + + l.loadedConfigLock.Lock() + defer l.loadedConfigLock.Unlock() + + l.loadedConfigEnabledNonAggregateRules = []string{} + + for _, r := range enabledRules { + if !slices.Contains(enabledAggregateRules, r) { + l.loadedConfigEnabledNonAggregateRules = append(l.loadedConfigEnabledNonAggregateRules, r) + } + } + + l.loadedConfigEnabledAggregateRules = enabledAggregateRules + + return nil +} + func (l *LanguageServer) templateContentsForFile(fileURI string) (string, error) { // this function should not be called with files in the root, but if it is, // then it is an error to prevent unwanted behavior. diff --git a/pkg/fixer/fixer.go b/pkg/fixer/fixer.go index efdbf008..68219eb1 100644 --- a/pkg/fixer/fixer.go +++ b/pkg/fixer/fixer.go @@ -24,6 +24,14 @@ const ( OnConflictRename OnConflictOperation = "rename" ) +// Fixer must be instantiated via NewFixer. +type Fixer struct { + registeredFixes map[string]any + onConflictOperation OnConflictOperation + registeredRoots []string + versionsMap map[string]ast.RegoVersion +} + // NewFixer instantiates a Fixer. func NewFixer() *Fixer { return &Fixer{ @@ -33,14 +41,6 @@ func NewFixer() *Fixer { } } -// Fixer must be instantiated via NewFixer. -type Fixer struct { - registeredFixes map[string]any - onConflictOperation OnConflictOperation - registeredRoots []string - versionsMap map[string]ast.RegoVersion -} - // SetOnConflictOperation sets the fixer's behavior when a conflict occurs. func (f *Fixer) SetOnConflictOperation(operation OnConflictOperation) { f.onConflictOperation = operation