rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
13.79k stars 1.52k forks source link

Auto-import destroys previous formatting of nested-imports (merges all into one line) #17391

Open LukasKalbertodt opened 2 weeks ago

LukasKalbertodt commented 2 weeks ago

rust-analyzer version: 0.3.1995-standalone

rustc version: rustc 1.78.0 (9b00956e5 2024-04-29)

editor or extension: VSCode (extension version v0.3.1995)

code snippet to reproduce:

use std::{
    collections::HashMap,
    net::IpAddr,
    fs::{File, copy},
};

fn main() {
    let x = HashSet::new();
}

When triggering an auto-import for HashSet in above snippet, the import is changed to:

use std::{
    collections::{HashMap, HashSet}, fs::{copy, File}, net::IpAddr
};

This makes the import a lot less readable and leads to more diff-noise in git. Instead, I would have expected this:

use std::{
    collections::{HashMap, HashSet},
    net::IpAddr,
    fs::{File, copy},
};

This bug is already present for quite a while (years, if my memory serves me right).

lnicola commented 2 weeks ago

If it's your project, I suggest enabling format on save. It might be a while before we get a proper formatter.

LukasKalbertodt commented 2 weeks ago

Ah, that explains why this bug has existed for so long without people complaining: everyone's using rustfmt. My projects (including the one in question) indeed don't use rustfmt.

While understandable, sad to hear that this will likely not fixed anytime soon. So you don't think this can be fixed without implementing a proper formatter? Otherwise, if you point me at the right files, I could take a stab at it.

lnicola commented 2 weeks ago

I think it's around https://github.com/rust-lang/rust-analyzer/blob/575becbf27dfb95f845f3dfe46e5adae188316de/crates/ide-db/src/imports/insert_use.rs#L191. The main issue is that we're updating the AST.

Veykril commented 2 weeks ago

This not easy to fix without having our own formatting infra unfortunately.

lnicola commented 2 weeks ago

I don't see, but we must be dropping the original whitespace somewhere, right? Maybe with non_trivia_sibling.

LukasKalbertodt commented 2 weeks ago

This not easy to fix without having our own formatting infra unfortunately.

Not even for the special case of imports? I would expect that formatting nested imports is vastly easier than a generic formatting infra. And as @lnicola says, maybe it's also just a small bug of dropping whitespace somewhere.

Veykril commented 2 weeks ago

I don't see, but we must be dropping the original whitespace somewhere, right? Maybe with non_trivia_sibling.

Well, we are effectively taking apart the tree and re-building it (as we are zipping the old import tree with the new import tree when merging them iirc) so we drop all the inbetween trivia whitespace since our trivia model has those not attached to tokens but to the parent nodes. So this also might get somewhat fixed by us moving to the roslyin trivia model but thats also still some time away.

Veykril commented 5 days ago

One thing you can change to improve your experience here is set "rust-analyzer.imports.granularity.group" to "item" and "rust-analyzer.imports.granularity.enforce" to "true". This shiuld cause ra to always insert ne use items, not touching your existing ones and breaking their formatting. So then you only have to merge those in yourself in the end as a final cleanup step which should be less worm