Improved redundant-existence-check (#1897)

By looking in both directions. Still not exhaustive, but that's
way too expensive.. and most redundant checks are adjacent to
the place of use.

Fixes #1805

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

Author: anderseknert

bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check.rego

@@ -8,18 +8,13 @@ import data.regal.result
 # METADATA
 # description: check rule bodies for redundant existence checks
 report contains violation if {
-	some rule_index, rule in input.rules
-	some expr_index, expr in _exprs[rule_index]
+	some rule_index, expr_index
+	expr := _static_ref_exprs[rule_index][expr_index]
 
-	expr.terms.type == "ref"
-
-	not expr.with
-
-	not rule.body[expr_index + 1].negated
-	ast.static_ref(expr.terms)
-
-	some term in rule.body[expr_index + 1].terms
+	some adjacent in [-1, 1]
+	some term in _exprs[rule_index][expr_index + adjacent].terms
 
+	term.type == "ref"
 	ast.is_terms_subset(expr.terms.value, term.value)
 
 	violation := result.fail(rego.metadata.chain(), result.ranged_from_ref(expr.terms.value))
@@ -34,37 +29,51 @@ report contains violation if {
 #  quite unlikely that existence checks are found there
 report contains violation if {
 	some func in ast.functions
+
+	arg_vars := {term.value |
+		some term in func.head.args
+		term.type == "var"
+	}
+
 	some expr in func.body
 
 	not expr.negated
 	expr.terms.type == "var"
-
-	some arg in func.head.args
-
-	arg.type == "var"
-	arg.value == expr.terms.value
+	expr.terms.value in arg_vars
 
 	violation := result.fail(rego.metadata.chain(), result.location(expr.terms))
 }
 
 # METADATA
 # description: check for redundant existence checks in rule head assignment
 report contains violation if {
-	some rule_index, rule in input.rules
+	some rule_index
+	input.rules[rule_index].head.value.type == "ref"
 
-	rule.head.value.type == "ref"
+	head := input.rules[rule_index].head
 
 	some expr in _exprs[rule_index]
 
-	not expr.negated
 	expr.terms.type == "ref"
-	ast.is_terms_subset(expr.terms.value, rule.head.value.value)
+	ast.is_terms_subset(expr.terms.value, head.value.value)
 
 	violation := result.fail(rego.metadata.chain(), result.ranged_from_ref(expr.terms.value))
 }
 
 # all top-level expressions in module
 _exprs[rule_index][expr_index] := expr if {
-	some rule_index, rule in input.rules
-	some expr_index, expr in rule.body
+	some rule_index, expr_index
+	input.rules[rule_index].body[expr_index]
+
+	expr := input.rules[rule_index].body[expr_index]
+	not expr.with
+	not expr.negated
+}
+
+_static_ref_exprs[rule_index][expr_index] := expr if {
+	some rule_index, expr_index
+	_exprs[rule_index][expr_index].terms.type == "ref"
+
+	expr := _exprs[rule_index][expr_index]
+	ast.static_ref(expr.terms)
 }

bundle/regal/rules/bugs/redundant-existence-check/redundant_existence_check_test.rego

@@ -16,7 +16,16 @@ test_fail_redundant_existence_check if {
 		"category": "bugs",
 		"description": "Redundant existence check",
 		"level": "error",
-		"location": {"col": 3, "file": "policy.rego", "row": 7, "text": "\t\tinput.foo", "end": {"col": 12, "row": 7}},
+		"location": {
+			"col": 3,
+			"file": "policy.rego",
+			"row": 7,
+			"text": "\t\tinput.foo",
+			"end": {
+				"col": 12,
+				"row": 7,
+			},
+		},
 		"related_resources": [{
 			"description": "documentation",
 			"ref": config.docs.resolve_url("$baseUrl/$category/redundant-existence-check", "bugs"),
@@ -36,7 +45,16 @@ test_fail_redundant_existence_check_subset if {
 		"category": "bugs",
 		"description": "Redundant existence check",
 		"level": "error",
-		"location": {"col": 3, "file": "policy.rego", "row": 7, "text": "\t\tinput.foo", "end": {"col": 12, "row": 7}},
+		"location": {
+			"col": 3,
+			"file": "policy.rego",
+			"row": 7,
+			"text": "\t\tinput.foo",
+			"end": {
+				"col": 12,
+				"row": 7,
+			},
+		},
 		"related_resources": [{
 			"description": "documentation",
 			"ref": config.docs.resolve_url("$baseUrl/$category/redundant-existence-check", "bugs"),
@@ -75,7 +93,16 @@ test_fail_redundant_existence_check_head_assignment_of_ref if {
 		"category": "bugs",
 		"description": "Redundant existence check",
 		"level": "error",
-		"location": {"col": 3, "file": "policy.rego", "row": 7, "text": "\t\tinput.foo", "end": {"col": 12, "row": 7}},
+		"location": {
+			"col": 3,
+			"file": "policy.rego",
+			"row": 7,
+			"text": "\t\tinput.foo",
+			"end": {
+				"col": 12,
+				"row": 7,
+			},
+		},
 		"related_resources": [{
 			"description": "documentation",
 			"ref": config.docs.resolve_url("$baseUrl/$category/redundant-existence-check", "bugs"),
@@ -94,7 +121,45 @@ test_fail_redundant_existence_check_function_arg if {
 		"category": "bugs",
 		"description": "Redundant existence check",
 		"level": "error",
-		"location": {"col": 3, "end": {"col": 6, "row": 7}, "file": "policy.rego", "row": 7, "text": "\t\tfoo"},
+		"location": {
+			"col": 3,
+			"end": {
+				"col": 6,
+				"row": 7,
+			},
+			"file": "policy.rego",
+			"row": 7,
+			"text": "\t\tfoo",
+		},
+		"related_resources": [{
+			"description": "documentation",
+			"ref": config.docs.resolve_url("$baseUrl/$category/redundant-existence-check", "bugs"),
+		}],
+		"title": "redundant-existence-check",
+	}}
+}
+
+test_fail_redundant_existence_check_function_arg_reference_after_use if {
+	r := rule.report with input as ast.with_rego_v1(`
+	fun(foo) if {
+		foo.type == "object"
+		foo.type
+	}`)
+
+	r == {{
+		"category": "bugs",
+		"description": "Redundant existence check",
+		"level": "error",
+		"location": {
+			"col": 3,
+			"end": {
+				"col": 11,
+				"row": 8,
+			},
+			"file": "policy.rego",
+			"row": 8,
+			"text": "\t\tfoo.type",
+		},
 		"related_resources": [{
 			"description": "documentation",
 			"ref": config.docs.resolve_url("$baseUrl/$category/redundant-existence-check", "bugs"),