Skip to content

Import assist for traits on unresolved trait item should offer importing anonymously #15684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Veykril opened this issue Sep 29, 2023 · 3 comments · Fixed by #15788
Closed

Import assist for traits on unresolved trait item should offer importing anonymously #15684

Veykril opened this issue Sep 29, 2023 · 3 comments · Fixed by #15788
Labels
A-assists C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started

Comments

@Veykril
Copy link
Member

Veykril commented Sep 29, 2023

That is in the following case

// foo.rs
trait Foo {
    fn f(&self) {}
}
impl Foo for u32 {}
// main.rs
mod foo;
fn main() {
    0u32.foo$0();
}

We should show the general Import `crate::foo::Foo` as well as a Import `crate::foo::Foo as _` that imports the trait without bringing the name of it into scope.

@Veykril Veykril added A-assists C-feature Category: feature request labels Sep 29, 2023
@Veykril Veykril self-assigned this Oct 6, 2023
@Veykril
Copy link
Member Author

Veykril commented Oct 10, 2023

What needs to be done is roughly the following:

let (import_assets, syntax_under_caret) = find_importable_node(ctx)?;

Here we are fetching the ImportAssets which describe what kind of thing we want to import, it contains an ImportCandidate :
pub enum ImportCandidate {
/// A path, qualified (`std::collections::HashMap`) or not (`HashMap`).
Path(PathImportCandidate),
/// A trait associated function (with no self parameter) or an associated constant.
/// For 'test_mod::TestEnum::test_function', `ty` is the `test_mod::TestEnum` expression type
/// and `name` is the `test_function`
TraitAssocItem(TraitImportCandidate),
/// A trait method with self parameter.
/// For 'test_enum.test_method()', `ty` is the `test_enum` expression type
/// and `name` is the `test_method`
TraitMethod(TraitImportCandidate),
}

Leveraging that info we need to add additional import groups that will offer an _ import here

for import in proposed_imports {
let import_path = import.import_path;
acc.add_group(
&group_label,
AssistId("auto_import", AssistKind::QuickFix),
format!("Import `{}`", import_path.display(ctx.db())),
range,
|builder| {
let scope = match scope.clone() {
ImportScope::File(it) => ImportScope::File(builder.make_mut(it)),
ImportScope::Module(it) => ImportScope::Module(builder.make_mut(it)),
ImportScope::Block(it) => ImportScope::Block(builder.make_mut(it)),
};
insert_use(&scope, mod_path_to_ast(&import_path), &ctx.config.insert_use);
},
);
}

To able to add the as _ part to the inserted use item, we need to modify the insert_use function and have it set the rename component appropriately here

let use_item =
make::use_(None, make::use_tree(path.clone(), None, None, false)).clone_for_update();

It's probably easiest to split out the function into two, one for the previous behavior, one for the underscore behavior and then have those wrap a function that takes an extra param to not having to edit all the current call sites of the function.

@Veykril Veykril added the E-has-instructions Issue has some instructions and pointers to code to get started label Oct 10, 2023
@Veykril Veykril removed their assignment Oct 16, 2023
@Young-Flash
Copy link
Member

I'd like to have a try.

It's probably easiest to split out the function into two, one for the previous behavior, one for the underscore behavior and then have those wrap a function that takes an extra param to not having to edit all the current call sites of the function.

But how could we have a new function with an extra param without edit other call of insert_use? There's not something like default param in Rust.

@Young-Flash
Copy link
Member

Young-Flash commented Oct 22, 2023

But how could we have a new function with an extra param without edit other call of insert_use? There's not something like default param in Rust.

oh maybe I got it.

pub fn insert_use(scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig) {
    insert_use_with_alias_option(scope, path, cfg, None);
}

pub fn insert_use_as_alias(scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig) {
    let alias = ...;
    insert_use_with_alias_option(scope, path, cfg, alias);
}

fn insert_use_with_alias_option(scope: &ImportScope, path: ast::Path, cfg: &InsertUseConfig, alias: Option<ast::Rename>) {
    ...

    let use_item = if alias.is_some() {
        make::use_(None, make::use_tree(path.clone(), None, alias, false)).clone_for_update()
    } else {
        make::use_(None, make::use_tree(path.clone(), None, None, false)).clone_for_update()
    };

    ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants