rule: Disable leaked_internal_reference for test files (#1228)

* rule: Disable leaked_internal_reference for test files

Option added to turn back on if needed

Signed-off-by: Charlie Egan <[email protected]>

* Explain the rule is of by default for tests

Signed-off-by: Charlie Egan <[email protected]>

---------

Signed-off-by: Charlie Egan <[email protected]>

bundle/regal/rules/bugs/leaked-internal-reference/leaked_internal_reference.rego

@@ -5,9 +5,12 @@ package regal.rules.bugs["leaked-internal-reference"]
 import rego.v1
 
 import data.regal.ast
+import data.regal.config
 import data.regal.result
 
 report contains violation if {
+	_enabled(_valid_test_file_name(input.regal.file.name), _enable_for_test_files)
+
 	ref := ast.found.refs[_][_]
 
 	contains(ast.ref_to_string(ref.value), "._")
@@ -16,9 +19,28 @@ report contains violation if {
 }
 
 report contains violation if {
+	_enabled(_valid_test_file_name(input.regal.file.name), _enable_for_test_files)
+
 	some imported in input.imports
 
 	contains(ast.ref_to_string(imported.path.value), "._")
 
 	violation := result.fail(rego.metadata.chain(), result.ranged_from_ref(imported.path.value))
 }
+
+default _enabled(_, _) := true
+
+_enabled(true, false) := false
+
+default _valid_test_file_name(_) := false
+
+_valid_test_file_name(filename) if endswith(filename, "_test.rego")
+
+# Styra DAS convention considered OK
+_valid_test_file_name("test.rego")
+
+_cfg := config.for_rule("bugs", "leaked-internal-reference")
+
+default _enable_for_test_files := false
+
+_enable_for_test_files := object.get(_cfg, "include-test-files", false)

bundle/regal/rules/bugs/leaked-internal-reference/leaked_internal_reference_test.rego

@@ -68,6 +68,30 @@ test_fail_leaked_internal_reference_in_nested_comprehension if {
 	})
 }
 
+test_ignore_test_file_by_default if {
+	r := rule.report with input as ast.with_rego_v1(`foo := data.bar._wow`)
+		with input.regal.file.name as "p_test.rego"
+
+	r == set()
+}
+
+test_ignore_test_file_can_be_disabled if {
+	r := rule.report with input as ast.with_rego_v1(`foo := data.bar._wow`)
+		with input.regal.file.name as "p_test.rego"
+		with config.for_rule as {"include-test-files": true}
+
+	r == expected_with_location({
+		"file": "p_test.rego",
+		"col": 8,
+		"row": 5,
+		"end": {
+			"col": 21,
+			"row": 5,
+		},
+		"text": "foo := data.bar._wow",
+	})
+}
+
 expected := {
 	"category": "bugs",
 	"description": "Outside reference to internal rule or function",

docs/rules/bugs/leaked-internal-reference.md

@@ -5,6 +5,7 @@
 **Category**: Bugs
 
 **Avoid**
+
 ```rego
 package policy
 
@@ -44,6 +45,14 @@ Do note that if you disagree with this rule, you don't need to disable it unless
 something else. If you don't use underscore prefixes, nothing will be reported by this rule anyway. It does however
 mean that the benefits listed above won't apply to your project.
 
+## Exceptions
+
+This rule is not enabled by default for test files. In tests, it can be useful
+to reference internal rules and functions to achieve good test coverage, which
+would be a violation of this rule. If you want to run this rule for tests
+too, you can set `include-test-files: true` in the configuration for this rule
+in your Regal config file.
+
 ## Configuration Options
 
 This linter rule provides the following configuration options:
@@ -54,6 +63,7 @@ rules:
     leaked-internal-reference:
       # one of "error", "warning", "ignore"
       level: error
+      include-test-files: false # default is false
 ```
 
 ## Related Resources