StyraInc / regal

Regal is a linter and language server for Rego, bringing your policy development experience to the next level!
https://docs.styra.com/regal
Apache License 2.0
261 stars 35 forks source link

Rule to not use `=` #720

Closed colezlaw closed 5 months ago

colezlaw commented 5 months ago

As a user, I would like to be warned when I use the = operator in a policy and be reminded that := and == are explicit and clear, whereas = can be ambiguous so that my policies are clearer and more maintainable.

I realize there's already a rule not to use = for assignment, but it would be nice to extend that rule when = is used for equality, or make a separate rule.

Because Rego rules are immutable, the first time and identifier is seen, the = operator is used for assignment. Once the left hand side has been assigned to, when referred to in the future, = is the equality operator. This simple example from the REPL shows what's going on:

> import rego.v1
> foo = 3
Rule 'foo' defined in package repl. Type 'show' to see rules.
> foo = 4
undefined
> foo = 3
true

I'd like a rule to tell me to use := for assignment and == for equality checks and to not use = because its meaning can be ambiguous.

anderseknert commented 5 months ago

Thanks for filing this issue! Detecting "unnecessary" use of = was one of the first issues created in this project... it's just that trying to determine when it's necessary or not is a bit of a... challenge 😅

All the operators are functions internally, so if you want to forbid it entirely, you can do that today via the custom forbidden-function-call rule, adding eq (which maps to the = operator) to the list of forbidden functions:

.regal/config.yaml

rules:
  custom:
    forbidden-function-call:
      level: error
      forbidden-functions:
        - eq

policy.rego

package policy

import rego.v1

allow if {
    input.x = true
}
regal lint policy.rego
Rule:           forbidden-function-call
Description:    Forbidden function call
Category:       custom
Location:       policy.rego:6:10
Text:           input.x = true
Documentation:  https://docs.styra.com/regal/rules/custom/forbidden-function-call

1 file linted. 1 violation found.

I suppose one improvement to consider here would be to allow custom error messages per forbidden function, as it might not be clear to the end user (if different from the "admin" who configured Regal) reading the error that = is the fordbidden function call, or a function call at all. But at least it's enforceable.

colezlaw commented 5 months ago

Yup - I should've closed the issue as soon as I found use-assignment-operator - I figured if that rule exists then something that checks if it's being used for equality exists as well.