-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Include annotations in rule AST #6771
Include annotations in rule AST #6771
Conversation
Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍, but I think there's one issue we need to fix.
ast/annotations.go
Outdated
var j int | ||
var found bool | ||
for i, a := range cpy { | ||
if rule.Loc().Row > a.Location.Row { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're only looking at rules declared below the annotation, won't we be dropping document-scope annotations for rules that don't happen to be located below the annotation, but have an applicable path?
E.g. this policy might not be good form, but the annotation block applies to both rules data.example.p
:
package example
import rego.v1
p contains {1, rego.metadata.chain()}
# METADATA
# scope: document
# title: p-rules
p contains {2, rego.metadata.chain()}
…ing on row after rule Signed-off-by: Johan Fylling <johan.dev@fylling.se>
No description provided.