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

Ignore directives for aggregate rules #344

Closed
anderseknert opened this issue Sep 23, 2023 · 1 comment
Closed

Ignore directives for aggregate rules #344

anderseknert opened this issue Sep 23, 2023 · 1 comment

Comments

@anderseknert
Copy link
Member

We will need to figure out what ignoring something means in the context of aggregate rules.

rules:
  testing: 
    some-aggregate-rule:
      level: error

ignore:
  files:
    - foo/bar.rego

The way we currently work with ignore directives, foo/bar.rego wouldn't even get parsed. This has proven to be necessary, as foo/bar.rego might not even contain valid Rego. However, perhaps we'll need to consider more fine-grained ignores for aggregate rules? It seems valid to me that one would want to include some file in aggregation, but not necessarily in the linter report. In other words, "use this file to determine whether we have a violation in another file, but ignore violations in the file itself".

Also, if an aggregate rule says "only allow imports of packages, not rules", we'll want to respect explicit, line-level ignore directives of that rule:

# regal ignore:prefer-package-imports
import foo.bar.rule

We should also provide a way for aggregate rules to consider this the same way we do for normal rules.

anderseknert added a commit that referenced this issue Mar 26, 2024
This fixes one issue reported in #605, but not the issue itself, as
that'll need more consideration — concerns mostly described in #344

Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert added a commit that referenced this issue Mar 26, 2024
This fixes one issue reported in #605, but not the issue itself, as
that'll need more consideration — concerns mostly described in #344

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

Having thought more about it, ignored files should be ignored entirely and not be aggregated. Similar to how we should treat them in the language server, i.e. not use them for completion suggestions and so on.

The ignore directive issue is however still relevant, but better described in #803, so I'll close this in favor of that issue.

srenatus pushed a commit to srenatus/regal that referenced this issue Oct 1, 2024
This fixes one issue reported in StyraInc#605, but not the issue itself, as
that'll need more consideration — concerns mostly described in StyraInc#344

Signed-off-by: Anders Eknert <anders@styra.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

1 participant