From 1db229a520710c814cacc2a8e4cf9ec1f60f3c66 Mon Sep 17 00:00:00 2001
From: Anders Eknert <anders@styra.com>
Date: Mon, 27 Jan 2025 12:36:13 +0100
Subject: [PATCH] Small fixes (#1367)

- Use `count` on `data.eval.params.ignore_files` as it's now
  always defined
- Set `inmem.OptReturnASTValuesOnRead(true)` on the LSP inmem
  store for a substantial performance boost!
- Remove `rego.StoreReadAST(true)` which was ignored since we
  set the store explicitly

Signed-off-by: Anders Eknert <anders@styra.com>
---
 bundle/regal/config/exclusion.rego           |  6 ++--
 internal/lsp/completions/providers/policy.go |  1 -
 internal/lsp/store.go                        |  2 +-
 internal/lsp/store_test.go                   | 31 ++++++++++++++++----
 pkg/config/config.go                         |  2 +-
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/bundle/regal/config/exclusion.rego b/bundle/regal/config/exclusion.rego
index 1831f442..caa1090f 100644
--- a/bundle/regal/config/exclusion.rego
+++ b/bundle/regal/config/exclusion.rego
@@ -13,9 +13,9 @@ excluded_file(category, title, file) if {
 	_exclude(pattern, file)
 }
 
-_global_ignore_patterns := data.eval.params.ignore_files
-
-_global_ignore_patterns := merged_config.ignore.files if not data.eval.params.ignore_files
+_global_ignore_patterns := data.eval.params.ignore_files if {
+	count(data.eval.params.ignore_files) > 0
+} else := merged_config.ignore.files
 
 # exclude imitates .gitignore pattern matching as best it can
 # ref: https://git-scm.com/docs/gitignore#_pattern_format
diff --git a/internal/lsp/completions/providers/policy.go b/internal/lsp/completions/providers/policy.go
index 44a3b47c..36cf4fc0 100644
--- a/internal/lsp/completions/providers/policy.go
+++ b/internal/lsp/completions/providers/policy.go
@@ -133,7 +133,6 @@ func prepareQuery(ctx context.Context, store storage.Store, query string) (*rego
 
 func prepareRegoArgs(store storage.Store, query ast.Body) []func(*rego.Rego) {
 	return []func(*rego.Rego){
-		rego.StoreReadAST(true),
 		rego.Store(store),
 		rego.ParsedQuery(query),
 		rego.ParsedBundle("regal", &rbundle.LoadedBundle),
diff --git a/internal/lsp/store.go b/internal/lsp/store.go
index 49fb4364..21ceb100 100644
--- a/internal/lsp/store.go
+++ b/internal/lsp/store.go
@@ -20,7 +20,7 @@ func NewRegalStore() storage.Store {
 			// we'll need to conform to the most basic "JSON" format understood by the store
 			"defined_refs": map[string]any{},
 		},
-	}, inmem.OptRoundTripOnWrite(false))
+	}, inmem.OptRoundTripOnWrite(false), inmem.OptReturnASTValuesOnRead(true))
 }
 
 func transact(ctx context.Context, store storage.Store, writeMode bool, op func(txn storage.Transaction) error) error {
diff --git a/internal/lsp/store_test.go b/internal/lsp/store_test.go
index e5bfb714..79615a48 100644
--- a/internal/lsp/store_test.go
+++ b/internal/lsp/store_test.go
@@ -3,14 +3,21 @@ package lsp
 import (
 	"context"
 	"encoding/json"
-	"slices"
+	"fmt"
 	"testing"
 
+	"github.com/open-policy-agent/opa/v1/ast"
 	"github.com/open-policy-agent/opa/v1/storage"
 
 	"github.com/styrainc/regal/internal/parse"
 )
 
+type illegalResolver struct{}
+
+func (illegalResolver) Resolve(ref ast.Ref) (interface{}, error) {
+	return nil, fmt.Errorf("illegal value: %v", ref)
+}
+
 func TestPutFileModStoresRoastRepresentation(t *testing.T) {
 	t.Parallel()
 
@@ -28,7 +35,17 @@ func TestPutFileModStoresRoastRepresentation(t *testing.T) {
 		t.Fatalf("store.Read failed: %v", err)
 	}
 
-	pretty, err := json.MarshalIndent(parsed, "", "  ")
+	parsedVal, ok := parsed.(ast.Value)
+	if !ok {
+		t.Fatalf("expected ast.Value, got %T", parsed)
+	}
+
+	parsedMap, err := ast.ValueToInterface(parsedVal, illegalResolver{})
+	if err != nil {
+		t.Fatalf("ast.ValueToInterface failed: %v", err)
+	}
+
+	pretty, err := json.MarshalIndent(parsedMap, "", "  ")
 	if err != nil {
 		t.Fatalf("json.MarshalIndent failed: %v", err)
 	}
@@ -95,12 +112,14 @@ func TestPutFileRefs(t *testing.T) {
 		t.Fatalf("store.Read failed: %v", err)
 	}
 
-	arr, ok := value.([]string)
+	arr, ok := value.(*ast.Array)
 	if !ok {
-		t.Fatalf("expected []string, got %T", value)
+		t.Fatalf("expected *ast.Array, got %T", value)
 	}
 
-	if !slices.Equal(arr, []string{"foo", "bar"}) {
-		t.Fatalf("expected [foo bar], got %v", arr)
+	expected := ast.NewArray(ast.StringTerm("foo"), ast.StringTerm("bar"))
+
+	if !expected.Equal(arr) {
+		t.Errorf("expected %v, got %v", expected, arr)
 	}
 }
diff --git a/pkg/config/config.go b/pkg/config/config.go
index e3614f36..5ed430cd 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -760,7 +760,7 @@ func fromOPABuiltin(builtin ast.Builtin) *Builtin {
 func fromOPACapabilities(capabilities *ast.Capabilities) *Capabilities {
 	var result Capabilities
 
-	result.Builtins = make(map[string]*Builtin)
+	result.Builtins = make(map[string]*Builtin, len(capabilities.Builtins))
 
 	for _, builtin := range capabilities.Builtins {
 		result.Builtins[builtin.Name] = fromOPABuiltin(*builtin)