mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-13 03:42:35 -06:00
format: Don't group rules that aren't written on one line (#8981)
`opa fmt` isn't idempotent for rules it can't write on a single line:
the first pass runs the rule together with the one that follows, and
only a second pass inserts the missing blank line.
Given this policy:
```rego
package example
allow if { {"admin", "dev"} }
deny if input.blocked
```
`opa fmt` produces output that it doesn't consider formatted:
```rego
package example
allow if {
{"admin", "dev"}
}deny if input.blocked
```
```console
$ opa fmt --fail example.rego; echo $?
2
```
Running it a second time inserts the blank line and settles:
```rego
package example
allow if {
{"admin", "dev"}
}
deny if input.blocked
```
with this fix fmt goes to the second result immediately
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
This commit is contained in:
@@ -584,6 +584,10 @@ func (w *writer) writeRules(rules []*ast.Rule, comments []*ast.Comment) ([]*ast.
|
||||
return comments, nil
|
||||
}
|
||||
|
||||
// groupableOneLiner reports whether rule is written on a single line, and so may
|
||||
// be grouped with an adjacent rule instead of being followed by a blank line.
|
||||
// These conditions must agree with the inline body branch of writeRule, which
|
||||
// doesn't end the line after the closing brace of a multi-line body.
|
||||
func (w *writer) groupableOneLiner(rule *ast.Rule) bool {
|
||||
// Location required to determine if two rules are adjacent in the policy.
|
||||
// If not, we respect line breaks between rules.
|
||||
@@ -591,6 +595,18 @@ func (w *writer) groupableOneLiner(rule *ast.Rule) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// An else block is always written on a line of its own, so the rule spans
|
||||
// multiple lines even when its own body is written inline.
|
||||
if rule.Else != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// A lone set term body keeps its enclosing braces, and so is written as a
|
||||
// multi-line block.
|
||||
if len(rule.Body) == 1 && isSetTerm(rule.Body[0]) {
|
||||
return false
|
||||
}
|
||||
|
||||
partialSetException := w.fmtOpts.contains || rule.Head.Value != nil
|
||||
|
||||
return (w.fmtOpts.regoV1 || w.fmtOpts.ifs) && partialSetException
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
package test
|
||||
|
||||
import future.keywords.if
|
||||
|
||||
# A lone set term body keeps its enclosing braces, so these rules are written as
|
||||
# multi-line blocks and must be separated by a blank line from the rule that
|
||||
# follows them.
|
||||
set_body_then_one_liner if { {1, 2} }
|
||||
one_liner := 1 if input.x
|
||||
|
||||
set_body_then_set_body if { {1} }
|
||||
another_set_body if { {2} }
|
||||
|
||||
# An else block is written on a line of its own.
|
||||
else_rule := 1 if input.x else := 2
|
||||
after_else_rule := 3 if input.x
|
||||
|
||||
# Rules that really are written on a single line stay grouped.
|
||||
grouped_ref := 1 if input.x
|
||||
grouped_negated_set := 2 if not {1}
|
||||
@@ -0,0 +1,31 @@
|
||||
package test
|
||||
|
||||
import future.keywords.if
|
||||
|
||||
# A lone set term body keeps its enclosing braces, so these rules are written as
|
||||
# multi-line blocks and must be separated by a blank line from the rule that
|
||||
# follows them.
|
||||
set_body_then_one_liner if {
|
||||
{1, 2}
|
||||
}
|
||||
|
||||
one_liner := 1 if input.x
|
||||
|
||||
set_body_then_set_body if {
|
||||
{1}
|
||||
}
|
||||
|
||||
another_set_body if {
|
||||
{2}
|
||||
}
|
||||
|
||||
# An else block is written on a line of its own.
|
||||
else_rule := 1 if input.x
|
||||
|
||||
else := 2
|
||||
|
||||
after_else_rule := 3 if input.x
|
||||
|
||||
# Rules that really are written on a single line stay grouped.
|
||||
grouped_ref := 1 if input.x
|
||||
grouped_negated_set := 2 if not {1}
|
||||
@@ -0,0 +1,29 @@
|
||||
package test
|
||||
|
||||
# Adjacent rules are only grouped without a blank line when both are written on
|
||||
# a single line. A lone set term body keeps its enclosing braces, so the rules
|
||||
# below are written as multi-line blocks and must be separated by a blank line.
|
||||
set_body_then_one_liner if { {1, 2} }
|
||||
one_liner := 1 if input.x
|
||||
|
||||
one_liner_then_set_body := 1 if input.x
|
||||
set_body if { {1, 2} }
|
||||
|
||||
set_body_then_set_body if { {1} }
|
||||
another_set_body if { {2} }
|
||||
|
||||
empty_set_body if { set() }
|
||||
parenthesized_set_body if { ({1, 2}) }
|
||||
|
||||
partial contains 1 if { {1, 2} }
|
||||
partial contains 2 if input.x
|
||||
|
||||
# An else block is written on a line of its own, so the rule spans more than one
|
||||
# line even though its own body is written inline.
|
||||
else_rule := 1 if input.x else := 2
|
||||
after_else_rule := 3 if input.x
|
||||
|
||||
# Rules that really are written on a single line stay grouped.
|
||||
grouped_ref := 1 if input.x
|
||||
grouped_negated_set := 2 if not {1}
|
||||
grouped_negated_empty_set := 3 if not set()
|
||||
@@ -0,0 +1,51 @@
|
||||
package test
|
||||
|
||||
# Adjacent rules are only grouped without a blank line when both are written on
|
||||
# a single line. A lone set term body keeps its enclosing braces, so the rules
|
||||
# below are written as multi-line blocks and must be separated by a blank line.
|
||||
set_body_then_one_liner if {
|
||||
{1, 2}
|
||||
}
|
||||
|
||||
one_liner := 1 if input.x
|
||||
|
||||
one_liner_then_set_body := 1 if input.x
|
||||
|
||||
set_body if {
|
||||
{1, 2}
|
||||
}
|
||||
|
||||
set_body_then_set_body if {
|
||||
{1}
|
||||
}
|
||||
|
||||
another_set_body if {
|
||||
{2}
|
||||
}
|
||||
|
||||
empty_set_body if {
|
||||
set()
|
||||
}
|
||||
|
||||
parenthesized_set_body if {
|
||||
{1, 2}
|
||||
}
|
||||
|
||||
partial contains 1 if {
|
||||
{1, 2}
|
||||
}
|
||||
|
||||
partial contains 2 if input.x
|
||||
|
||||
# An else block is written on a line of its own, so the rule spans more than one
|
||||
# line even though its own body is written inline.
|
||||
else_rule := 1 if input.x
|
||||
|
||||
else := 2
|
||||
|
||||
after_else_rule := 3 if input.x
|
||||
|
||||
# Rules that really are written on a single line stay grouped.
|
||||
grouped_ref := 1 if input.x
|
||||
grouped_negated_set := 2 if not {1}
|
||||
grouped_negated_empty_set := 3 if not set()
|
||||
Reference in New Issue
Block a user