nix-community / nixpkgs-fmt

Nix code formatter for nixpkgs [maintainer=@zimbatm]
https://nix-community.github.io/nixpkgs-fmt/
Apache License 2.0
581 stars 34 forks source link

Confusing attset merging #310

Open layus opened 1 year ago

layus commented 1 year ago

Input

foo test {
a = 1;
} // {
b = 2;
c = 3;
}

Output

foo test
  {
    a = 1;
  } // {
  b = 2;
  c = 3;
}

Desired output

I think the following output makes more sense, because it matches the implicit precedence of the merge operator.

foo test {
  a = 1;
} // {
  b = 2;
  c = 3;
}

When parentheses are added according to precedence, nixpkgs-fmt gives a result similar to the expected output above.

(foo test {
  a = 1;
}) // {
  b = 2;
  c = 3;
}

Hence, I think the current output is confusing readers into the wrong precedence for //, and may trick them into parsing the expression incorrectly.


Now, I understand this issue is a bit tricky. In particular, when parentheses are added to perform the merge first (force higher precedence than default), nixpkgs-fmt gives a result quite similar to the one above. So parenthesizing the expression about anywhere changes the output. And I do not have a general rule to apply here yet, just this snippet.

foo test ({
  a = 1;
} // {
  b = 2;
  c = 3;
})

PS: The online formatter with "submit code sample" is so nice to use !