Increase test coverage

bundle/regal/ast/ast.rego

@@ -104,11 +104,30 @@ rule_names contains ref_to_string(rule.head.ref) if some rule in rules
 
 # METADATA
 # description: |
-#   determine if var in ref (e.g. `x` in `input[x]`) is used as input or output
-is_output_var(rule, ref, location) if {
-	startswith(ref.value, "$")
+#   determine if var in var (e.g. `x` in `input[x]`) is used as input or output
+# scope: document
+is_output_var(rule, var) if {
+	# test the cheap and common case first, and 'else' only when it's not
+	startswith(var.value, "$")
 } else if {
-	not ref.value in (find_names_in_scope(rule, location) - find_some_decl_names_in_scope(rule, location))
+	not var.value in (rule_names | imported_identifiers) # regal ignore:external-reference
+
+	num_above := sum([1 |
+		some above in find_vars_in_local_scope(rule, var.location)
+		above.value == var.value
+	])
+	num_some := sum([1 |
+		some name in find_some_decl_names_in_scope(rule, var.location)
+		name == var.value
+	])
+
+	# only the first ref variable in scope can be an output! meaning that:
+	# allow if {
+	#     some x
+	#     input[x]    # <--- output
+	#     data.bar[x] # <--- input
+	# }
+	num_above - num_some == 0
 }
 
 # METADATA
@@ -195,10 +214,6 @@ static_ref(ref) if every t in array.slice(ref.value, 1, count(ref.value)) {
 	t.type != "var"
 }
 
-static_rule_ref(ref) if every t in array.slice(ref, 1, count(ref)) {
-	t.type != "var"
-}
-
 # METADATA
 # description: provides a set of names of all built-in functions called in the input policy
 builtin_functions_called contains name if {
@@ -257,25 +272,6 @@ implicit_boolean_assignment(rule) if {
 # or sometimes, like this...
 implicit_boolean_assignment(rule) if rule.head.value.location == rule.head.location
 
-# or like this...
-implicit_boolean_assignment(rule) if {
-	# This handles the *quite* special case of
-	# `a.b if true`, which is "rewritten" to `a.b = true` *and*  where a location is still added to the value
-	# see https://github.com/open-policy-agent/opa/issues/6184 for details
-	#
-	# Do note that technically, it is possible to write a rule where the `true` value actually is on column 1, i.e.
-	#
-	# a.b =
-	# true
-	# if true
-	#
-	# If you write Rego like that — you're not going to use Regal anyway, are you? ¯\_(ツ)_/¯
-	rule.head.value.type == "boolean"
-	rule.head.value.value == true
-
-	rule.head.value.location.col == 1
-}
-
 # METADATA
 # description: |
 #   object containing all available built-in and custom functions in the

bundle/regal/ast/ast_test.rego

@@ -283,3 +283,21 @@ test_all_refs if {
 
 	text_refs == {":=", "data.foo.bar", "data.foo.bax", "data.foo.baz"}
 }
+
+test_provided_capabilities_never_undefined if {
+	capabilities.provided == {} with data.internal as {}
+}
+
+test_function_calls if {
+	calls := ast.function_calls["0"] with input as ast.with_rego_v1(`
+	rule if {
+		x := 1
+		f(2)
+	}`)
+
+	{"assign", "f"} == {call.name | some call in calls}
+}
+
+test_implicit_boolean_assignment if {
+	ast.implicit_boolean_assignment(ast.with_rego_v1(`a.b if true`).rules[0])
+}

bundle/regal/main/main.rego

@@ -24,10 +24,7 @@ rules_to_run[category][title] if {
 	not config.excluded_file(category, title, input.regal.file.name)
 }
 
-notices contains notice if {
-	some category, title
-	some notice in grouped_notices[category][title]
-}
+notices contains grouped_notices[_][_][_]
 
 grouped_notices[category][title] contains notice if {
 	some category, title

bundle/regal/rules/bugs/inconsistent-args/inconsistent_args.rego

@@ -29,7 +29,7 @@ report contains violation if {
 	# "Partition" the args by their position
 	by_position := [s |
 		some i, _ in args_list[0]
-		s := [x | x := args_list[_][i]]
+		s := [item[i] | some item in args_list]
 	]
 
 	some position in by_position

bundle/regal/rules/bugs/unused-output-variable/unused_output_variable.rego

@@ -22,20 +22,20 @@ import data.regal.result
 #   some x in data.foo.bar
 #   ```
 report contains violation if {
-	some rule_index, name
-
-	var_refs := _ref_vars[rule_index][name]
+	some rule_index
+	var_refs := _ref_vars[rule_index][_]
 
 	count(var_refs) == 1
 
 	some var in var_refs
 
 	not _var_in_head(input.rules[to_number(rule_index)].head, var.value)
 	not _var_in_call(ast.function_calls, rule_index, var.value)
+	not _ref_base_vars[rule_index][var.value]
 
 	# this is by far the most expensive condition to check, so only do
 	# so when all other conditions apply
-	ast.is_output_var(input.rules[to_number(rule_index)], var, var.location)
+	ast.is_output_var(input.rules[to_number(rule_index)], var)
 
 	violation := result.fail(rego.metadata.chain(), result.ranged_location_from_text(var))
 }
@@ -47,6 +47,15 @@ _ref_vars[rule_index][var.value] contains var if {
 	not startswith(var.value, "$")
 }
 
+# "a" in "a[foo]", and not foo
+_ref_base_vars[rule_index][term.value] contains term if {
+	some rule_index
+	term := ast.found.refs[rule_index][_].value[0]
+
+	term.type == "var"
+	not startswith(term.value, "$")
+}
+
 _var_in_head(head, name) if head.value.value == name
 
 _var_in_head(head, name) if {
@@ -63,6 +72,12 @@ _var_in_head(head, name) if {
 	var.value == name
 }
 
+_var_in_head(head, name) if {
+	some i, var in head.ref
+	i > 0
+	var.value == name
+}
+
 _var_in_call(calls, rule_index, name) if _var_in_arg(calls[rule_index][_].args[_], name)
 
 _var_in_arg(arg, name) if {
@@ -71,16 +86,7 @@ _var_in_arg(arg, name) if {
 }
 
 _var_in_arg(arg, name) if {
-	arg.type == "ref"
-
-	some val in arg.value
-
-	val.type == "var"
-	val.value == name
-}
-
-_var_in_arg(arg, name) if {
-	arg.type in {"array", "object"}
+	arg.type in {"array", "object", "set"}
 
 	some var in ast.find_term_vars(arg)
 

bundle/regal/rules/bugs/unused-output-variable/unused_output_variable_test.rego

@@ -72,6 +72,28 @@ test_success_not_unused_variable_in_head_value if {
 	r == set()
 }
 
+test_success_not_unused_variable_in_head_term_value if {
+	module := ast.with_rego_v1(`
+	success := {x} if {
+		input[x]
+	}
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
+test_success_not_unused_variable_in_head_term_key if {
+	module := ast.with_rego_v1(`
+	success contains {x} if {
+		input[x]
+	}
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
 test_success_not_unused_variable_in_head_key if {
 	module := ast.with_rego_v1(`
 	success contains x if {
@@ -96,6 +118,19 @@ test_success_not_unused_output_variable_function_call if {
 	r == set()
 }
 
+test_success_not_unused_output_variable_function_call_arg_term if {
+	module := ast.with_rego_v1(`
+	success if {
+		some x
+		input[x]
+		f({x})
+	}
+	`)
+
+	r := rule.report with input as module
+	r == set()
+}
+
 test_success_not_unused_output_variable_other_ref if {
 	module := ast.with_rego_v1(`
 	success if {

bundle/regal/rules/idiomatic/use-strings-count/use_strings_count.rego

@@ -12,7 +12,7 @@ import data.regal.result
 # description: Missing capability for built-in function `strings.count`
 # custom:
 #   severity: warning
-notices contains result.notice(rego.metadata.chain()) if not capabilities.has_object_keys
+notices contains result.notice(rego.metadata.chain()) if not capabilities.has_strings_count
 
 # METADATA
 # description: flag calls to `count` where the first argument is a call to `indexof_n`

bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports.rego

@@ -54,30 +54,27 @@ aggregate_report contains violation if {
 	}
 
 	some imp in all_imports
-	resolves(imp.path, all_package_paths)
+	_resolves(imp.path, all_package_paths)
 	not imp.path in all_package_paths
-	not imp.path in ignored_import_paths
+	not imp.path in _ignored_import_paths
 
 	violation := result.fail(rego.metadata.chain(), {"location": imp.location})
 }
 
-# METADATA
-# description: |
-#   returns true if the path "resolves" to *any*
-#   package part of the same length as the path
-resolves(path, pkg_paths) if count([path |
+# returns true if the path "resolves" to *any* package part of the same length as the path
+_resolves(path, pkg_paths) if count([path |
 	some pkg_path in pkg_paths
 	pkg_path == array.slice(path, 0, count(pkg_path))
 ]) > 0
 
-ignored_import_paths contains path if {
+_ignored_import_paths contains path if {
 	some item in cfg["ignore-import-paths"]
 	path := [part |
 		some i, p in split(item, ".")
-		part := normalize_part(i, p)
+		part := _normalize_part(i, p)
 	]
 }
 
-normalize_part(0, part) := part if part != "data"
+_normalize_part(0, part) := part if part != "data"
 
-normalize_part(i, part) := part if i > 0
+_normalize_part(i, part) := part if i > 0

bundle/regal/rules/imports/prefer-package-imports/prefer_package_imports_test.rego

@@ -98,14 +98,23 @@ test_success_aggregate_report_on_import_with_unresolved_path if {
 }
 
 test_success_aggregate_report_ignored_import_path if {
-	aggregate := {{"aggregate_data": {
-		"package_path": ["a"],
-		"imports": [{"path": ["b"], "location": {"col": 1, "file": "policy.rego", "row": 3, "text": "import data.b"}}],
-	}}}
+	aggregate := {
+		{"aggregate_data": {
+			"package_path": ["a"],
+			"imports": [{
+				"path": ["b", "c"],
+				"location": {"col": 1, "file": "policy.rego", "row": 3, "text": "import data.b.c"},
+			}],
+		}},
+		{"aggregate_data": {
+			"package_path": ["b"],
+			"imports": [],
+		}},
+	}
 
 	r := rule.aggregate_report with input.aggregate as aggregate with config.for_rule as {
 		"level": "error",
-		"ignore-import-paths": ["data.b"],
+		"ignore-import-paths": ["data.b.c"],
 	}
 
 	r == set()

bundle/regal/rules/style/default-over-not/default_over_not.rego

@@ -46,6 +46,10 @@ _static_rule_name(rule) := concat(".", array.concat([rule.head.ref[0].value], [r
 	some i, ref in rule.head.ref
 	i > 0
 ])) if {
-	count(rule.head.ref) > 1
-	ast.static_rule_ref(rule.head.ref)
+	c := count(rule.head.ref)
+	c > 1
+
+	every t in array.slice(rule.head.ref, 1, c) {
+		t.type != "var"
+	}
 }

bundle/regal/rules/style/prefer-some-in-iteration/prefer_some_in_iteration.rego

@@ -29,7 +29,7 @@ report contains violation if {
 
 		# we don't need the location of each var, but using the first
 		# ref will do, and will hopefully help with caching the result
-		ast.is_output_var(rule, var, value.location)
+		ast.is_output_var(rule, var)
 	])
 
 	num_output_vars != 0

bundle/regal/rules/style/rule-length/rule_length_test.rego

@@ -73,6 +73,25 @@ test_success_rule_longer_than_configured_max_length_but_comments if {
 	r == set()
 }
 
+test_success_rule_longer_than_configured_max_length_but_no_body_and_exception_configured if {
+	module := regal.parse_module("policy.rego", `package p
+
+	my_short_rule := {
+		"a": input.a,
+		"b": input.b,
+		"c": input.c,
+		"d": input.d,
+	}
+	`)
+
+	r := rule.report with input as module with config.for_rule as {
+		"level": "error",
+		"max-rule-length": 2,
+		"except-empty-body": true,
+	}
+	r == set()
+}
+
 test_success_rule_length_equals_max_length if {
 	module := regal.parse_module("policy.rego", `package p
 

bundle/regal/rules/style/unnecessary-some/unnecessary_some.rego

@@ -11,8 +11,7 @@ report contains violation if {
 	# No need to traverse rules here if we're not importing `in`
 	ast.imports_keyword(input.imports, "in")
 
-	some rule_index
-	symbols := ast.found.symbols[rule_index][_]
+	symbols := ast.found.symbols[_][_]
 
 	symbols[0].type == "call"
 	symbols[0].value[0].type == "ref"