From 0c067f0cbdf355bf9cd84f45f81b70175303d72c Mon Sep 17 00:00:00 2001
From: Anders Eknert <anders@styra.com>
Date: Wed, 1 Nov 2023 10:54:58 +0100
Subject: [PATCH] `rule-length`: add `count-comments` configuration option

This is set to `false` by default, as we don't want to "punish"
comments in rules.

Fixes #432

Signed-off-by: Anders Eknert <anders@styra.com>
---
 bundle/regal/config/provided/data.yaml        |  1 +
 bundle/regal/rules/style/rule_length.rego     | 24 ++++++++++++++++++-
 .../regal/rules/style/rule_length_test.rego   | 22 +++++++++++++++++
 docs/rules/style/rule-length.md               |  3 +++
 e2e/testdata/violations/most_violations.rego  |  1 +
 5 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/bundle/regal/config/provided/data.yaml b/bundle/regal/config/provided/data.yaml
index 49992c25b..439d375be 100644
--- a/bundle/regal/config/provided/data.yaml
+++ b/bundle/regal/config/provided/data.yaml
@@ -93,6 +93,7 @@ rules:
       ignore-nesting-level: 2
       level: error
     rule-length:
+      count-comments: false
       except-empty-body: true
       level: error
       max-rule-length: 30
diff --git a/bundle/regal/rules/style/rule_length.rego b/bundle/regal/rules/style/rule_length.rego
index 5f91b8084..aaffbabcc 100644
--- a/bundle/regal/rules/style/rule_length.rego
+++ b/bundle/regal/rules/style/rule_length.rego
@@ -16,7 +16,7 @@ report contains violation if {
 	some rule in input.rules
 	lines := split(base64.decode(rule.location.text), "\n")
 
-	count(lines) > cfg["max-rule-length"]
+	line_count(cfg, rule, lines) > cfg["max-rule-length"]
 
 	not generated_body_exception(cfg, rule)
 
@@ -27,3 +27,25 @@ generated_body_exception(conf, rule) if {
 	conf["except-empty-body"] == true
 	ast.generated_body(rule)
 }
+
+line_count(cfg, _, lines) := count(lines) if cfg["count-comments"] == true
+
+line_count(cfg, rule, lines) := n if {
+	not cfg["count-comments"]
+
+	# Note that this assumes } on its own line
+	body_start := rule.location.row + 1
+	body_end := (body_start + count(lines)) - 3
+	body_total := (body_end - body_start) + 1
+
+	# This does not take into account comments that are
+	# on the same line as regular code
+	body_comments := sum([1 |
+		some comment in input.comments
+
+		comment.Location.row >= body_start
+		comment.Location.row <= body_end
+	])
+
+	n := body_total - body_comments
+}
diff --git a/bundle/regal/rules/style/rule_length_test.rego b/bundle/regal/rules/style/rule_length_test.rego
index 70e10cc15..45eae95d1 100644
--- a/bundle/regal/rules/style/rule_length_test.rego
+++ b/bundle/regal/rules/style/rule_length_test.rego
@@ -21,6 +21,7 @@ test_fail_rule_longer_than_configured_max_length if {
 	r := rule.report with input as module with config.for_rule as {
 		"level": "error",
 		"max-rule-length": 3,
+		"count-comments": true,
 	}
 	r == {{
 		"category": "style",
@@ -49,6 +50,26 @@ test_success_rule_not_longer_than_configured_max_length if {
 	r := rule.report with input as module with config.for_rule as {
 		"level": "error",
 		"max-rule-length": 30,
+		"count-comments": true,
+	}
+	r == set()
+}
+
+test_success_rule_longer_than_configured_max_length_but_comments if {
+	module := regal.parse_module("policy.rego", `package p
+
+	my_short_rule {
+		# this rule is not longer than the configured max length
+		# which in this case is 30 lines
+		#
+		input.x
+	}
+	`)
+
+	r := rule.report with input as module with config.for_rule as {
+		"level": "error",
+		"max-rule-length": 2,
+		"count-comments": false,
 	}
 	r == set()
 }
@@ -62,6 +83,7 @@ test_success_rule_length_equals_max_length if {
 	r := rule.report with input as module with config.for_rule as {
 		"level": "error",
 		"max-rule-length": 1,
+		"count-comments": false,
 	}
 	r == set()
 }
diff --git a/docs/rules/style/rule-length.md b/docs/rules/style/rule-length.md
index 95a916317..fdb0e3b96 100644
--- a/docs/rules/style/rule-length.md
+++ b/docs/rules/style/rule-length.md
@@ -32,6 +32,9 @@ rules:
       level: error
       # default limit is 30 lines
       max-rule-length: 30
+      # whether to count comments as lines
+      # by default, this is set to false
+      count-comments: false
       # except rules with empty bodies from this rule, as they're
       # likely an assignment of long values rather than a "rule"
       # with conditions:
diff --git a/e2e/testdata/violations/most_violations.rego b/e2e/testdata/violations/most_violations.rego
index 488b328fd..c93b51515 100644
--- a/e2e/testdata/violations/most_violations.rego
+++ b/e2e/testdata/violations/most_violations.rego
@@ -154,6 +154,7 @@ rule_length {
 	input.x28
 	input.x29
 	input.x30
+	input.x31
 }
 
 default_over_else := 1 {