From d36655f5fb7b97c00746183addc03fff4010f23d Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Mon, 10 Aug 2026 10:08:45 -0500 Subject: [PATCH] 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 --- v1/format/format.go | 16 ++++++ .../testfiles/v0/test_rule_grouping.rego | 20 ++++++++ .../v0/test_rule_grouping.rego.formatted | 31 +++++++++++ .../testfiles/v1/test_rule_grouping.rego | 29 +++++++++++ .../v1/test_rule_grouping.rego.formatted | 51 +++++++++++++++++++ 5 files changed, 147 insertions(+) create mode 100644 v1/format/testfiles/v0/test_rule_grouping.rego create mode 100644 v1/format/testfiles/v0/test_rule_grouping.rego.formatted create mode 100644 v1/format/testfiles/v1/test_rule_grouping.rego create mode 100644 v1/format/testfiles/v1/test_rule_grouping.rego.formatted diff --git a/v1/format/format.go b/v1/format/format.go index 4e4935672a..5ef4603bbc 100644 --- a/v1/format/format.go +++ b/v1/format/format.go @@ -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 diff --git a/v1/format/testfiles/v0/test_rule_grouping.rego b/v1/format/testfiles/v0/test_rule_grouping.rego new file mode 100644 index 0000000000..40c694f400 --- /dev/null +++ b/v1/format/testfiles/v0/test_rule_grouping.rego @@ -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} diff --git a/v1/format/testfiles/v0/test_rule_grouping.rego.formatted b/v1/format/testfiles/v0/test_rule_grouping.rego.formatted new file mode 100644 index 0000000000..21645319b9 --- /dev/null +++ b/v1/format/testfiles/v0/test_rule_grouping.rego.formatted @@ -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} diff --git a/v1/format/testfiles/v1/test_rule_grouping.rego b/v1/format/testfiles/v1/test_rule_grouping.rego new file mode 100644 index 0000000000..a846d75ba7 --- /dev/null +++ b/v1/format/testfiles/v1/test_rule_grouping.rego @@ -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() diff --git a/v1/format/testfiles/v1/test_rule_grouping.rego.formatted b/v1/format/testfiles/v1/test_rule_grouping.rego.formatted new file mode 100644 index 0000000000..e3ae3703cc --- /dev/null +++ b/v1/format/testfiles/v1/test_rule_grouping.rego.formatted @@ -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()