mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
format: Keep rule body inline when the head spans multiple lines (#8904)
### Why the changes in this PR are needed? Fixes #8894. `opa fmt` expands a one-line `if` condition into a block whenever the rule head's *value* expression spans multiple lines, even though the condition itself is a single simple term. For example: ```rego foo := sprintf( "%d", [1], ) if allow ``` was reformatted to: ```rego foo := sprintf( "%d", [1], ) if { allow } ``` ### What are the changes in this PR? The inline-`if` path in `writeRule` decides whether to keep `if <term>` on one line by comparing the body term's row to the rule head's row: ```go if rule.Body[0].Location.Row == rule.Head.Location.Row { ``` `rule.Head.Location.Row` is the head's **start** row. Once the head value wraps onto later lines, the single body term sits on a later row than the head start, the equality fails, and formatting falls through to the block form. The fix compares against the head's **end** row instead (start row plus the number of newlines in the head's location text), so a single body term on the same line as `if` stays inline regardless of how many lines the head value occupies. Single-line heads are unaffected (end row == start row), and genuinely multi-statement bodies still expand as before (they don't hit the `len(rule.Body) == 1` branch). ### Notes to assist PR review: Added `v1/format/testfiles/v1/test_issue_8894.rego` (+`.formatted`) with the exact repro from the issue; it fails on `master` (expands to a block) and passes with this change. The rest of the format golden suite is unchanged. Signed-off-by: Kunalbehbud <b.kunal2002@gmail.com>
This commit is contained in:
+6
-1
@@ -640,7 +640,12 @@ func (w *writer) writeRule(rule *ast.Rule, isElse bool, comments []*ast.Comment)
|
||||
if (w.fmtOpts.regoV1 || w.fmtOpts.ifs) && partialSetException {
|
||||
w.write(" if")
|
||||
if len(rule.Body) == 1 {
|
||||
if rule.Body[0].Location.Row == rule.Head.Location.Row {
|
||||
// Keep `if <term>` on one line when the single body term sits on the
|
||||
// same line as the end of the head. Comparing against the head's
|
||||
// start row would wrongly expand the condition into a block whenever
|
||||
// the head value spans multiple lines (e.g. a multi-line call).
|
||||
headEndRow := rule.Head.Location.Row + strings.Count(string(rule.Head.Location.Text), "\n")
|
||||
if rule.Body[0].Location.Row == headEndRow {
|
||||
w.write(" ")
|
||||
var err error
|
||||
comments, err = w.writeExpr(rule.Body[0], comments)
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
package test
|
||||
|
||||
foo := sprintf(
|
||||
"%d",
|
||||
[1],
|
||||
) if allow
|
||||
@@ -0,0 +1,6 @@
|
||||
package test
|
||||
|
||||
foo := sprintf(
|
||||
"%d",
|
||||
[1],
|
||||
) if allow
|
||||
Reference in New Issue
Block a user