Skip to content

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

@LukasKalbertodt

Description

@LukasKalbertodt
Member

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).

Activity

lnicola

lnicola commented on Jun 11, 2024

@lnicola
Member

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

LukasKalbertodt

LukasKalbertodt commented on Jun 11, 2024

@LukasKalbertodt
MemberAuthor

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

lnicola commented on Jun 11, 2024

@lnicola
Member

I think it's around

fn insert_use_with_alias_option(
. The main issue is that we're updating the AST.

Veykril

Veykril commented on Jun 11, 2024

@Veykril
Member

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

lnicola

lnicola commented on Jun 11, 2024

@lnicola
Member

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

LukasKalbertodt

LukasKalbertodt commented on Jun 11, 2024

@LukasKalbertodt
MemberAuthor

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

Veykril commented on Jun 11, 2024

@Veykril
Member

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

Veykril commented on Jun 26, 2024

@Veykril
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-formattingformatting of r-a output/formatting on saveC-bugCategory: bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @lnicola@Veykril@LukasKalbertodt@DropDemBits

        Issue actions

          Auto-import destroys previous formatting of nested-imports (merges all into one line) · Issue #17391 · rust-lang/rust-analyzer