-
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
[lsp] Implement rulerefs in rego #849
Conversation
Signed-off-by: Charlie Egan <[email protected]>
Signed-off-by: Charlie Egan <[email protected]>
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.
Amazing work! Some questions, but OK to merge now and discuss more later.
some file, parsed in data.workspace.parsed | ||
|
||
package_name := concat(".", [path.value | | ||
some i, path in parsed["package"].path |
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.
Don't need i
here
And yes, we shouldn't have to care: #60
@@ -398,7 +401,15 @@ | |||
}, | |||
"context": { | |||
"description": "extra attributes provided in the specific evaluation context", | |||
"type": "object" | |||
"type": "object", |
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.
I wonder if we should just create a new schema for this entrypoint if we're not going to use the AST from there
@@ -35,8 +35,8 @@ var regalRules = func() bundle.Bundle { | |||
// NewPolicy creates a new Policy provider. This provider is distinctly different from the other providers | |||
// as it acts like the entrypoint for all Rego-based providers, and not a single provider "function" like | |||
// the Go providers do. | |||
func NewPolicy() *Policy { | |||
pq, err := prepareQuery("completions := data.regal.lsp.completion.items") | |||
func NewPolicy(store storage.Store) *Policy { |
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.
Very nice!
@@ -22,27 +23,58 @@ import ( | |||
// updateParse updates the module cache with the latest parse result for a given URI, | |||
// if the module cannot be parsed, the parse errors are saved as diagnostics for the | |||
// URI instead. | |||
func updateParse(ctx context.Context, cache *cache.Cache, uri string) (bool, error) { | |||
content, ok := cache.GetFileContents(uri) | |||
func updateParse(ctx context.Context, cache *cache.Cache, store storage.Store, fileURI string) (bool, error) { |
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.
I guess we'll have to make sure never to reference this storage from regular linting policies, as this isn't going to be available in this context?
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.
Indeed, it's only going to be available in the lsp.
This update has the locals provider also use the workspace contents Signed-off-by: Charlie Egan <[email protected]>
Thanks for the review, will patch up when I'm back on tomorrow morning but I've also added 694ea52 |
* [lsp] Implement rulerefs in rego Signed-off-by: Charlie Egan <[email protected]> * [lsp] Add client id to rego input context Signed-off-by: Charlie Egan <[email protected]> * [lsp] update local provider This update has the locals provider also use the workspace contents Signed-off-by: Charlie Egan <[email protected]> --------- Signed-off-by: Charlie Egan <[email protected]>
This implements the rule_refs provider in Rego. This involves using a shared inmem rego store to allow the parsed modules for all workspace files to be accessible when running the new Rego-based policy provider.