Define "small" heuristic to improve multi-line expansion

Open trueNAHO opened this issue 1 week ago • 1 comments

Description

To avoid opinionated formatting nitpick reviews, Stylix removes artistic code formatting by enforcing nixfmt --strict. IMHO artistic formatting (not using --strict equivalents) is great for projects with few contributors, but suboptimal when managing countless contributors and reviewers. Specifically, projects like Nixpkgs might also benefit from this.

That being said, this proposal is not about whether to enforce --strict in Nixpkgs, but whether the following proposal should only be applied with --strict.

This proposal is about defining Rust's "small" equivalent for Nixfmt:

small items

In many places in this guide we specify formatting that depends on a code construct being small. For example, single-line vs multi-line struct literals:
// Normal formatting
Foo {
    f1: an_expression,
    f2: another_expression(),
}

// "small" formatting
Foo { f1, f2 }
We leave it to individual tools to decide on exactly what small means. In particular, tools are free to use different definitions in different circumstances.

Some suitable heuristics are the size of the item (in characters) or the complexity of an item (for example, that all components must be simple names, not more complex sub-expressions). For more discussion on suitable heuristics, see this issue.

-- github:rust-lang/rust, /src/doc/style-guide/src/README.md:321

Specifically, more eager multi-line expansion reduces merge conflicts, which would be invaluable for projects with as much as traffic as Nixpkgs:

And on a more technical side: I absolutely detest the mindless and
completely crazy Rust format checking.

I noticed that people added multiple

  use crate::xyz;

next to each other, so I turned them into

  use crate::{
     xyz,
    abc,
  };

instead to make it easy to just add another crate without messing crap
up. The use statements around it had that format too, so it all seemed
sensible and visually consistent.

But then I run rustfmtcheck, and that thing is all bass-ackwards
garbage. Instead of making it clean and clear to add new rules, it
suggests

  use crate::{xyz, abc};

but I have no idea what the heuristics for when to use multiple lines
and when to use that compressed format are.

This is just ANNOYING. It's automated tooling that is literally making
bad decisions for the maintainability. This is the kind of thing that
makes future conflicts harder for me to deal with.

Miguel, I know you asked me to run rustfmtcheck, but that thing is
just WRONG. It may be right "in the moment", but it is

 (a) really annoying when merging and not knowing what the heck the rules are

 (b) it's bad long term when you don't have clean lists of "add one
line for a new use"

Is there some *sane* solution to this? Because I left my resolution
alone and ignored the horrible rustfmtcheck results.

I tried to google the rust format rules, and apparently it's this:

    https://doc.rust-lang.org/style-guide/index.html#small-items

can we please fix up whatever random heuristics? That small items
thing may make sense when we're talking things that really are one
common data structure, but the "use" directive is literally about
*independent* things that get used, and smushing them all together
seems entirely wrong.

I realize that a number of users seem to just leave the repeated

   use kernel::xyz;
   use kernel::abc;

as separate lines, possibly *becasue* of this horrendous rustfmt
random heuristic behavior.

-- Linus Torvalds, LKML, "Re: [git pull] drm for 6.18-rc1", 2025-10-02 19:53

> The main complaint with rustfmt is that it is extremely twitchy and
> unstable with respect to one-line, vs. multi-line output.
>
> *Especially* with "use" statements.

The reason I'd like to fix the rules for "use" statements in
particular is that they do get a rather high rate of conflicts, and
then the "multiple entries per line" is actually very annoying
(because the merge turns into a "figure out small change within a
line" rather than "one line from side A, one line from side B").

And that's not because "use" lines are bad - it's actually pretty
natural, and is very similar to what we see with #include lines in C
files. Those too get much higher rate of conflicts than normal code,
and it simply isn't a problem: the conflicts are trivial to resolve.

Because unlike normal code where different people typically work on
different functions etc, the header includes - and for Rust, the "use"
lines - are kind of that shared area where everybody who makes a
change does so in the same place.

So conflicts in that area are normal and expected, and not generally a
sign of any problem.

But then that "small-items" rule makes for extra pain in this area.

Is it a _huge_ pain? No. But it's an unnecessary annoyance, I feel.

IOW, I really think "use" is fundamentally somewhat different from the
other Rust cases.

-- Linus Torvalds, LKML, "Re: [git pull] drm for 6.18-rc1", 2025-10-04 2:17

As mentioned by Linus, eager multi-line expansion should not generally extend to everything because some constructs inherently do not change frequently. The following examples demonstrate arguably overkill multi-line expansions:

```
-1 + 2 + 3
+1 +
+2 +
+3
```

-if true then null else null
+if true then
+    null
+else
+    null

I assume that the Nix equivalent for importing would be one of the following, although none of them are native Nix syntax, similar to Rust's use keyword:

let
  # This is a regular 'inherit (<NAMESPACE>) <IDENTIFIERS>;' construct.
  inherit (lib)
    one
    two
    three;
in

# This is a regular list.
imports = [
  "one"
  "two"
  "three"
];

For reference, the following documentation instances cover multi-line expansion:

https://github.com/NixOS/nixfmt/blob/42e43d9fcabadf57fdcefa6da355cd8dcf5b7d36/standard.md?plain=1#L68
https://github.com/NixOS/nixfmt/blob/42e43d9fcabadf57fdcefa6da355cd8dcf5b7d36/standard.md?plain=1#L297-L299
https://github.com/NixOS/nixfmt/blob/42e43d9fcabadf57fdcefa6da355cd8dcf5b7d36/standard.md?plain=1#L172-L177
https://github.com/NixOS/nixfmt/blob/42e43d9fcabadf57fdcefa6da355cd8dcf5b7d36/standard.md?plain=1#L578-L582
https://github.com/NixOS/nixfmt/blob/42e43d9fcabadf57fdcefa6da355cd8dcf5b7d36/standard.md?plain=1#L1194-L1195

Small example input

List expansion seems to be poor when it is not the last element on the line:

lib: {
  inherit null; # Force the entire attribute set to be multi-line, even without comments.

  # 1. The list is eagerly expanded when using 'map lib.id [ /* ... */ ]'.
  # 2. The list is expanded when appending '"seven"' to the list.
  list = lib.forEach [ "one" "two" "three" "four" "five" "six" ] lib.id;
}

Expected output

List expansion should be more eager:

lib: {
  inherit null; # Force the entire attribute set to be multi-line, even without comments.

  # 1. The list is eagerly expanded when using 'map lib.id [ /* ... */ ]'.
  # 2. The list is expanded when appending '"seven"' to the list.
  list = lib.forEach [
    "one"
    "two"
    "three"
    "four"
    "five"
    "six"
  ] lib.id;
}

Specifically, it should expand earlier, like when appending "seven" to the list. In fact, this output was generated by appending "seven" and then manually removing that line.

Actual output

This is the unmodified input:

lib: {
  inherit null; # Force the entire attribute set to be multi-line, even without comments.

  # 1. The list is eagerly expanded when using 'map lib.id [ /* ... */ ]'.
  # 2. The list is expanded when appending '"seven"' to the list.
  list = lib.forEach [ "one" "two" "three" "four" "five" "six" ] lib.id;
}

Although this issue is incidentally about the specific function [ /* ... */ ] argument formatting issue, I would like to hear opinions whether this reasoning should also be extended to other cases. Or does Nixfmt already do this for everything except this edge case?

Also, should we define "small"? Since I am not familiar with the Nixfmt implementation, I may be unaware of crucial and irreducible complexity, but maybe "small" could be defined as something as simple as:

# Expand too long lines.
if line.length > cfg.maxLineLenght then
    expand line

# Expand non-small lines.
else if line.elements.length > cfg.small then
    expand line

# Do not expand short and small lines.
else
    line

Dec 16 '25 23:12 trueNAHO