Skip to content

Commit

Permalink
Ignore refs from ignored and unimported files
Browse files Browse the repository at this point in the history
Fixes #766

Signed-off-by: Charlie Egan <charlie@styra.com>
  • Loading branch information
charlieegan3 committed Jun 13, 2024
1 parent e3e12a9 commit ada4a70
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 7 deletions.
9 changes: 8 additions & 1 deletion internal/lsp/completions/providers/options.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
package providers

import (
"github.com/styrainc/regal/internal/lsp/clients"
"github.com/styrainc/regal/pkg/config"
)

type Options struct {
RootURI string
RootURI string
ClientIdentifier clients.Identifier
Ignore config.Ignore
}
35 changes: 32 additions & 3 deletions internal/lsp/completions/providers/rulerefs.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package providers

import (
"errors"
"strings"

"github.com/styrainc/regal/internal/lsp/cache"
"github.com/styrainc/regal/internal/lsp/types"
"github.com/styrainc/regal/internal/lsp/types/completion"
"github.com/styrainc/regal/internal/lsp/uri"
"github.com/styrainc/regal/pkg/config"
)

// RuleRefs is a completion provider that returns completions for
Expand All @@ -18,8 +21,13 @@ type RuleRefs struct{}
func (*RuleRefs) Run(
c *cache.Cache,
params types.CompletionParams,
_ *Options,
opts *Options,
) ([]types.CompletionItem, error) {
// opts are needed here to ignore refs from ignored files
if opts == nil {
return nil, errors.New("options are required")
}

fileURI := params.TextDocument.URI

lines, currentLine := completionLineHelper(c, fileURI, params.Position.Line)
Expand All @@ -43,7 +51,19 @@ func (*RuleRefs) Run(

refsForContext := make(map[string]types.Ref)

for uri, refs := range c.GetAllFileRefs() {
for workspaceFileURI, refs := range c.GetAllFileRefs() {
var isIgnored bool

paths, err := config.FilterIgnoredPaths(
[]string{uri.ToPath(opts.ClientIdentifier, workspaceFileURI)},
opts.Ignore.Files,
false,
uri.ToPath(opts.ClientIdentifier, opts.RootURI),
)
if err != nil || len(paths) == 0 {
isIgnored = true
}

for _, ref := range refs {
// we are not interested in packages here, only rules
if ref.Kind == types.Package {
Expand All @@ -58,17 +78,26 @@ func (*RuleRefs) Run(

// for refs from imported packages, we need to strip the start of the
// package string, e.g. data.foo.bar -> bar
var isImported bool

key := ref.Label

for _, i := range mod.Imports {
if k := attemptToStripImportPrefix(key, i.Path.String()); k != "" {
key = k

isImported = true

break
}
}

if isIgnored && !isImported {
continue
}

// suggest rules from the current file without any package prefix
if uri == fileURI {
if workspaceFileURI == fileURI {
parts := strings.Split(key, ".")
key = parts[len(parts)-1]
}
Expand Down
162 changes: 160 additions & 2 deletions internal/lsp/completions/providers/rulerefs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import (
"testing"

"github.com/styrainc/regal/internal/lsp/cache"
"github.com/styrainc/regal/internal/lsp/clients"
"github.com/styrainc/regal/internal/lsp/completions/refs"
"github.com/styrainc/regal/internal/lsp/types"
"github.com/styrainc/regal/internal/parse"
"github.com/styrainc/regal/pkg/config"
)

func TestRuleFromImportedPackageRefs(t *testing.T) {
func TestRuleFromWorkspaceRefs(t *testing.T) {
t.Parallel()

c := cache.NewCache()
Expand Down Expand Up @@ -75,7 +77,10 @@ deny := false
},
}

completions, err := p.Run(c, completionParams, nil)
completions, err := p.Run(c, completionParams, &Options{
RootURI: "file:///",
ClientIdentifier: clients.IdentifierGeneric,
})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
Expand Down Expand Up @@ -106,3 +111,156 @@ deny := false
)
}
}

func TestRuleFromImportedPackageRefs_IgnoredFiles(t *testing.T) {
t.Parallel()

c := cache.NewCache()

currentlyEditingFileContents := `package example`

regoFiles := map[string]string{
"file:///bar/bar.rego": `package bar
import rego.v1
default allow := false
allow if input.admin
`,
"file:///example.rego": currentlyEditingFileContents,
}

for uri, contents := range regoFiles {
mod, err := parse.Module(uri, contents)
if err != nil {
t.Fatalf("Unexpected error when parsing %s contents: %v", uri, err)
}

c.SetModule(uri, mod)

c.SetFileRefs(uri, refs.DefinedInModule(mod))
}

c.SetFileContents("file:///example.rego", currentlyEditingFileContents+"\n\nallow if d")

p := &RuleRefs{}

completionParams := types.CompletionParams{
TextDocument: types.TextDocumentIdentifier{
URI: "file:///example.rego",
},
Position: types.Position{
Line: 2,
Character: 8,
},
}

completions, err := p.Run(c, completionParams, &Options{
RootURI: "file:///",
ClientIdentifier: clients.IdentifierGeneric,
Ignore: config.Ignore{
Files: []string{"bar/bar.rego"},
},
})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

expectedRefs := []string{}

foundRefs := make([]string, len(completions))

for i, c := range completions {
foundRefs[i] = c.Label
}

slices.Sort(foundRefs)

if !slices.Equal(expectedRefs, foundRefs) {
t.Fatalf(
"Expected completions to be\n%s\ngot:\n%s",
strings.Join(expectedRefs, "\n"),
strings.Join(foundRefs, "\n"),
)
}
}

func TestRuleFromImportedPackageRefs_IgnoredFileButRefImportedExplicitly(t *testing.T) {
t.Parallel()

c := cache.NewCache()

currentlyEditingFileContents := `package example
import data.bar
`

regoFiles := map[string]string{
"file:///bar/bar.rego": `package bar
import rego.v1
default allow := false
allow if input.admin
`,
"file:///example.rego": currentlyEditingFileContents,
}

for uri, contents := range regoFiles {
mod, err := parse.Module(uri, contents)
if err != nil {
t.Fatalf("Unexpected error when parsing %s contents: %v", uri, err)
}

c.SetModule(uri, mod)

c.SetFileRefs(uri, refs.DefinedInModule(mod))
}

c.SetFileContents("file:///example.rego", currentlyEditingFileContents+"\n\nallow if b")

p := &RuleRefs{}

completionParams := types.CompletionParams{
TextDocument: types.TextDocumentIdentifier{
URI: "file:///example.rego",
},
Position: types.Position{
Line: 5,
Character: 8,
},
}

completions, err := p.Run(c, completionParams, &Options{
RootURI: "file:///",
ClientIdentifier: clients.IdentifierGeneric,
Ignore: config.Ignore{
Files: []string{"bar/bar.rego"},
},
})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

expectedRefs := []string{
"bar.allow",
}

foundRefs := make([]string, len(completions))

for i, c := range completions {
foundRefs[i] = c.Label
}

slices.Sort(foundRefs)

if !slices.Equal(expectedRefs, foundRefs) {
t.Fatalf(
"Expected completions to be\n%s\ngot:\n%s",
strings.Join(expectedRefs, "\n"),
strings.Join(foundRefs, "\n"),
)
}
}
10 changes: 9 additions & 1 deletion internal/lsp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,15 @@ func (l *LanguageServer) handleTextDocumentCompletion(
}

// items is allocated here so that the return value is always a non-nil CompletionList
items, err := l.completionsManager.Run(params, &providers.Options{RootURI: l.clientRootURI})
opts := &providers.Options{
RootURI: l.clientRootURI,
ClientIdentifier: l.clientIdentifier,
}
if l.loadedConfig != nil {
opts.Ignore = l.loadedConfig.Ignore
}

items, err := l.completionsManager.Run(params, opts)
if err != nil {
return nil, fmt.Errorf("failed to find completions: %w", err)
}
Expand Down

0 comments on commit ada4a70

Please sign in to comment.