Skip to content

format: Add support for and/or logical expressions - #9006

Merged
johanfylling merged 4 commits into
open-policy-agent:mainfrom
johanfylling:and_or/formatter
Aug 13, 2026
Merged

format: Add support for and/or logical expressions#9006
johanfylling merged 4 commits into
open-policy-agent:mainfrom
johanfylling:and_or/formatter

Conversation

@johanfylling

Copy link
Copy Markdown
Contributor

Fixes: #8683

* fold and/or after a not-body operand
* reject brace-led and/or lhs operands
* report brace-led and/or rhs operands
* name whole operand in brace-led hints

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Fixes: open-policy-agent#8683
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Signed-off-by: Johan Fylling <johan.dev@fylling.se>

import future.keywords.and
import future.keywords.or

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p if {
        (input.a | input.b) or input.c
}

formats as:

p if {
      input.a | input.b or input.c
}

The parentheses are required here otherwise it parses as a set comprehension, an edge case only happens when (input.a | input.b) or input.c is alone in the body and not inline.

Recommended fix is to add isUnionExpr(expr) to logicalOperandNeedsParens

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh! These freakin' comprehensions 🤬. Nice catch! 👍

@johanfylling johanfylling Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 59dc118

The fix applies a bit broader than for just the and/or operators, as this was a more general issue.

…owing an opening brace (`{`) ambiguous with a comprehension

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@johanfylling
johanfylling merged commit ce44e07 into open-policy-agent:main Aug 13, 2026
43 checks passed
@github-actions

Copy link
Copy Markdown
Benchmark Comparison (`48bb309719d039dd725d0c4ba2614e784ec28596` vs `ce44e0705e1c278a1cd1ba79291c448c8c717ae6`)
benchmark delta
RewriteDynamics/100 -1.99%
RewriteDynamics/1000 -1.29%
GenerateLocalVar -3.09%
BuildEqIndex/10000 +6.49%
BuildNonsensicalIndex/1 -3.76%
ParseManyAnnotations -2.10%
CommentString -4.50%
ObjectGet/existing_int_number_key -0.39%
ObjectFind/5_50 +1.05%
ObjectConstruction/increasing_keys/5 -5.43%
RefString/var_term -5.23%
InterfaceToValueInt/non-interned_int_value +4.67%
EscapeTemplateStringStringPart/_*_100 -11.39%
EscapeTemplateStringStringPart/{_*_100 +11.68%
CountUnescapedLeftCurly/_*_100 -9.69%
CountUnescapedLeftCurly/{unes{caped_*_100 +6.18%
CountUnescapedLeftCurly/{{{{{{{{{{_*_100 +2.91%
TemplateStringToString -3.92%
TemplateStringToStringEscapeCurlies -5.11%
GenericVisitorWalkVsTypeVisitor/GenericVisitor +4.61%
InterningAccessValue/interned_value -3.85%
InterningAccessValue/object_value -5.08%
FromBuiltinNames/two_parts -51.09%
NoNodeTypeAllocatesOnAppend/ref_head_with_value +7.29%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/null -0.95%
NoASTTypeAllocatesOnAppendToBufferOfStringLength/object +2.92%

This comment was automatically generated by the benchmarks workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

and/or keywords: formatter

2 participants