Fix #2443

[edwin0cheng]

This PR try to fix #2443 by update all use-as alias after PerNs changed in DefCollector.

[edwin0cheng]

Side note: The main reason I would like to fix this weird bug is, I want to move :
https://github.com/rust-analyzer/rust-analyzer/blob/8b278b1ab660df0728508e45e88ac769a2e03a58/crates/ra_hir_def/src/nameres/collector.rs#L770-L779

to

https://github.com/rust-analyzer/rust-analyzer/blob/8b278b1ab660df0728508e45e88ac769a2e03a58/crates/ra_hir_def/src/nameres/collector.rs#L470

And shared the collect_macro_expansion invocation. This will help us to refactor the whole macro expansion later.

[matklad]

This info is only need during name-resolutrion process, not afterweards, so this field should be a part of ModCollector

[edwin0cheng]

Yes, I agreed.

[matklad]

Hm, I am feeling uneasy about this approach. Seems like Name -> Name mapping errases essential information (like from what import the name initially came). I imagine use foo::a as b; use bar::a as b might actually be legal if a's live in different namespaces?

And I think we actually have all the required infrastructure in place to fix this properly? Basically, an use foo as bar import should be marked unresolved, and, when we add foo to the module namespace, on the next iteration of the loop { } we should also add bar. It seems like we need to fix some bug in existing infra, rather than add something specific for aliases tracking.

[edwin0cheng]

And I think we actually have all the required infrastructure in place to fix this properly? Basically, an use foo as bar import should be marked unresolved.

Um.. imagine this example:

mod m {
    macro_rules! _foo {
        ($x:ident) => { type $x = u64; }
    }
    pub(crate) use _foo as foo;
}

macro_rules! foo {
    () => {}
}

m::foo!(foo);
use foo as bar;   // <-----

The order of resolving will be:

mod m
macro_rules _foo 
use _foo as foo
macro_rules foo
use foo as bar  // foo is macro_rules! foo
m:foo!(foo)
type foo = u64

[edwin0cheng]

Hm, I am feeling uneasy about this approach. Seems like Name -> Name mapping errases essential information (like from what import the name initially came). I imagine use foo::a as b; use bar::a as b might actually be legal if a's live in different namespaces?

Yes, I agree, I didn't think of this problem. Can I fix it by adding more information to alias map ? Or do you feel there is a better approach to solve it ?

[matklad]

Ah, so the problem is that we end up with bar only as macro, and miss bar as a type, right?

So looks like the problem is that we remove use foo as bar from the set of unresolved imports when it resolves in any namespace, while we should only do this if it resolves in all namespaces. I wonder if we construct a test case here which doens't involve any macros? something like this

type T = ();

use self::T as  Y;

use m::*;

mod m {
  pub const T: () = ();
}

fn main() { let _: Y = Y; }

Looks like rust-analyzer indeed fails to see that Y is a value here:

[matklad]

yeah, one doesn't need aliases to construct a similar problem:

[edwin0cheng]

Yes indeed. The problem here is the use alias. My approach to fix it is mark the alias and rewrite to it. OTOH, we can mark it to unresolved, but the problem is, how can we know the resolve loop should be ended ?

[edwin0cheng]

yeah, one doesn't need aliases to construct a similar problem:

Okay, my alias fix won't work in your case. :)

[matklad]

Just checked that IntelliJ also doesn't handle the above example 😩

[edwin0cheng]

Closed, in flavor of #2466