nixfmt
nixfmt copied to clipboard
`f x.a or b` should be formatted to `f (x.a or b)`
Description
This is a ferociously confusing parse, as precedent from Nix and Haskell would have you expect that no operator binds tighter than function calls, and the spaced keyword especially makes it strongly look like (f x.a) or b.
Note that this also applies to e.g. [ x.a or b ] and even x.a or f b. I think the rule should be something like “any non‐parenthesized x.a or b expression that is a direct child of a function call or list should have parentheses added”.
See https://github.com/NixOS/nixpkgs/pull/339006#pullrequestreview-2276246040 for a real‐world example where this came up and could avoid future readability nit‐picking.
Small example input
{ a ? x.a or f b }: {
glurk = x.a or 1 + 2;
zorpy = f x.a or b;
yammo = [ x.a or b x.y or c (x.a or f x.y or b) ];
}
Expected output
{
a ? x.a or f b,
}:
{
glurk = x.a or 1 + 2;
zorpy = f (x.a or b);
yammo = [
(x.a or b)
(x.y or c)
((x.a or f) (x.y or b))
];
}
Actual output
{
a ? x.a or f b,
}:
{
glurk = x.a or 1 + 2;
zorpy = f x.a or b;
yammo = [
x.a or b
x.y or c
(x.a or f x.y or b)
];
}
Note that I think x.a or [ ] ++ b is also confusing, but just the regular kind of confusing that operator precedence tends to be, so I wouldn’t include that here. (Though again the or keyword adds to the confusion vs. a symbolic operator.)
Based on the discussion in #330, this should probably be closed as "won't do" as well.
@piegamesde is there an argument to be made that adding parens is fundamentally simpler than normalising or removing them?
Either way, I'm leaning towards closing just to be consistent with our stance on #330 and other examples of syntax transformations.
I'm happy to leave this unresolved until our next meeting, if preferred.
Adding is a lot simpler than removing. Making it look decent in all edge cases w.r.t. comments might still take some tweaking, but overall from a technical point of view this should be feasible in Nixfmt.
IMHO, this is language issue that should be fixed on the language level, and I do have ideas on how to eventually fix this on the language level (by emitting parser warnings for some of the weirdest uses of or). So until all other options (fix in language, fallback to implement in a linter) are exhausted, I'd prefer not implementing this in Nixfmt.