Chained rule body

[anderseknert]

From the OPA docs.

If the head of the rule is same, we can chain multiple rule bodies together to obtain the same result. We don’t recommend using this form anymore.

instances contains instance if {
    server := sites[_].servers[_]
    instance := {"address": server.hostname, "name": server.name}
} {
    container := containers[_]
    instance := {"address": container.ipaddress, "name": container.name}
}

I don't see this used very often, but it'd be good to add as a style category rule.

[anderseknert]

This isn't directly visible in the AST as it's currently provided, but would be helped by open-policy-agent/opa#6213, as we could then check the text attribute of the head's location, and see that it starts with {, i.e. that it goes straight to the body.

[srenatus]

This is also somewhat covered by the opa-fmt rule: opa fmt will not let this through. But you could have been disabling the opa-fmt rule for some reason, I suppose.

[anderseknert]

That is a good point! But yeah, our goal should be to not make rules depend on other rules. Like we're about to have one rule that forbids using print, and now soon another that warns on print + sprintf.

Funny thing is I just tried to lint a simple policy using this style, and it turns out it also renders a few false positives 😅

Linting

foo {
    true
} {
    false
}

Result

Rule:         	use-assignment-operator
Description:  	Prefer := over = for assignment
Category:     	style
Location:     	p.rego:5:3
Text:         	} {
Documentation:	https://docs.styra.com/regal/rules/style/use-assignment-operator

Rule:         	metasyntactic-variable
Description:  	Metasyntactic variable name
Category:     	testing
Location:     	p.rego:5:3
Text:         	} {
Documentation:	https://docs.styra.com/regal/rules/testing/metasyntactic-variable

The first case is a bug, obviously, as there is no =, but an implicit one. But contrary to when this happens in "normal" rule heads, the value now does come with a location attached 🙃 So we can't rely on that for these rules, and the text attribute would be helpful for that purpose too.

I guess in the second case, it does make sense to say that even the second rule is called "foo" — but it does look funny when you see the Text that comes with the violation 😆 I think flagging only the first occasion, where foo actually is printed, makes sense in that case.

We might get around not having the text attribute in the location for this rule though, as I think we can simply scan the input.regal.file.lines at the location provided and extract it from there. Looking 👀

[anderseknert]

This is also not recognized in strict mode — or rather, it is recognized, but this style isn't going to work for functions with args:

x(y) {
	input.foo
} {
	y
}

1 error occurred: policy.rego:3: rego_compile_error: unused argument y. (hint: use _ (wildcard variable) instead)

😄