-
Notifications
You must be signed in to change notification settings - Fork 35
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
Enhanced type checking using Regal AST schema #201
Conversation
ebe9005
to
c252b6b
Compare
// so while this isn't pretty, we'll use a separate package to carry state from here. If you | ||
// know of a better way of doing this, don't hesitate to fix this. | ||
embeds.EmbedBundleFS = bundle | ||
embeds.ASTSchema = schema |
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.
thought: hmm, yeah. I don't know of a better way to do this. I think it's best to have the bundle at the top level. I wonder if schema needs to be too though? Perhaps it could sit under embeds
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.
True. I'll leave it as is for now, but I might move the schema into embeds later 👍
Currently enabled for the `regal test` command, which is the most likely entrypoint for rule authors, whether for built-in Regal rules or custom ones. While it would be useful to provide this also for `regal lint` — we'll probably want to do this together with the strictness rules, as we currently don't require compilation, and it's not clear that we'll want to unless a rule is configured that needs this, like most of the strictness rules would. Additionally, the schema is provided when running the Rego tests via `go test ./...`, which should help catch errors we make ourselves. It would be nice if custom rule authors didn't have to include the schema in the metadata annotation, but I've yet to figure out the best way of doing that, as we can't bundle a custom policy and also have them read from disk without the bundle "owning" that path. I suppose we could just parse a policy from a string, including a subpackage scoped custom.regal.rules package annotation... but I'll need to think more about this. Fixes #52 Signed-off-by: Anders Eknert <anders@styra.com>
c252b6b
to
1217e28
Compare
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.
This looks good to me! Tried it out too 🙂
$ go run main.go test e2e/testdata/ast_type_failure/
1 error occurred: e2e/testdata/ast_type_failure/custom_type_fail.rego:13: rego_type_error: undefined ref: input.foo
input.foo
^
have: "foo"
want (one of): ["annotations" "comments" "imports" "package" "regal" "rules"]
exit status 1
Currently enabled for the `regal test` command, which is the most likely entrypoint for rule authors, whether for built-in Regal rules or custom ones. While it would be useful to provide this also for `regal lint` — we'll probably want to do this together with the strictness rules, as we currently don't require compilation, and it's not clear that we'll want to unless a rule is configured that needs this, like most of the strictness rules would. Additionally, the schema is provided when running the Rego tests via `go test ./...`, which should help catch errors we make ourselves. It would be nice if custom rule authors didn't have to include the schema in the metadata annotation, but I've yet to figure out the best way of doing that, as we can't bundle a custom policy and also have them read from disk without the bundle "owning" that path. I suppose we could just parse a policy from a string, including a subpackage scoped custom.regal.rules package annotation... but I'll need to think more about this. Fixes StyraInc#52 Signed-off-by: Anders Eknert <anders@styra.com>
Currently enabled for the
regal test
command, which is the most likely entrypoint for rule authors, whether for built-in Regal rules or custom ones.While it would be useful to provide this also for
regal lint
— we'll probably want to do this together with the strictness rules, as we currently don't require compilation, and it's not clear that we'll want to unless a rule is configured that needs this, like most of the strictness rules would.Additionally, the schema is provided when running the Rego tests via
go test ./...
, which should help catch errors we make ourselves.It would be nice if custom rule authors didn't have to include the schema in the metadata annotation, but I've yet to figure out the best way of doing that, as we can't bundle a custom policy and also have them read from disk without the bundle "owning" that path. I suppose we could just parse a policy from a string, including a subpackage scoped custom.regal.rules package annotation... but I'll need to think more about this.
Fixes #52