Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor rule metadata logic #174

Merged
merged 1 commit into from
Jun 26, 2023
Merged

Refactor rule metadata logic #174

merged 1 commit into from
Jun 26, 2023

Conversation

anderseknert
Copy link
Member

Some additional refactoring made possible now that each rule resides in its own file. Extend this further to have each rule in its own package, and move the metadata from the report rule in each package and to the package declaration. We may then use rego.metadata.chain() instead, which includes the path of the package containing both the category and the title, thus allowing us to skip having to redeclare that in annotations.

This has a number of benefits:

  • Tests are now proper unit tests, and no longer test the whole category.
  • Checking if a rule is ignored can now be the responsibility of the main rule, and linter rules need no longer be concerned with that unless they have unique configuration options.
  • Reduce the amount of clutter in each rule related to metadata, and with that the possibility of mistakes.

Some additional refactoring made possible now that each rule
resides in its own file. Extend this further to have each rule
in its own package, and move the metadata from the `report`
rule in each package and to the package declaration. We may
then use `rego.metadata.chain()` instead, which includes the
path of the package containing both the category and the title,
thus allowing us to skip having to redeclare that in annotations.

This has a number of benefits:

- Tests are now proper _unit_ tests, and no longer test the whole
  category.
- Checking if a rule is ignored can now be the responsibility of the
  main rule, and linter rules need no longer be concerned with that
  unless they have unique configuration options.
- Reduce the amount of clutter in each rule related to metadata, and
  with that the possibility of mistakes.

Signed-off-by: Anders Eknert <anders@styra.com>
Copy link
Member

@charlieegan3 charlieegan3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me. It's more magic, but I think it's still a good improvement that should speed up adding new rules. It also makes rules share less code, making wider updates easier to make too.

@anderseknert anderseknert merged commit cafd6b7 into main Jun 26, 2023
@anderseknert anderseknert deleted the refactor-factor-fact-or branch June 26, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants