Some fixes for detecting external refs (#723)

The following cases are now covered:

```rego
f(_) := r

f(_) := {"r": r}
```
And variations thereof.

Fixes #719

Although it doesn't fix the `func_2` example there, this is about as
much we can do without some major effort.. so I'm not sure it's worth
keeping that issue open as I doubt we'll get there anytime soon.

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

README.md

@@ -242,7 +242,7 @@ The following rules are currently available:
 | style       | [default-over-not](https://docs.styra.com/regal/rules/style/default-over-not)                         | Prefer default assignment over negated condition          |
 | style       | [detached-metadata](https://docs.styra.com/regal/rules/style/detached-metadata)                       | Detached metadata annotation                              |
 | style       | [double-negative](https://docs.styra.com/regal/rules/style/double-negative)                           | Avoid double negatives                                    |
-| style       | [external-reference](https://docs.styra.com/regal/rules/style/external-reference)                     | Reference to input, data or rule ref in function body     |
+| style       | [external-reference](https://docs.styra.com/regal/rules/style/external-reference)                     | External reference in function                            |
 | style       | [file-length](https://docs.styra.com/regal/rules/style/file-length)                                   | Max file length exceeded                                  |
 | style       | [function-arg-return](https://docs.styra.com/regal/rules/style/function-arg-return)                   | Function argument used for return value                   |
 | style       | [line-length](https://docs.styra.com/regal/rules/style/line-length)                                   | Line too long                                             |

bundle/regal/ast/ast.rego

@@ -221,6 +221,17 @@ _find_vars(path, value, last) := _find_assign_vars(path, value) if {
 	value[0].value[0].value == "assign"
 }
 
+# `=` isn't necessarily assignment, and only considering the variable on the
+# left-hand side is equally dubious, but we'll treat `x = 1` as `x := 1` for
+# the purpose of this function until we have a more robust way of dealing with
+# unification
+_find_vars(path, value, last) := _find_assign_vars(path, value) if {
+	last == "terms"
+	value[0].type == "ref"
+	value[0].value[0].type == "var"
+	value[0].value[0].value == "eq"
+}
+
 _find_vars(_, value, _) := find_ref_vars(value) if value.type == "ref"
 
 _find_vars(path, value, last) := _find_some_in_decl_vars(path, value) if {
@@ -390,6 +401,7 @@ builtin_functions_called contains name if {
 # description: |
 #   Returns custom functions declared in input policy in the same format as builtin capabilities
 function_decls(rules) := {rule_name: decl |
+	# regal ignore:external-reference
 	some rule in functions
 
 	rule_name := name(rule)

bundle/regal/result.rego

@@ -162,8 +162,8 @@ resource_urls(related_resources, category) := [r |
 with_text(location) := {"location": object.union(
 	location,
 	{
-		"file": input.regal.file.name,
-		"text": input.regal.file.lines[location.row - 1],
+		"file": input.regal.file.name, # regal ignore:external-reference
+		"text": input.regal.file.lines[location.row - 1], # regal ignore:external-reference
 	},
 )} if {
 	location.row

bundle/regal/rules/bugs/impossible_not_test.rego

@@ -161,4 +161,5 @@ expected := {
 	"title": "impossible-not",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)

bundle/regal/rules/bugs/inconsistent_args_test.rego

@@ -73,8 +73,10 @@ expected := {
 	"title": "inconsistent-args",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": loc}) |
 	some loc in location
 } if {

bundle/regal/rules/custom/one_liner_rule_test.rego

@@ -160,4 +160,5 @@ expected := {
 	"title": "one-liner-rule",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})}

bundle/regal/rules/custom/prefer_value_in_head_test.rego

@@ -102,4 +102,5 @@ expected := {
 	"title": "prefer-value-in-head",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})}

bundle/regal/rules/idiomatic/equals_pattern_matching_test.rego

@@ -80,8 +80,10 @@ expected := {
 	"title": "equals-pattern-matching",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": loc}) |
 	some loc in location
 } if {

bundle/regal/rules/idiomatic/use_in_operator_test.rego

@@ -160,8 +160,10 @@ expected := {
 	"title": "use-in-operator",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": loc}) |
 	some loc in location
 } if {

bundle/regal/rules/style/external_reference.rego

@@ -1,5 +1,5 @@
 # METADATA
-# description: Reference to input, data or rule ref in function body
+# description: External reference in function
 package regal.rules.style["external-reference"]
 
 import rego.v1
@@ -13,10 +13,11 @@ report contains violation if {
 	some fn in ast.functions
 
 	named_args := {arg.value | some arg in fn.head.args; arg.type == "var"}
-	head_vars := {v.value | some v in ast.find_term_vars(fn.head.value)}
+
+	head_vars := {v.value | some v in ast.find_vars(fn.head.value)}
 	body_vars := {v.value | some v in ast.find_vars(fn.body)}
 	else_vars := {v.value | some v in ast.find_vars(fn["else"])}
-	own_vars := (head_vars | body_vars) | else_vars
+	own_vars := (body_vars | head_vars) | else_vars
 
 	# note: parens added by opa fmt 🤦
 	allowed_refs := (named_args | own_vars) | fn_namespaces

bundle/regal/rules/style/external_reference_test.rego

@@ -10,51 +10,21 @@ import data.regal.rules.style["external-reference"] as rule
 test_fail_function_references_input if {
 	r := rule.report with input as ast.policy(`f(_) { input.foo }`)
 		with data.internal.combined_config as {"capabilities": capabilities.provided}
-	r == {{
-		"category": "style",
-		"description": "Reference to input, data or rule ref in function body",
-		"location": {"col": 8, "file": "policy.rego", "row": 3, "text": `f(_) { input.foo }`},
-		"related_resources": [{
-			"description": "documentation",
-			"ref": config.docs.resolve_url("$baseUrl/$category/external-reference", "style"),
-		}],
-		"title": "external-reference",
-		"level": "error",
-	}}
+	r == expected_with_location({"col": 8, "file": "policy.rego", "row": 3, "text": `f(_) { input.foo }`})
 }
 
 test_fail_function_references_data if {
 	r := rule.report with input as ast.policy(`f(_) { data.foo }`)
 		with data.internal.combined_config as {"capabilities": capabilities.provided}
-	r == {{
-		"category": "style",
-		"description": "Reference to input, data or rule ref in function body",
-		"related_resources": [{
-			"description": "documentation",
-			"ref": config.docs.resolve_url("$baseUrl/$category/external-reference", "style"),
-		}],
-		"title": "external-reference",
-		"location": {"col": 8, "file": "policy.rego", "row": 3, "text": `f(_) { data.foo }`},
-		"level": "error",
-	}}
+	r == expected_with_location({"col": 8, "file": "policy.rego", "row": 3, "text": `f(_) { data.foo }`})
 }
 
 test_fail_function_references_data_in_expr if {
 	r := rule.report with input as ast.policy(`f(x) {
 		x == data.foo
 	}`)
 		with data.internal.combined_config as {"capabilities": capabilities.provided}
-	r == {{
-		"category": "style",
-		"description": "Reference to input, data or rule ref in function body",
-		"related_resources": [{
-			"description": "documentation",
-			"ref": config.docs.resolve_url("$baseUrl/$category/external-reference", "style"),
-		}],
-		"title": "external-reference",
-		"location": {"col": 8, "file": "policy.rego", "row": 4, "text": "\t\tx == data.foo"},
-		"level": "error",
-	}}
+	r == expected_with_location({"col": 8, "file": "policy.rego", "row": 4, "text": "\t\tx == data.foo"})
 }
 
 test_fail_function_references_rule if {
@@ -67,17 +37,19 @@ f(x, y) {
 }
 	`)
 		with data.internal.combined_config as {"capabilities": capabilities.provided}
-	r == {{
-		"category": "style",
-		"description": "Reference to input, data or rule ref in function body",
-		"location": {"col": 7, "file": "policy.rego", "row": 8, "text": `	y == foo`},
-		"related_resources": [{
-			"description": "documentation",
-			"ref": config.docs.resolve_url("$baseUrl/$category/external-reference", "style"),
-		}],
-		"title": "external-reference",
-		"level": "error",
-	}}
+	r == expected_with_location({"col": 7, "file": "policy.rego", "row": 8, "text": `	y == foo`})
+}
+
+test_fail_external_reference_in_head_assignment if {
+	r := rule.report with input as ast.policy(`f(_) := r`)
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == expected_with_location({"col": 9, "file": "policy.rego", "row": 3, "text": "f(_) := r"})
+}
+
+test_fail_external_reference_in_head_terms if {
+	r := rule.report with input as ast.policy(`f(_) := {"r": r}`)
+		with data.internal.combined_config as {"capabilities": capabilities.provided}
+	r == expected_with_location({"col": 15, "file": "policy.rego", "row": 3, "text": "f(_) := {\"r\": r}"})
 }
 
 test_success_function_references_no_input_or_data if {
@@ -133,3 +105,17 @@ test_success_function_references_external_function_in_expr if {
 		with data.internal.combined_config as {"capabilities": capabilities.provided}
 	r == set()
 }
+
+expected := {
+	"category": "style",
+	"description": "External reference in function",
+	"related_resources": [{
+		"description": "documentation",
+		"ref": config.docs.resolve_url("$baseUrl/$category/external-reference", "style"),
+	}],
+	"title": "external-reference",
+	"level": "error",
+}
+
+# regal ignore:external-reference
+expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)

bundle/regal/rules/style/messy_rule_test.rego

@@ -96,4 +96,5 @@ expected := {
 	"title": "messy-rule",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)

bundle/regal/rules/style/yoda_condition_test.rego

@@ -74,8 +74,10 @@ expected := {
 	"title": "yoda-condition",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": location})} if is_object(location)
 
+# regal ignore:external-reference
 expected_with_location(location) := {object.union(expected, {"location": loc}) |
 	some loc in location
 } if {

bundle/regal/rules/testing/metasyntactic_variable_test.rego

@@ -49,4 +49,5 @@ expected := {
 	"title": "metasyntactic-variable",
 }
 
+# regal ignore:external-reference
 expected_with_location(location) := object.union(expected, {"location": location})