auto_import: multiple assist actions to select import container

[eulerdisk]

This make auto_import assist to produce multiple actions to select in which module/function to perform auto import.

For example, if we have:

mod foo {
    mod bar {
        fn func1() {
            fn func0 () {
                std::fmt::Debug<|>
            }
        }
    }
}

you will see the following actions in this order:

import std::fmt::Debug in mod bar
import std::fmt::Debug in mod foo
import std::fmt::Debug in the current file
import std::fmt::Debug in fn func0
import std::fmt::Debug in fn func1

The priority here is modules > file > functions.

[kjeremy]

LGTM but I think the order should be file > modules > function since that order corresponds more to the natural file structure and most people put imports at the top of the file. If ordered in that way the order of arrow keys to your chosen option is also more natural.

[DJMcNab]

Are all of those guaranteed valid? I didn't think the stuff imported into parent modules would be in scope?

[kjeremy]

Sigh. Sometimes I forget that rust isn't C#.

[eulerdisk]

Are all of those guaranteed valid? I didn't think the stuff imported into parent modules would be in scope?

Nope. I plan to use name resolution in the future to provide only valid scopes and other improvements. At the moment auto import is only syntactic, it relies on people knowing what they are doing.
[Still better than going up and down doing things manually though]

[matklad]

Hm, I think this doesn't quite strike the right balance of "usefulness / attention" in this form. The general big problem with IDEs is that, although you can suggest a ton of stuff to the user, they can process only so much of it. So an intention which is always available but used only rarely is a net negative.

In this case, I think we almost always want to add import to the start of enclosing module: local imports are extremely rare, and importing "across" modules seems even less likely.

Its interesting that IntelliJ has a nice UI for such situations: you can "expand" a particular assist option from the menu to get a list of various flavors of the assist. But LSP does not support this yet. So I am inclined to close.

[eulerdisk]

Yes the fact that LSP has no "second level" actions is a problem here and for others assists I have in mind. This somewhat circumvent that limitation.

@matklad I'm thinking of two alternatives for this particular PR:

• remove functions from the list. In general there is only on nested module in the current file, so most of the times you will have two options, the current file and the nested module.
• remove functions from the list and show the "mod option" only if we are inside a test module (detecting the test module by the test attribute?). So you will have only the file option most of the time, and one mod option only when you are inside a test module. That covers a fairly common use case.

Tell me if you think one of these is reasonable, otherewise I'm fine if you close this.
Anyway, I would like to keep at least the first commit, which has some small fixes.

[daboross]

Just to ask, is there any situation you'd ever want import std::fmt::Debug in mod foo or
import std::fmt::Debug in the current file? If we know the user's cursor is within a child module, will it ever be useful to insert an import statement outside of that module?

I'm trying to think of a situation but I can't ever imagine it. If we don't have complete information then it makes sense - but it seems like this would be suggested even if we know for sure that the cursor is in a child module?

[eulerdisk]

[Maybe it was not clear for the first post, but this PR only show scopes containing the cursor]

@daboross the current implementation shows only options that contains the cursor.
So if we have this instead:

mod foo {
    std::fmt::Debug<|>
    mod bar {
        fn func1() {
            fn func0 () {
            }
        }
    }
}

that will show:

import std::fmt::Debug in mod foo
import std::fmt::Debug in the current file

Both mod foo and the current file make sense, which is better depends on what you are doing and what you are importing. It's impossible to guess which is right.
[I'm not sure if that was your question tough.]

[flodiebold]

If you were to add the import in the file in that situation, it wouldn't actually resolve, because named modules don't inherit the scope of their parent (anonymous modules do, though).

On the other hand, a common situation might be

mod test {
    use super::*;
    std::fmt::Debug<|>
}

, in which case importing in the file would work...

[eulerdisk]

Yes. Currently in that hypotetical situation you need to import two times the same thing, one for the file and one with the super specifier for the inner module,

[daboross]

Alright, thanks! That answers my question. I hadn't thought of the use super::*; case.

What I meant was that, for example, in this code you mentioned

mod foo {
    std::fmt::Debug<|>
    mod bar {
        fn func1() {
            fn func0 () {
            }
        }
    }
}

"import std::fmt::Debug in the current file" would have no use.

But it does in the case that use super::*; is used, so that's reasonable.

[matklad]

So you will have only the file option most of the time, and one mod option only when you are inside a test module.

Not that even inside test modules with use super::*, adding import to the parent module might not be what you want: in non-#[cfg(test)] mode, you'll get an "unused import" warning. It seems that if you are really in the situation when you need to add import to the parent module, it's easier to just type the import there in the first place, and maybe invoke some kind of "organize imports" intention to do the auto-merging.

So I suggest closing this and filing the first commit as a separate PR!

[eulerdisk]

@matklad Ok, then.
What about making auto import showing the enclosing module as the only option instead of the current file?

[matklad]

yep, we should imports to the current module by default!