Skip to content

Latest commit

 

History

History
98 lines (69 loc) · 3.61 KB

rego-style-guide.md

File metadata and controls

98 lines (69 loc) · 3.61 KB
Raw

Rego Style Guide rules in Regal

Some notes, and a checklist for tracking, on implementing linter rules based on the Rego Style Guide.

Rules with a strikethrough are considered to be either impossible, or undesirable, for Regal to check, and should be targeted by other means.

General Advice

  • Optimize for readability, not performance
  • Use opa fmt
  • Use strict mode
  • Use metadata annotations
  • Get to know the built-in functions
  • Consider using JSON schemas for type checking

Notes

Both opa fmt and strict mode checks can be implemented by tapping into the corresponding features of OPA, and simply report errors as linter violations. Why not just use OPA for that? Mainly because we want a single command (and config) for anything related to code quality in Rego. Rules can be disabled here in favor of OPA if someone prefers that. Do note that we won't actually format anything, just as we won't do any other type of remediation — the rule should only check if the files are formatted, similar to opa fmt --diff --fail.

Requiring the use of metadata annotations is doable, but a rule like that would certainly have to be configurable to be usable. Same goes for JSON schemas.

Style

  • Prefer snake_case for rule names and variables
  • Keep line length <= 120 characters

Rules

  • Use helper rules and functions
  • Use negation to handle undefined
  • Consider partial helper rules over comprehensions in rule bodies
  • Avoid prefixing rules and functions with get_ or list_
  • Prefer unconditional assignment in rule head over rule body

Notes

Three quite complex rules in the top here. While it's not going to be very scientific, we could try to determine whether helper rules are used to a satisfying degree by checking the dependencies of rules vs the number of expressions, and come up with some (configurable) thresholds. "Use negation to handle undefined" could be very hard to implement, and might be that we shouldn't. "Consider" partial rules over comprehensions feels impossible to determine whether the author has done, and both have valid use cases.

Variables and Data Types

  • Use in to check for membership
  • Prefer some .. in for iteration
  • Use every to express FOR ALL
  • Don't use unification operator for assignment or comparison
  • Don't use undeclared variables
  • Prefer sets over arrays (where applicable)

Notes

Almost all of these should be doable, with some possibly being quite challenging. One that could be very hard to implement is the every rule, as that would require us to determine what other method was used and that every is a suitable replacement. Except for "Use in to check for membership", none of the above rules can be enforced using the AST alone.

Functions

  • Prefer using arguments over input and data
  • Avoid using the last argument for the return value

Regex

  • Use raw strings for regex patterns

Notes

Can only be done by scanning the original code, as this is lost in the AST.

Imports

  • Use explicit imports for future keywords
  • Prefer importing packages over rules and functions
  • Avoid importing input

Notes

Checking for package imports requires a view of all modules. We may assume that anything not found there are base documents to be provided at runtime.

Community

If you'd like to discuss the Rego Style Guide for Regal, or just talk about Regal in general, please join us in the #regal channel in the Styra Community Slack!