Skip to content

Commit

Permalink
Fix missing annotations in LSP parsing (#1354)
Browse files Browse the repository at this point in the history
Also, update the missing-metadata rule to report violation
locations on the rule head ref and not the whole rule head.

Signed-off-by: Anders Eknert <[email protected]>
  • Loading branch information
anderseknert authored Jan 21, 2025
1 parent d9749f4 commit bea7398
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
15 changes: 11 additions & 4 deletions bundle/regal/rules/custom/missing-metadata/missing_metadata.rego
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,23 @@ _rule_annotations[rule_path] contains annotated if {
annotated := count(object.get(rule, "annotations", [])) > 0
}

_rule_locations[rule_path] := util.to_location_object(location) if {
_rule_locations[rule_path] := location if {
some rule_path, annotated in _rule_annotations

# we only care about locations of non-annotated rules
not true in annotated

location := [h.location |
h := ast.public_rules_and_functions[_].head
concat(".", [ast.package_name, ast.ref_static_to_string(h.ref)]) == rule_path
first_rule_index := [i |
some i

# false positive: https://github.com/StyraInc/regal/issues/1353
# regal ignore:unused-output-variable
ref := ast.public_rules_and_functions[i].head.ref
concat(".", [ast.package_name, ast.ref_static_to_string(ref)]) == rule_path
][0]

ref := ast.public_rules_and_functions[first_rule_index].head.ref
location := object.remove(result.ranged_from_ref(ref).location, ["file"])
}

# METADATA
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ none := false
"col": 1,
"row": 9,
"end": {
"col": 14,
"col": 5,
"row": 9,
},
"text": "none := false",
Expand Down Expand Up @@ -143,7 +143,7 @@ baz := true
"file": "p.rego",
"row": 5,
"end": {
"col": 12,
"col": 4,
"row": 5,
},
"text": "baz := true",
Expand Down
9 changes: 3 additions & 6 deletions internal/lsp/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,10 @@ func updateParse(
}

lines := strings.Split(content, "\n")
options := rparse.ParserOptions()
options.RegoVersion = version

var (
err error
module *ast.Module
)

module, err = rparse.ModuleWithOpts(fileURI, content, ast.ParserOptions{RegoVersion: version})
module, err := rparse.ModuleWithOpts(fileURI, content, options)
if err == nil {
// if the parse was ok, clear the parse errors
cache.SetParseErrors(fileURI, []types.Diagnostic{})
Expand Down
55 changes: 55 additions & 0 deletions internal/lsp/server_aggregates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,61 @@ import data.qux # new name for bar.rego package
}
}

// Test to ensure that annotations are parsed correctly.
func TestRulesWithMetadataNotReportedForMissingMeta(t *testing.T) {
t.Parallel()

files := map[string]string{
"foo.rego": `# METADATA
# title: foo
package foo
`,
"bar.rego": `# METADATA
# title: foo
package bar
`,
".regal/config.yaml": `rules:
custom:
missing-metadata:
level: error
idiomatic:
directory-package-mismatch:
level: ignore
`,
}

messages := createMessageChannels(files)
logger := newTestLogger(t)
clientHandler := createClientHandler(t, logger, messages)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

tempDir := t.TempDir()

_, _, err := createAndInitServer(ctx, logger, tempDir, files, clientHandler)
if err != nil {
t.Fatalf("failed to create and init language server: %s", err)
}

timeout := time.NewTimer(determineTimeout())
defer timeout.Stop()

// no missing-metadata
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if len(violations) > 0 {
t.Errorf("unexpected violations for foo.rego: %v", violations)
}

success = true
case <-timeout.C:
t.Fatalf("timed out waiting for expected foo.rego diagnostics")
}
}
}

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

Expand Down

0 comments on commit bea7398

Please sign in to comment.