Remove used refs completion provider (#896)

Refs, whether used or not, are now handled by the `rulerefs` provider, and the suggestions from "used" refs felt mostly like duplicating the suggestions of the former. Signed-off-by: Anders Eknert <[email protected]>
StyraInc · Jul 3, 2024 · d000d21 · d000d21
1 parent 4ec7018
commit d000d21
Show file tree

Hide file tree

Showing 6 changed files with 4 additions and 340 deletions.
diff --git a/internal/lsp/cache/cache.go b/internal/lsp/cache/cache.go
@@ -50,11 +50,6 @@ type Cache struct {
 	// fileRefs is expected to be updated when a file is successfully parsed.
 	fileRefs  map[string]map[string]types.Ref
 	fileRefMu sync.Mutex
-
-	// usedRefs is a map of file URI to a list of string ref names used in that file.
-	// These are intended to be used for completions in that file.
-	usedRefs   map[string][]string
-	usedRefsMu sync.Mutex
 }
 
 func NewCache() *Cache {
@@ -71,8 +66,6 @@ func NewCache() *Cache {
 		builtinPositionsFile: make(map[string]map[uint][]types.BuiltinPosition),
 
 		fileRefs: make(map[string]map[string]types.Ref),
-
-		usedRefs: make(map[string][]string),
 	}
 }
 
@@ -279,22 +272,6 @@ func (c *Cache) GetAllFileRefs() map[string]map[string]types.Ref {
 	return maps.Clone(c.fileRefs)
 }
 
-func (c *Cache) SetUsedRefs(fileURI string, items []string) {
-	c.usedRefsMu.Lock()
-	defer c.usedRefsMu.Unlock()
-
-	c.usedRefs[fileURI] = items
-}
-
-func (c *Cache) GetUsedRefs(fileURI string) ([]string, bool) {
-	c.usedRefsMu.Lock()
-	defer c.usedRefsMu.Unlock()
-
-	refs, ok := c.usedRefs[fileURI]
-
-	return refs, ok
-}
-
 // Delete removes all cached data for a given URI. Ignored file contents are
 // also removed if found for a matching URI.
 func (c *Cache) Delete(fileURI string) {
@@ -326,10 +303,6 @@ func (c *Cache) Delete(fileURI string) {
 	delete(c.fileRefs, fileURI)
 	c.fileRefMu.Unlock()
 
-	c.usedRefsMu.Lock()
-	delete(c.usedRefs, fileURI)
-	c.usedRefsMu.Unlock()
-
 	c.ignoredFileContentsMu.Lock()
 	delete(c.ignoredFileContents, fileURI)
 	c.ignoredFileContentsMu.Unlock()

diff --git a/internal/lsp/completions/manager.go b/internal/lsp/completions/manager.go
@@ -36,16 +36,13 @@ func NewDefaultManager(c *cache.Cache, store storage.Store) *Manager {
 	m.RegisterProvider(&providers.RuleHead{})
 	m.RegisterProvider(&providers.RuleHeadKeyword{})
 	m.RegisterProvider(&providers.Input{})
-	m.RegisterProvider(&providers.UsedRefs{})
 
 	m.RegisterProvider(providers.NewPolicy(store))
 
 	return m
 }
 
 func (m *Manager) Run(params types.CompletionParams, opts *providers.Options) ([]types.CompletionItem, error) {
-	completions := make(map[string][]types.CompletionItem)
-
 	if m.isInsideOfComment(params) {
 		// Exit early if caret position is inside a comment. We currently don't have any provider
 		// where doing completions inside of a comment makes much sense. Behavior is also editor-specific:
@@ -54,6 +51,8 @@ func (m *Manager) Run(params types.CompletionParams, opts *providers.Options) ([
 		return []types.CompletionItem{}, nil
 	}
 
+	var completionsList []types.CompletionItem
+
 	for _, provider := range m.providers {
 		providerCompletions, err := provider.Run(m.c, params, opts)
 		if err != nil {
@@ -67,62 +66,14 @@ func (m *Manager) Run(params types.CompletionParams, opts *providers.Options) ([
 				return []types.CompletionItem{completion}, nil
 			}
 
-			completions[completion.Label] = append(completions[completion.Label], completion)
-		}
-	}
-
-	var completionsList []types.CompletionItem
-
-	for _, completionItems := range completions {
-		if len(completionItems) < 2 {
-			completionsList = append(completionsList, completionItems...)
-
-			continue
-		}
-
-		maxRank := 0
-
-		for _, completion := range completionItems {
-			if completion.Regal == nil {
-				continue
-			}
-
-			if rank := rankProvider(completion.Regal.Provider); rank > maxRank {
-				maxRank = rank
-			}
+			completion.Regal = nil
+			completionsList = append(completionsList, completion)
 		}
-
-		for _, completion := range completionItems {
-			if completion.Regal == nil {
-				completionsList = append(completionsList, completion)
-
-				continue
-			}
-
-			if rank := rankProvider(completion.Regal.Provider); rank == maxRank {
-				completionsList = append(completionsList, completion)
-			}
-		}
-	}
-
-	for i := range completionsList {
-		completionsList[i].Regal = nil
 	}
 
 	return completionsList, nil
 }
 
-func rankProvider(provider string) int {
-	switch provider {
-	case "rulerefs":
-		return 100
-	case "usedrefs":
-		return 90
-	default:
-		return 0
-	}
-}
-
 func (m *Manager) RegisterProvider(provider Provider) {
 	m.providers = append(m.providers, provider)
 }

diff --git a/internal/lsp/completions/manager_test.go b/internal/lsp/completions/manager_test.go
@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/open-policy-agent/opa/ast"
-	"github.com/open-policy-agent/opa/storage/inmem"
 
 	"github.com/styrainc/regal/internal/lsp/cache"
 	"github.com/styrainc/regal/internal/lsp/completions/providers"
@@ -49,81 +48,3 @@ func TestManagerEarlyExitInsideComment(t *testing.T) {
 		t.Errorf("Expected no completions, got: %v", completions)
 	}
 }
-
-func TestManagerRankCompletions(t *testing.T) {
-	t.Parallel()
-
-	file1Contents := `package example
-
-import rego.v1
-
-foo := true
-`
-
-	file1 := ast.MustParseModule(file1Contents)
-
-	file2Contents := `package example2
-
-import rego.v1
-
-bar := data.example.foo
-`
-
-	file2 := ast.MustParseModule(file2Contents)
-
-	store := inmem.NewFromObject(map[string]interface{}{
-		"workspace": map[string]interface{}{
-			"parsed": map[string]interface{}{
-				"file:///file1.rego": file1,
-				"file:///file2.rego": file2,
-			},
-		},
-	})
-
-	policyProvider := providers.NewPolicy(store)
-
-	file2ContentsEdited := `package example2
-
-import rego.v1
-
-bar := data.example.foo
-
-baz := data.
-`
-
-	c := cache.NewCache()
-
-	c.SetFileContents("file:///file2.rego", file2ContentsEdited)
-	c.SetUsedRefs("file:///file2.rego", []string{"data.example.foo"})
-
-	mgr := NewManager(c, &ManagerOptions{})
-	mgr.RegisterProvider(&providers.UsedRefs{})
-	mgr.RegisterProvider(policyProvider)
-
-	completionParams := types.CompletionParams{
-		TextDocument: types.TextDocumentIdentifier{
-			URI: "file:///file2.rego",
-		},
-		Position: types.Position{
-			Line:      6,
-			Character: 13,
-		},
-	}
-
-	completions, err := mgr.Run(completionParams, &providers.Options{})
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
-	}
-
-	count := 0
-
-	for _, item := range completions {
-		if item.Label == "data.example.foo" {
-			count++
-		}
-	}
-
-	if count != 1 {
-		t.Fatalf("Expected exactly one completion for 'data.example.foo', got: %v", completions)
-	}
-}
diff --git a/internal/lsp/completions/providers/used_refs.go b/internal/lsp/completions/providers/used_refs.go