fix: respect ref head rules in rule-name-repeats-package (#1022)

Also made some adjustments to make the rule follow more modern conventions, and more importantly, to report end locations. Fixes #1015 Signed-off-by: Anders Eknert <[email protected]>
StyraInc · Aug 29, 2024 · a92a31f · a92a31f
1 parent a9d4e2f
commit a92a31f
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 76 deletions.
diff --git a/bundle/regal/rules/style/rule_name_repeats_package.rego b/bundle/regal/rules/style/rule_name_repeats_package.rego
@@ -1,19 +1,30 @@
 # METADATA
 # description: Rule name repeats package
 # related_resources:
-# - description: documentation
-#   ref: https://docs.styra.com/regal/rules/style/rule-name-repeats-package
-# schemas:
-# - input: schema.regal.ast
+#   - description: documentation
+#     ref: https://docs.styra.com/regal/rules/style/rule-name-repeats-package
 package regal.rules.style["rule-name-repeats-package"]
 
 import rego.v1
 
+import data.regal.ast
 import data.regal.result
 
-titleize(str) := upper(str) if count(str) == 1
+# METADATA
+# description: reports any location where a rule name repeats the package name
+report contains violation if {
+	some rule in input.rules
+
+	name := ast.ref_static_to_string(rule.head.ref)
+
+	strings.any_prefix_match(name, _possible_offending_prefixes)
+
+	violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(rule.head.ref[0]))
+}
+
+_titleize(str) := upper(str) if count(str) == 1
 
-titleize(str) := result if {
+_titleize(str) := result if {
 	chrs := regex.split(``, str)
 	count(chrs) > 1
 
@@ -23,51 +34,41 @@ titleize(str) := result if {
 	)
 }
 
-package_path := input["package"].path
-
-package_path_components := [component.value |
-	some component in package_path
+_package_path_components := [component.value |
+	some component in input["package"].path
 	component.type == "string"
 ]
 
-num_package_path_components := count(package_path_components)
+_num_package_path_components := count(_package_path_components)
 
-possible_path_component_combinations contains combination if {
-	some end in numbers.range(1, num_package_path_components)
+_possible_path_component_combinations contains combination if {
+	some end in numbers.range(1, _num_package_path_components)
 
 	combination := array.slice(
-		package_path_components,
-		num_package_path_components - end,
-		num_package_path_components,
+		_package_path_components,
+		_num_package_path_components - end,
+		_num_package_path_components,
 	)
 }
 
-possible_offending_prefixes contains prefix if {
-	some combination in possible_path_component_combinations
+_possible_offending_prefixes contains prefix if {
+	some combination in _possible_path_component_combinations
 
 	prefix := concat("_", combination)
 }
 
-possible_offending_prefixes contains prefix if {
-	some combination in possible_path_component_combinations
+_possible_offending_prefixes contains prefix if {
+	some combination in _possible_path_component_combinations
 
 	count(combination) > 1
 
 	formatted_combination := array.concat(
 		[combination[0]],
 		[w |
 			some word in array.slice(combination, 1, count(combination))
-			w := titleize(word)
+			w := _titleize(word)
 		],
 	)
 
 	prefix := concat("", formatted_combination)
 }
-
-report contains violation if {
-	some rule in input.rules
-
-	strings.any_prefix_match(rule.head.name, possible_offending_prefixes)
-
-	violation := result.fail(rego.metadata.chain(), result.location(rule.head))
-}
diff --git a/bundle/regal/rules/style/rule_name_repeats_package_test.rego b/bundle/regal/rules/style/rule_name_repeats_package_test.rego
@@ -18,24 +18,6 @@ base_result := {
 	"related_resources": related_resources,
 }
 
-test_possible_offending_prefixes if {
-	module := regal.parse_module("example.rego", `
-    package policy.foo.bar
-
-    allow := true
-    `)
-
-	r := rule.possible_offending_prefixes with input as module
-
-	r == {
-		"policy_foo_bar",
-		"foo_bar",
-		"bar",
-		"policyFooBar",
-		"fooBar",
-	}
-}
-
 test_rule_empty_if_no_repetition if {
 	module := regal.parse_module("example.rego", `
     package policy.foo.bar
@@ -59,14 +41,21 @@ test_rule_violation_if_repetition if {
 
 	r == {object.union(
 		base_result,
-		{"location": {"col": 5, "file": "example.rego", "row": 4, "text": "    bar := true"}},
+		{"location": {
+			"row": 4,
+			"col": 5,
+			"end": {
+				"col": 8,
+				"row": 4,
+			},
+			"file": "example.rego",
+			"text": "    bar := true",
+		}},
 	)}
 }
 
 test_rule_violation_if_repetition_of_more_than_one_path_component if {
-	module := regal.parse_module("example.rego", `
-    package policy.foo.bar.baz
-
+	module := regal.parse_module("example.rego", `package policy.foo.bar.baz
     foo_bar_baz := true
 
     barBaz := 1
@@ -75,17 +64,24 @@ test_rule_violation_if_repetition_of_more_than_one_path_component if {
 	r := rule.report with input as module
 
 	r == {
-		object.union(
-			base_result,
-			{"location": {"col": 5, "file": "example.rego", "row": 4, "text": "    foo_bar_baz := true"}},
-		),
-		object.union(
-			base_result,
-			{"location": {"col": 5, "file": "example.rego", "row": 6, "text": "    barBaz := 1"}},
-		),
+		object.union(base_result, {"location": {
+			"row": 2,
+			"col": 5,
+			"end": {"col": 16, "row": 2},
+			"file": "example.rego",
+			"text": "    foo_bar_baz := true",
+		}}),
+		object.union(base_result, {"location": {
+			"row": 4,
+			"col": 5,
+			"end": {"col": 11, "row": 4},
+			"file": "example.rego",
+			"text": "    barBaz := 1",
+		}}),
 	}
 }
 
+# regal ignore:rule-length
 test_rule_violation_if_repetition_multiple if {
 	module := regal.parse_module("example.rego", `
     package policy.foo.bar
@@ -98,12 +94,27 @@ test_rule_violation_if_repetition_multiple if {
 	r := rule.report with input as module
 
 	r == {
-		object.union(base_result, {"location": {"col": 5, "file": "example.rego", "row": 4, "text": "    bar := true"}}),
-		object.union(base_result, {"location": {"col": 5, "file": "example.rego", "row": 5, "text": "    barNumber := 3"}}),
-		object.union(
-			base_result,
-			{"location": {"col": 5, "file": "example.rego", "row": 6, "text": "    barString := \"string\""}},
-		),
+		object.union(base_result, {"location": {
+			"col": 5,
+			"file": "example.rego",
+			"row": 4,
+			"end": {"col": 8, "row": 4},
+			"text": "    bar := true",
+		}}),
+		object.union(base_result, {"location": {
+			"col": 5,
+			"file": "example.rego",
+			"row": 5,
+			"end": {"col": 14, "row": 5},
+			"text": "    barNumber := 3",
+		}}),
+		object.union(base_result, {"location": {
+			"col": 5,
+			"file": "example.rego",
+			"row": 6,
+			"end": {"col": 14, "row": 6},
+			"text": "    barString := \"string\"",
+		}}),
 	}
 }
 
@@ -118,13 +129,18 @@ test_rule_violation_if_repetition_in_function if {
 
 	r == {object.union(
 		base_result,
-		{"location": {"col": 5, "file": "example.rego", "row": 4, "text": "    bar(_) := true"}},
+		{"location": {
+			"col": 5,
+			"file": "example.rego",
+			"row": 4,
+			"end": {"col": 8, "row": 4},
+			"text": "    bar(_) := true",
+		}},
 	)}
 }
 
 test_rule_violation_if_repetition_in_defaults if {
-	module := regal.parse_module("example.rego", `
-    package policy.foo.bar
+	module := regal.parse_module("example.rego", `package policy.foo.bar
 
     default bar(_) := true
     default barNumber := 3
@@ -133,13 +149,39 @@ test_rule_violation_if_repetition_in_defaults if {
 	r := rule.report with input as module
 
 	r == {
-		object.union(
-			base_result,
-			{"location": {"col": 13, "file": "example.rego", "row": 4, "text": "    default bar(_) := true"}},
-		),
-		object.union(
-			base_result,
-			{"location": {"col": 13, "file": "example.rego", "row": 5, "text": "    default barNumber := 3"}},
-		),
+		object.union(base_result, {"location": {
+			"col": 13,
+			"file": "example.rego",
+			"row": 3,
+			"end": {"col": 16, "row": 3},
+			"text": "    default bar(_) := true",
+		}}),
+		object.union(base_result, {"location": {
+			"col": 13,
+			"file": "example.rego",
+			"row": 4,
+			"end": {"col": 22, "row": 4},
+			"text": "    default barNumber := 3",
+		}}),
 	}
 }
+
+test_rule_violation_if_repetition_ref_head_rule if {
+	module := regal.parse_module("example.rego", `
+	package policy
+
+	import rego.v1
+
+	policy.decision contains "nope"
+	`)
+
+	r := rule.report with input as module
+
+	r == {object.union(base_result, {"location": {
+		"col": 2,
+		"file": "example.rego",
+		"row": 6,
+		"end": {"col": 8, "row": 6},
+		"text": "\tpolicy.decision contains \"nope\"",
+	}})}
+}