Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle file:// URLs in exclusion policy #1207

Merged
merged 1 commit into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 46 additions & 37 deletions bundle/regal/config/exclusion.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
32 changes: 21 additions & 11 deletions bundle/regal/config/exclusion_test.rego
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
78 changes: 78 additions & 0 deletions internal/lsp/lint_test.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand Down Expand Up @@ -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)
}
}