Vscode-rust: Format document / selection makes destructive edits

[pocesar]

This snippet is enough to show the problem

before:

use self::winapi::um::{
  winnt::WCHAR,
  winbase::{
    CreateSymbolicLinkW,
    CopyFileW
  }
};

after:

use self::winapi::um::{
  winbase::{CopyFileW, CreateSymbolicLinkW},

};

happens with both Format document or Format selection

[elniallo]

Seeing the same behaviour, when autoformat tries to alphabetise imports, the imports are overwritten rather than orderered

[tensor-programming]

Yes, also seeing this behavior. Tons of imports just getting removed and causing issues on save and format. Sort of weird and extremely annoying.

[museun]

Setting reorder_imports = false and reorder_modules = false in my rustfmt.toml is a temporarily solution for this.

Similarly this bug happens with mod. Some observations:

Edit-order dependent sorting:

mod a;
mod b; // I move this above 'a'
// to produce
mod b;
mod a;

I get:

                // a blank line appeared here
mod a;
mod b;

yet if I move 'a' below 'b'

mod a; // I move this below the b
mod b; 
// to produce
mod b;
mod a;

I get this:

                // a blank line appeared here
mod a;mod b;

which if I format it again, I get

                // a blank line appeared here
mod a;
mod b;

To get a destructive write to happen, simply:

mod b;
mod a;
mod c;

will produce

                // a blank line appeared here
mod a;
mod b;

it deleted the first line and never inserted the mod c back in.
this deletion happens with b,a,c and b,c,a (it compounds when you have way more than just 3 options)

with a,c,b or c,b,a it sorts them properly, but adds a blank line after the first

mod a;
mod c;
mod b;
// produces
mod a;
                // a blank line appeared here
mod b;
mod c;

but

mod c;
mod a;
mod b;
// produces
                // a blank line appeared here
mod a;
mod b;
mod c;

It doesn't happen with cargo fmt (or rustfmt) (stable, or the (current) nightly). Changing the line endings doesn't change the bad results,

For added fun, try out reorder_impl_items = true (not enabled by default, while the other two are.)

trait T {
    type A;
    const B: i32;
    fn c();
}
struct S;
impl T for S {
    const B: i32 = 1;
    fn c() {}
    type A = ();
}

will be changed into..

// ...
struct S;
impl T for S {
    type A = ();



}

(Depending on the order of A,B,c the error can be as serious as removing 2/3rds of the statements, or just dropping the final })

And for comparison. This (correctly) is what rustfmt produces for the above:

struct S;
impl T for S {
    type A = ();

    const B: i32 = 1;

    fn c() {}
}

I'd probably advise putting a big warning up on that one until this is fixed. Losing a use-statement, or module decl while, very annoying, is just a small setback and just a potential "..I thought I imported that" during a cargo check. But losing, potentially, an entire trait implementation is quite bad.

** Edit. I forgot to include the versions I tested the above with. I've seen it happen on both (current) stable and nightly for both msvc and gnu (wsl, technically in my case) toolchains.

Tested/demoed with:

rustfmt --version
rustfmt 1.2.1-nightly (b860feaf 2019-04-18)

rls --version
rls 1.36.0 (20e3268 2019-04-21)

[viktorku]

Another easy way to repro:

use std::fs::File;
use std::io::prelude::*;
use std::collections::HashMap;

fn main() -> std::io::Result<()> {
    let mut file = File::open("foo.txt")?;
    Ok(())
}

Formatting removes the File import, resulting in a broken state:

use std::collections::HashMap;
use std::io::prelude::*;


fn main() -> std::io::Result<()> {
    let mut file = File::open("foo.txt")?;
    Ok(())
}

It does indeed seem like the reordering is to blame.

[tensor-programming]

I can verify that this is absolutely related to import reordering. I've also found a way to potentially avoid this behavior though its not really a solution in my mind.

Say you are imports declared as so:

use std::env;
use std::io;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};
use std::fs;
use std::fs::File;

Turns into this upon save (deleting use std::io;:

use std::env;
use std::fs;
use std::fs::File;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};

However if you format your imports like this:

use std::env;
use std::io;
use std::io::{Read, Write};
use std::path::{Path, PathBuf};

use std::fs;
use std::fs::File;

it will not change the ordering or destroy any of the other imports. It seems that a new line is enough to stop this so long as all of the blocks are already ordered. The implication is that you can place newlines between all of your imports but this is just too unwieldy. This wouldn't be such a problem if there was a way to turn off import ordering.

[Ten0]

Same issue here.
I think we'd have noticed this issue if it had been here before (so this appeared quite recently).
Also, this doesn't happen when directly using rustfmt.

[Spanfile]

Ran into this myself, got a case where there's no imports to reorder:

fn main() {
    std::thread::spawn(move || {
        loop {
            if true {
                println!("a");
            }

            println!("a");
        }
    });
}

Formatting turns it into:

fn main() {
    std::thread::spawn(move || loop {
        if true {
            println!("a");
        }

        println!("a");
}

The brackets surrounding the loop are removed, but so is the loop's own closing bracket, the method's closing parenthesis and the semicolon following it. However, deleting the if inside the loop so the function is just:

fn main() {
    std::thread::spawn(move || {
        loop {
            println!("a");
        }
    });
}

and then formatting turns it properly into:

fn main() {
    std::thread::spawn(move || loop {
        println!("a");
    });
}

Plugin version: 0.6.1. RLS, Cargo and rustc version: 1.35.0.

[leo-lb]

Same issue https://github.com/rust-lang/rls-vscode/issues/603 (I believe)

[feuerste]

As well as #595 ?

[Xanewok]

This is confirmed and fixed in Rust 1.36 (now in beta, will be released in a week), please try using 1.36 and reopen if that still occurs using that channel. Thanks for reporting this and sorry for the hassle!