Various small fixes (#1562)

- Fix issue with determining Rego version for some paths
- Some Rego improvements

Signed-off-by: Anders Eknert <anders@styra.com>

Author: anderseknert

bundle/regal/ast/search.rego

@@ -50,7 +50,7 @@ find_ref_vars(value) := [var | # regal ignore:narrow-argument
 ]
 
 # one or two vars declared via `every`, i.e. `every x in y {}`
-# or `every`, i.e. `every x, y in y {}`
+# or `every`, i.e. `every x, y in z {}`
 _find_every_vars(value) := array.concat(key_var, val_var) if {
 	key_var := [value.key |
 		value.key.type == "var"
@@ -256,7 +256,7 @@ found.comprehensions[rule_index] contains value if {
 }
 
 # METADATA
-# description: set containing all negated expressions in input AST
+# description: set containing all expressions in input AST
 found.expressions[rule_index] contains value if {
 	some i, rule_index in rule_index_strings
 	some node in ["head", "body", "else"]

bundle/regal/lsp/completion/providers/locals/locals.rego

@@ -43,21 +43,15 @@ items contains item if {
 _excluded(line, position) if _function_args_position(substring(line, 0, position.character))
 
 _function_args_position(text) if {
-	text == trim_left(text, " \t")
 	contains(text, "(")
 	not contains(text, "=")
+	text == trim_left(text, " \t")
 }
 
 default _same_line_loop_vars(_) := []
 
-_same_line_loop_vars(line) := d if {
-	expr := trim_space(line)
-
-	strings.any_prefix_match(expr, {"some", "every"})
+_same_line_loop_vars(line) := vars if {
+	regex.match(`^\s*(some|every)`, line)
 
-	# ---------------------------------------------------> "some k, v in coll"
-	a := substring(expr, 0, indexof(expr, " in")) # -----> "some k, v"
-	b := trim_prefix(trim_prefix(a, "some"), "every") # -> "k, v"
-	c := replace(b, " ", "") # --------------------------> "k,v"
-	d := split(c, ",") # --------------------------------> [k, v] or [v]
+	vars := split(regex.replace(line, `(?:\s*(?:some|every)\s+)(\w+)(?:,?\s*)(\w+)?(?:\s+.*)in.*`, "$1,$2"), ",")
 }

bundle/regal/rules/idiomatic/directory-package-mismatch/directory_package_mismatch.rego

@@ -14,33 +14,30 @@ import data.regal.util
 notices contains result.notice(rego.metadata.chain()) if "no_filename" in config.capabilities.special
 
 report contains violation if {
+	parts := split(input.regal.file.abs, input.regal.environment.path_separator)
+	file_path_values := array.slice(parts, 0, count(parts) - 1)
+
 	# get the last n components from file path, where n == count(_pkg_path_values)
 	file_path_length_matched := array.slice(
-		_file_path_values,
-		count(_file_path_values) - count(_pkg_path_values),
-		count(_file_path_values),
+		file_path_values,
+		count(file_path_values) - count(_pkg_path_values),
+		count(file_path_values),
 	)
 
 	file_path_length_matched != _pkg_path_values
 
-	violation := result.fail(
-		rego.metadata.chain(),
-		# skip the "data" part of the path, as it has no location
-		result.ranged_from_ref(util.rest(input["package"].path)),
-	)
+	# skip the "data" part of the path, as it has no location
+	violation := result.fail(rego.metadata.chain(), result.ranged_from_ref(util.rest(input["package"].path)))
 }
 
-_pkg_path_values := ast.package_path if not config.rules.idiomatic["directory-package-mismatch"]["exclude-test-suffix"]
-
 _pkg_path_values := without_test_suffix if {
 	config.rules.idiomatic["directory-package-mismatch"]["exclude-test-suffix"] == true
 
+	name := regal.last(ast.package_path)
+	endswith(name, "_test")
+
 	without_test_suffix := array.concat(
 		array.slice(ast.package_path, 0, count(ast.package_path) - 1),
-		[trim_suffix(regal.last(ast.package_path), "_test")],
+		[trim_suffix(name, "_test")],
 	)
-}
-
-_file_path_values := array.slice(parts, 0, count(parts) - 1) if {
-	parts := split(input.regal.file.abs, input.regal.environment.path_separator)
-}
+} else := ast.package_path

e2e/cli_test.go

@@ -577,7 +577,7 @@ func TestLintAggregateIgnoreDirective(t *testing.T) {
 	}
 
 	if rep.Summary.NumViolations != 2 {
-		t.Errorf("expected 2 violations, got %d", rep.Summary.NumViolations)
+		t.Fatalf("expected 2 violations, got %d", rep.Summary.NumViolations)
 	}
 
 	if rep.Summary.NumViolations == 0 {

pkg/linter/linter_test.go

@@ -728,7 +728,7 @@ import data.unresolved`,
 	}
 }
 
-// 968805333 ns/op	2970674744 B/op	56876788 allocs/op
+// 968805333 ns/op	2970674744 B/op	56876788 allocs/op - main
 // ...
 func BenchmarkRegalLintingItself(b *testing.B) {
 	conf, err := config.FromPath(filepath.Join("..", "..", ".regal", "config.yaml"))
@@ -741,9 +741,6 @@ func BenchmarkRegalLintingItself(b *testing.B) {
 		WithBaseCache(cache.NewBaseCache()).
 		WithUserConfig(conf)
 
-	b.ResetTimer()
-	b.ReportAllocs()
-
 	var rep report.Report
 
 	for range b.N {
@@ -773,9 +770,6 @@ func BenchmarkRegalLintingItselfPrepareOnce(b *testing.B) {
 		WithUserConfig(conf).
 		MustPrepare(ctx)
 
-	b.ResetTimer()
-	b.ReportAllocs()
-
 	var rep report.Report
 
 	for range b.N {
@@ -790,17 +784,14 @@ func BenchmarkRegalLintingItselfPrepareOnce(b *testing.B) {
 	}
 }
 
-// 164353357 ns/op	455624941 B/op	 8560088 allocs/op
+// 6	 168875708 ns/op	455586606 B/op	 8537889 allocs/op
 // ...
 func BenchmarkRegalNoEnabledRules(b *testing.B) {
 	linter := NewLinter().
 		WithInputPaths([]string{"../../bundle"}).
 		WithBaseCache(cache.NewBaseCache()).
 		WithDisableAll(true)
 
-	b.ResetTimer()
-	b.ReportAllocs()
-
 	var err error
 
 	var rep report.Report
@@ -826,9 +817,6 @@ func BenchmarkRegalNoEnabledRulesPrepareOnce(b *testing.B) {
 		WithDisableAll(true).
 		MustPrepare(context.Background())
 
-	b.ResetTimer()
-	b.ReportAllocs()
-
 	var err error
 
 	var rep report.Report

pkg/rules/rules.go

@@ -152,12 +152,12 @@ func RegoVersionFromVersionsMap(
 
 	var longestMatch int
 
-	dir := filepath.Dir(filename)
+	dir := path.Join("/", filepath.Dir(filename))
 
 	for versionedDir := range versionsMap {
 		matchingVersionedDir := path.Join("/", versionedDir, "/")
 
-		if strings.HasPrefix(dir+"/", matchingVersionedDir) {
+		if strings.HasPrefix(dir, matchingVersionedDir) {
 			// >= as the versioned dir might be "" for the project root
 			if len(versionedDir) >= longestMatch {
 				longestMatch = len(versionedDir)

pkg/rules/rules_test.go

@@ -77,6 +77,15 @@ func TestRegoVersionFromVersionsMap(t *testing.T) {
 			Filename:        "/foo/bar.rego",
 			ExpectedVersion: ast.RegoV1,
 		},
+		"file has version from current dir (no leading slash)": {
+			VersionsMap: map[string]ast.RegoVersion{
+				"foo":     ast.RegoV1,
+				"bar":     ast.RegoV0,
+				"unknown": ast.RegoUndefined,
+			},
+			Filename:        "/foo/bar.rego",
+			ExpectedVersion: ast.RegoV1,
+		},
 		"file has version from parent dir": {
 			VersionsMap: map[string]ast.RegoVersion{
 				"foo":     ast.RegoV1,