opa
opa copied to clipboard
opa fmt 2.0
The opa fmt
command is great! The current version however has not moved at the same pace as the rest of the project, and could benefit from a remake. Creating this ticket to collect ideas and feature requests for a possible next iteration of the opa fmt
command.
Some initial ideas from myself and previous tickets on the topic:
- Opinionated formatting to follow best practices. Examples of this includes using assignment
:=
and equality==
operators rather than unification=
where possible. Other possible ideas to explore could be reformatting old style iteration to usesome
andevery
, membership checks to usein
, etc... i.e. define best practice for a domain and have the formatter apply those where possible.
Original ticket: https://github.com/open-policy-agent/opa/issues/4076
- Allow formatting of snippets/statements, and not just entire modules—e.g.
$ echo 'xs := {1, 3,2,3,4}' | opa fmt xs := {1, 2, 3, 4}
Original ticket: https://github.com/open-policy-agent/opa/issues/4104
- Decide on formatting for multi-line expressions. This includes unions of sets, long chains of
with
statements in tests, etc. These currently don't look as good as when manually formatted, and could definitely be improved.
Original ticket: https://github.com/open-policy-agent/opa/issues/2958
- Better consideration of how comments are preserved or moved around.
Original ticket: https://github.com/open-policy-agent/opa/issues/1549
-
Take line length into account. This is especially painful when dealing with large objects such as mocks in tests, where the formatter sometimes reformats a map to be compressed into a single long line, in extreme cases several hundred characters long.
-
Allow a directive (like a comment) to have the formatter ignore a specific line or rule. This could be useful when manual formatting is preferred for e.g. a table of data where extra indents and whitespace helps with readability.
More suggestions are welcome!
@anderseknert not sure if this one is included in the list, but what about controlling the indentation (spaces vs tabs)
Thanks @oren-zohar!
Yeah I think controlling (i.e. configuration of) anything at all (indentation, max line length, style preferences) is a topic that definitely should be up for discussion. go fmt
famously does not allow that, and I think the opa fmt
command is largely modeled after that, but there are many other formatting tools that do allow configuration, so.. I don't know 🤷😄
One obvious benefit of keeping a single coherent model for formatting is of course that any policy formatted with the tool will pass the formatter checks regardless of where it came from. And maintenance of the tool will likely be more cumbersome if configuration is added to the mix. Perhaps a middle way could be to make the code modular enough that it's easy for anyone to simply extract it and build their own customizations on top of it.
Sorting and grouping imports!
import data.aws.iam.user.excluded_principal_name
import data.assertions.assert_not_in
import data.test_helpers.create_with_properties
import future.keywords
import data.aws.iam.user.deny
import data.assertions.assert_in
import data.test_helpers.with_properties
=>
import future.keywords
import data.aws.iam.user.deny
import data.aws.iam.user.excluded_principal_name
import data.assertions.assert_in
import data.assertions.assert_not_in
import data.test_helpers.create_with_properties
import data.test_helpers.with_properties
It would be nice to have both --fail
and --diff
at the same time. Right now, --fail
trumps --diff
.
Optimize long lines.
dropped_capabilities[affected] {
ctrs := containers[_]
affected = {"image": ctrs.image, "caps_dropped": {cap | cap := ctrs.securityContext.capabilities.drop[_]; count(cap) > 0}}
}
=>
dropped_capabilities[affected] {
ctrs := containers[_]
affected = {
"image": ctrs.image,
"caps_dropped": {cap |
cap := ctrs.securityContext.capabilities.drop[_]
count(cap) > 0
},
}
}
It would be nice to have both
--fail
and--diff
at the same time. Right now,--fail
trumps--diff
.
also --fail
and --list
please!
The formatting currently applied to else
clauses, where opa fmt
adds a body with an empty true
in it, feels like a low-hanging fruit to improve. I.e:
number := 10 {
input.foo
} else := 20
formats to ->
number := 10 {
input.foo
} else := 20 {
true
}
The body added to the else clause here is IMHO redundant, and does not help make the rule more readable.
EDIT: This was fixed in OPA v0.47.0 :)
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
Related to https://github.com/StyraInc/rego-style-guide/pull/19 merged today, might it make sense to automatically correct this? i.e.
full_name := name {
name := concat(", ", [input.first_name, input.last_name])
}
divide_by_ten(x) := y {
y := x / 10
}
Formats to:
full_name := concat(", ", [input.first_name, input.last_name])
divide_by_ten(x) := x / 10
@charlieegan3 it does make sense, although it's hard to say where the responsibilities of the formatter starts and ends. I could imagine this as a linter rule just as well. Well, if we had one, that is 😆
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
I'd like to add feature request for print width / line length feature. opa fmt
is formatting multi line string to a really long line.
Before (please ignore +
usage, that seems to be wrong)
After
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.