Skip to content

Commit

Permalink
Fix negative number returned by rulerefs provider issue (#894)
Browse files Browse the repository at this point in the history
Since character position is a `uint`, this would fail when returned
to the Go handler.

Also:
- Remove some prints we did to debug performance
- Some cleanups to align rulerefs with other providers

Fixes #887

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert committed Jul 3, 2024
1 parent b7c7385 commit 4ec7018
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 32 deletions.
7 changes: 6 additions & 1 deletion bundle/regal/lsp/completion/main.rego
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,9 @@ import rego.v1
# METADATA
# description: main entry point for completion suggestions
# entrypoint: true
items contains data.regal.lsp.completion.providers[_].items[_]
items contains item if {
some provider
completion := data.regal.lsp.completion.providers[provider].items[_]

item := object.union(completion, {"_regal": {"provider": provider}})
}
19 changes: 5 additions & 14 deletions bundle/regal/lsp/completion/providers/rulerefs/rulerefs.rego
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ ref_is_internal(ref) if contains(ref, "._")

default determine_ref_prefix(_) := ""

determine_ref_prefix(word) := word if {
word != ":="
}
determine_ref_prefix(word) := word if word != ":="

position := location.to_position(input.regal.context.location)

line := input.regal.file.lines[position.line]

last_word := regal.last(regex.split(`\s+`, trim_space(line)))
word := location.ref_at(line, input.regal.context.location.col)

workspace_rule_refs contains ref if {
some refs in data.workspace.defined_refs
Expand Down Expand Up @@ -100,7 +98,7 @@ matching_rule_ref_suggestions contains ref if {
line != ""
location.in_rule_body(line)

prefix := determine_ref_prefix(last_word)
prefix := determine_ref_prefix(word.text)

some ref in rule_ref_suggestions

Expand All @@ -120,23 +118,16 @@ grouped_refs[size] contains ref if {
}

items := [item |
some _, group in grouped_refs
some group in grouped_refs
some ref in sort(group)

item := {
"label": ref,
"kind": kind.variable,
"detail": "rule ref",
"textEdit": {
"range": {
"start": {
"line": position.line,
"character": position.character - count(last_word),
},
"end": position,
},
"range": location.word_range(word, position),
"newText": ref,
},
"_regal": {"provider": "rulerefs"},
}
]
18 changes: 10 additions & 8 deletions bundle/regal/lsp/completion/providers/rulerefs/rulerefs_test.rego
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package regal.lsp.completion.providers.rulerefs_test

import data.regal.ast
import data.regal.lsp.completion.providers.rulerefs
import rego.v1

import data.regal.ast

import data.regal.lsp.completion.providers.rulerefs as provider

workspace := {
"current_file.rego": `package foo
Expand Down Expand Up @@ -69,7 +71,7 @@ another_local_rule := `])
}},
}}

items := rulerefs.items with input as regal_module
items := provider.items with input as regal_module
with data.workspace.parsed as parsed_modules
with data.workspace.defined_refs as defined_refs

Expand Down Expand Up @@ -98,11 +100,11 @@ another_local_rule := imp`])
},
"context": {"location": {
"row": 10,
"col": 21,
"col": 26,
}},
}}

items := rulerefs.items with input as regal_module
items := provider.items with input as regal_module
with data.workspace.parsed as parsed_modules
with data.workspace.defined_refs as defined_refs

Expand Down Expand Up @@ -135,7 +137,7 @@ a`])
}},
}}

items := rulerefs.items with input as regal_module
items := provider.items with input as regal_module
with data.workspace.parsed as parsed_modules
with data.workspace.defined_refs as defined_refs

Expand All @@ -161,7 +163,7 @@ local_rule if local`])
}},
}}

items := rulerefs.items with input as regal_module
items := provider.items with input as regal_module
with data.workspace.parsed as parsed_modules
with data.workspace.defined_refs as defined_refs

Expand Down Expand Up @@ -191,7 +193,7 @@ local_func("foo") := local_f`])
}},
}}

items := rulerefs.items with input as regal_module
items := provider.items with input as regal_module
with data.workspace.parsed as parsed_modules
with data.workspace.defined_refs as defined_refs

Expand Down
14 changes: 5 additions & 9 deletions internal/lsp/completions/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package completions

import (
"fmt"
"os"
"time"

"github.com/open-policy-agent/opa/storage"

Expand Down Expand Up @@ -49,23 +47,21 @@ func (m *Manager) Run(params types.CompletionParams, opts *providers.Options) ([
completions := make(map[string][]types.CompletionItem)

if m.isInsideOfComment(params) {
// Exit early if caret position is inside a comment. Most clients won't show
// suggestions there anyway, and there's no need to ask providers for completions.
// 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:
// - Zed: always on, with no way to disable
// - VSCode: disabled but can be enabled with "editor.quickSuggestions.comments" setting
return []types.CompletionItem{}, nil
}

for _, provider := range m.providers {
now := time.Now()

providerCompletions, err := provider.Run(m.c, params, opts)
if err != nil {
return nil, fmt.Errorf("error running completion provider: %w", err)
}

fmt.Fprintf(os.Stderr, "Provider %s took %v\n", provider.Name(), time.Since(now))

for _, completion := range providerCompletions {
// if a provider returns a mandatory completion, return it immediately
// If a provider returns a mandatory completion, return it immediately
// as it is the only completion that should be shown.
if completion.Mandatory {
return []types.CompletionItem{completion}, nil
Expand Down

0 comments on commit 4ec7018

Please sign in to comment.