Handle file:// URLs in exclusion policy (#1207)

This is not an ideal solution, as we should not be sending URLs to
the linter in the first place. But as fixing that is a much broader
issue, we'll have to settle for a hack in the meantime.

Fixes #1204

Signed-off-by: Anders Eknert <[email protected]>

bundle/regal/config/exclusion.rego

@@ -3,46 +3,61 @@ package regal.config
 import rego.v1
 
 # METADATA
-# description: determines if file should be excluded by the given rule
+# description: |
+#   determines if file should be excluded, either because it's globally
+#   ignored, or because the specific rule configuration excludes it
 excluded_file(category, title, file) if {
-	_force_exclude_file(file)
-} else if {
-	rule_config := for_rule(category, title)
-	ex := rule_config.ignore.files
-	is_array(ex)
-	some pattern in ex
-	_exclude(pattern, file)
-} else := false
-
-_force_exclude_file(file) if {
 	# regal ignore:external-reference
 	some pattern in _global_ignore_patterns
-	_exclude(pattern, file)
+	_exclude(pattern, _relative_to_pattern(pattern, file))
+} else if {
+	some pattern in for_rule(category, title).ignore.files
+	_exclude(pattern, _relative_to_pattern(pattern, file))
 }
 
-_global_ignore_patterns := merged_config.ignore.files if {
-	not data.eval.params.ignore_files
-} else := data.eval.params.ignore_files
+# NOTE
+# this is an awful hack which is needed when the language server
+# invokes linting, as it currently will provide filenames in the
+# form of URIs rather than as relative paths... and as we do not
+# know the base/workspace path here, we can only try to make the
+# path relative to the *pattern* providedm which is something...
+#
+# pattern: foo/**/bar.rego
+# file:    file://my/workspace/foo/baz/bar.rego
+# returns: foo/baz/bar.rego
+#
+_relative_to_pattern(pattern, file) := relative if {
+	startswith(file, "file://")
+
+	absolute := trim_suffix(trim_prefix(file, "file://"), "/")
+	file_parts := indexof_n(absolute, "/")
 
-# exclude imitates Gits .gitignore pattern matching as best it can
-# Ref: https://git-scm.com/docs/gitignore#_pattern_format
+	relative := substring(
+		absolute,
+		array.slice(
+			file_parts, (count(file_parts) - strings.count(pattern, "/")) - 1,
+			count(file_parts),
+		)[0] + 1,
+		-1,
+	)
+} else := file
+
+_global_ignore_patterns := data.eval.params.ignore_files
+
+_global_ignore_patterns := merged_config.ignore.files if not data.eval.params.ignore_files
+
+# exclude imitates .gitignore pattern matching as best it can
+# ref: https://git-scm.com/docs/gitignore#_pattern_format
 _exclude(pattern, file) if {
-	patterns := _pattern_compiler(pattern)
-	some p in patterns
+	some p in _pattern_compiler(pattern)
 	glob.match(p, ["/"], file)
-} else := false
+}
 
-# pattern_compiler transforms a glob pattern into a set of glob patterns to make the
-# combined set behave as Gits .gitignore
-_pattern_compiler(pattern) := ps1 if {
-	p := _internal_slashes(pattern)
-	p1 := _leading_slash(p)
-	ps := _leading_doublestar_pattern(p1)
-	ps1 := {pat |
-		some _p, _ in ps
-		nps := _trailing_slash(_p)
-		some pat, _ in nps
-	}
+# pattern_compiler transforms a glob pattern into a set of glob
+# patterns to make the combined set behave as .gitignore
+_pattern_compiler(pattern) := {pat |
+	some p in _leading_doublestar_pattern(trim_prefix(_internal_slashes(pattern), "/"))
+	some pat in _trailing_slash(p)
 }
 
 # Internal slashes means that the path is relative to root,
@@ -73,9 +88,3 @@ _trailing_slash(pattern) := {pattern, np} if {
 	endswith(pattern, "/")
 	np := concat("", [pattern, "**"])
 } else := {pattern}
-
-# If a pattern starts with a "/", the leading slash is ignored but according to
-# the .gitignore rule of internal slashes, it is relative to root
-_leading_slash(pattern) := substring(pattern, 1, -1) if {
-	startswith(pattern, "/")
-} else := pattern

bundle/regal/config/exclusion_test.rego

@@ -64,36 +64,46 @@ rules_config_ignore_delta := {"rules": {"test": {"test-case": {"ignore": {"files
 config_ignore := {"ignore": {"files": ["p.rego"]}}
 
 test_excluded_file_default if {
-	e := config.excluded_file("test", "test-case", "p.rego") with data.eval.params as params
+	not config.excluded_file("test", "test-case", "p.rego") with data.eval.params as params
 		with config.merged_config as rules_config_error
-
-	e == false
 }
 
 test_excluded_file_with_ignore if {
 	c := object.union(rules_config_error, rules_config_ignore_delta)
-	e := config.excluded_file("test", "test-case", "p.rego") with data.eval.params as params
+
+	config.excluded_file("test", "test-case", "p.rego") with data.eval.params as params
 		with config.merged_config as c
-	e == true
 }
 
 test_excluded_file_config if {
-	e := config.excluded_file("test", "test-case", "p.rego") with config.merged_config as config_ignore
-	e == true
+	config.excluded_file("test", "test-case", "p.rego") with config.merged_config as config_ignore
 }
 
 test_excluded_file_cli_flag if {
-	e := config.excluded_file("test", "test-case", "p.rego") with data.eval.params as object.union(
+	config.excluded_file("test", "test-case", "p.rego") with data.eval.params as object.union(
 		params,
 		{"ignore_files": ["p.rego"]},
 	)
-	e == true
 }
 
 test_excluded_file_cli_overrides_config if {
-	e := config.excluded_file("test", "test-case", "p.rego") with config.merged_config as config_ignore
+	not config.excluded_file("test", "test-case", "p.rego") with config.merged_config as config_ignore
 		with data.eval.params as object.union(params, {"ignore_files": [""]})
-	e == false
+}
+
+test_excluded_file_using_uri if {
+	conf := {"rules": {"test": {"rule": {
+		"level": "error",
+		"ignore": {"files": ["foo/**/p.rego"]},
+	}}}}
+
+	config.excluded_file("test", "rule", "file:///workspace/folder/foo/bar/p.rego") with config.merged_config as conf
+}
+
+test_not_excluded_file_using_uri if {
+	conf := {"rules": {"test": {"rule": {"level": "error"}}}}
+
+	not config.excluded_file("test", "rule", "file:///workspace/folder/foo/bar/p.rego") with config.merged_config as conf
 }
 
 test_trailing_slash if {

internal/lsp/lint_test.go

@@ -1,10 +1,14 @@
 package lsp
 
 import (
+	"context"
 	"reflect"
 	"testing"
 
+	"github.com/styrainc/regal/internal/lsp/cache"
 	"github.com/styrainc/regal/internal/lsp/types"
+	"github.com/styrainc/regal/internal/parse"
+	"github.com/styrainc/regal/pkg/config"
 	"github.com/styrainc/regal/pkg/report"
 )
 
@@ -65,3 +69,77 @@ func TestConvertReportToDiagnostics(t *testing.T) {
 		t.Errorf("Expected file diagnostics: %v, got: %v", expectedFileDiags, fileDiags)
 	}
 }
+
+func TestLintWithConfigIgnoreWildcards(t *testing.T) {
+	t.Parallel()
+
+	conf := &config.Config{
+		Rules: map[string]config.Category{
+			"idiomatic": {
+				"directory-package-mismatch": config.Rule{
+					Level: "ignore",
+				},
+			},
+		},
+	}
+
+	contents := "package p\n\nimport rego.v1\n\ncamelCase := 1\n"
+	module := parse.MustParseModule(contents)
+	workspacePath := "/workspace"
+	fileURI := "file:///workspace/ignore/p.rego"
+	state := cache.NewCache()
+
+	state.SetFileContents(fileURI, contents)
+	state.SetModule(fileURI, module)
+	state.SetFileDiagnostics(fileURI, []types.Diagnostic{})
+
+	if err := updateFileDiagnostics(
+		context.Background(),
+		state,
+		conf,
+		fileURI,
+		workspacePath,
+		[]string{"prefer-snake-case"},
+	); err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	diagnostics, _ := state.GetFileDiagnostics(fileURI)
+	if len(diagnostics) != 1 {
+		t.Fatalf("Expected one diagnostic item, got %v", diagnostics)
+	}
+
+	if diagnostics[0].Code != "prefer-snake-case" {
+		t.Errorf("Expected diagnostic code to be prefer-snake-case, got %v", diagnostics[0].Code)
+	}
+
+	// Clear the diagnostic and update the config with a wildcard ignore
+	// for any file in the ignore directory.
+
+	state.SetFileDiagnostics(fileURI, []types.Diagnostic{})
+
+	conf.Rules["style"] = config.Category{
+		"prefer-snake-case": config.Rule{
+			Level: "error",
+			Ignore: &config.Ignore{
+				Files: []string{"ignore/**"},
+			},
+		},
+	}
+
+	if err := updateFileDiagnostics(
+		context.Background(),
+		state,
+		conf,
+		fileURI,
+		workspacePath,
+		[]string{"prefer-snake-case"},
+	); err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	diagnostics, _ = state.GetFileDiagnostics(fileURI)
+	if len(diagnostics) != 0 {
+		t.Fatalf("Expected no diagnostics, got %v", diagnostics)
+	}
+}