Prefer imports starting with std

[SomeoneToIgnore]

Closes #2915

[SomeoneToIgnore]

I'd like to write a test on it, but could not reproduce the alloc::sync::Arc & std::sync::Arc structure in the test: if I try to use nested modules as /std/sync/mod.rs crate:std deps:alloc, I get panics despite the paths being correct (as if the crate name is not considered in the imports).
If I try to emulate it with the pub mod sync instead, I always get the best_path == None and cannot get into the right match branch in the method I've added.

Apparently, I'm doing something wrong, but I have no clue on how to make it proper.
Any ideas?

[matklad]

@SomeoneToIgnore could you post an exact test that does not work?

[matklad]

Can we express this as a key function? like (path.starts_with_std(), path.len())?

[SomeoneToIgnore]

Yes, way more readable now, thanks.

[SomeoneToIgnore]

@SomeoneToIgnore could you post an exact test that does not work?

I think I should stop writing the code late in the night (I wish I have any other time to do so for RA though... )
I've started writing an extended explanation to you and realised that all my assumptions about those tests are wrong.

For some reason, I've thought that lines //- /sync/mod.rs crate:std deps:alloc do something, as if they were forcing the files with the code below to be created or something.
Now I realize those are just comments.

I'll try to dig it more, thank you for reaching out to help me here.

[matklad]

For some reason, I've thought that lines //- /sync/mod.rs crate:std deps:alloc do something

They very much do something: https://github.com/rust-analyzer/rust-analyzer/blob/0d6e5a986cc7c3a434c2107edf748d1c26ac3f69/crates/ra_db/src/fixture.rs#L152-L163

[SomeoneToIgnore]

Then I'm fully confused by their behavior (I know I should look in the test code better, but did not have the possibility to do that properly yet).

I try to replicate the Arc location in both crates and import it.

This test panics:

#[test]
fn why_does_it_panic() {
    let code = r#"
    //- /main.rs crate:main deps:std
    <|>
    //- /sync/mod.rs crate:std deps:alloc
    pub use alloc::sync::Arc;
    //- /sync.rs crate:alloc
    pub struct Arc;
    "#;
    check_found_path(code, "std::sync::Arc");
}

its simplified version also:

#[test]
fn why_does_it_panic() {
    let code = r#"
    //- /main.rs crate:main deps:std
    <|>
    //- /sync/mod.rs crate:std
    pub struct Arc;
    "#;
    check_found_path(code, "std::sync::Arc");
}

When I try to use pub mod instead, it "works:"

#[test]
fn zzzzz() {
    let code = r#"
    //- /main.rs crate:main deps:std
    <|>
    //- /sync/mod.rs crate:std deps:alloc
    pub mod sync {
        pub use alloc::sync::Arc;
    }
    //- /sync.rs crate:alloc
    pub mod sync {
        pub struct Arc;
    }
    "#;
    check_found_path(code, "std::sync::Arc");
}

but that is odd, since I basically duplicate module names in the file and in the mod declaration, from my experience with the Rust module system, the resulting import should be std::sync::sync::Arc.

And, anyway, when I put the debug prints into the prefer_new_path method, it never gets to any of the first two match arms, which is exactly what I want to test.
Moreover, best_path is always None for that test.

[matklad]

Ah, well, it works exactly as expected! (but it took me a long time to realize this).

You want something like this:

        let code = r#"
        //- /main.rs crate:main deps:std
        <|>

        //- /std.rs crate:std deps:alloc
        pub use alloc::sync

        //- /alloc.rs crate:alloc
        pub mod sync {
            pub struct Arc;
        }
        "#;

files with //- are crate roots, you need to make sure they contain the module structure you want.

[SomeoneToIgnore]

Ok, so that's basically my 2nd option that does not go into the if branch I want :D
Thanks for the explanation anyway, I will keep searching for some test case that covers it.

[matklad]

Yeah, to clarify, if you write crate:foo, the file name is mostly irrelevant, as it becomes the root module of the crate.

The file name matters for finding child modules, but that's it

[flodiebold]

Also note that if your main crate in the test only has the dependency to std, the test doesn't actually reproduce the problem, since the main crate won't be able to refer to alloc at all.

[SomeoneToIgnore]

Well, simply adding it to the std dependencies did not help to enter the required branch, but thanks a lot, that's what I've been missing from the very start when tried to make it work in the night.

[SomeoneToIgnore]

And now it's ready to be reviewed, thank you again for the help.

[matklad]

bors r+

[matklad]

👍

[SomeoneToIgnore]

bors r+

Judging by the look of the checks, bors decided to ignore this one :)

[matklad]

bors ping

[bors]

pong

[matklad]

bors merge

[bors]

Build succeeded

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript

[flodiebold]

Bors doesn't seem to take commands from review comments anymore.