Rename type import to alias

[fritzrehde]

If I have use Type; imported, I would like to be able to refactor that to use Type as CustomName; (in VS Code). Then, I would like all instances that previously used Type in that file to now use CustomName.
Is this already possible in VS Code somehow?

[Young-Flash]

Maybe could impl within a assist

[fritzrehde]

Sorry, I'm not sure what your response means @Young-Flash. Do you mean the feature exists already and I could implement/use it by using this assist somehow? Or do you mean that rust-analyzer could implement this internally with assist?

[fritzrehde]

If it really doesn't exist yet, I would propose one could just use the normal rename LSP command (e.g. Fn+F2 on Linux) for this, which currently says "Cannot rename a non-local definition".

[alibektas]

👋

We have the infra for this so it should be easy to do. Do you want to do it @Young-Flash or should I do it?

[fritzrehde]

Awesome, great news!

[Young-Flash]

Do you mean the feature exists already and I could implement/use it by using this assist somehow? Or do you mean that rust-analyzer could implement this internally with assist?

@fritzrehde Sorry for the unclearness, it's the latter. That feature hasn't implement for now. assist for R-A is something that offer by a💡in VSCode, which implement here, you can learn more about it from this blog.

[Young-Flash]

We have the infra for this so it should be easy to do. Do you want to do it @Young-Flash or should I do it?

@alibektas I could have a try.

It's okay if @fritzrehde feel like to implement this personally, I think R-A Community will happy to help new contributor to make their first contribution, there are some related PR which could refers to

[alibektas]

We have the infra for this so it should be easy to do. Do you want to do it @Young-Flash or should I do it?

@alibektas I could have a try.

It's okay if @fritzrehde feel like to implement this personally, I think R-A Community will happy to help new contributor to make their first contribution, there are some related PR which could refers to

Sure I will be posting some instructions in a couple of hours that will help with the implementation.

[alibektas]

IMPORTANT : Implementing this as an assist has a serious shortcoming, so it'd be better if we'd implemented it as a special case of renaming. See comments below where the problem is discussed.

So let's talk about how this issue can be solved. I will mostly omit the assist-related part. For this you will have numerous examples in the ide-assists crate. Just take a look at one of the files and do shamelessly copy and paste. 😄

Renaming in rust-analyzer is handled by two crates : ide and ide-db. ide will be the endpoint for renaming, basically the thing that is called once we get a renaming request ( and btw the cannot rename non-local definition is a bug and is discussed here #15656 , it will soon be fixed ). What we will be using is the ide_db::rename::Definition::rename fn. The problem however is that this returns a SourceChange

rust-analyzer/crates/ide-db/src/source_change.rs

Lines 19 to 23 in c1c9e10

	pub struct SourceChange {
	pub source_file_edits: IntMap<FileId, (TextEdit, Option<SnippetEdit>)>,
	pub file_system_edits: Vec<FileSystemEdit>,
	pub is_snippet: bool,
	}

Two things immediately draw our attention about SourceChange :

1. We only want to edit a single file ( FileId is the unique Id that you need to be using ) so you will need to limit your findings to the scope of this file. This should give you an idea.
   

   rust-analyzer/crates/ide-db/src/search.rs

   Lines 315 to 324 in c1c9e10

   	pub fn usages<'a>(self, sema: &'a Semantics<'_, RootDatabase>) -> FindUsages<'a> {
   	FindUsages {
   	def: self,
   	assoc_item_container: self.as_assoc_item(sema.db).map(|a| a.container(sema.db)),
   	sema,
   	scope: None,
   	include_self_kw_refs: None,
   	search_self_mod: false,
   	}
   	}

2. Most of the assist ( probably all of them ) you will see build their own SourceChanges by using a SourceChangeBuilder but you can change a SourceChangeBuilder's SourceChange by accessing its .source_change field.

Feel free to ask any questions you have!

[Veykril]

I actually had the idea in my mind that trigger a rename on an import should introduce an alias as the initial issue here outlines as well. Though we could probably have both, that rename behavior and an assist (for discoverability).

[UserIsntAvailable]

👋 I'm interested in implementing this, however I some questions regarding how it will work for end-users (specially after reading #15656).

The way that I particularly would like to implement this would be: if Type is considered "Local" (by R-A terms) then perform the renaming as usual, otherwise create a type alias. So:

use std::io::Result;

fn main() -> Result<()> {
    Ok(())
}

then If I try to rename Result to IoResult, it would do this:

use std::io::Result as IoResult;

fn main() -> IoResult<()> {
    Ok(())
}

This would work fine, but some users might not want this behavior. I think we could use rust-analyzer.rename.allowExternalItems here (or change the name to something like rust-analyzer.rename.externalItems), then provide an enum with two options: 1) alias, would create an alias like I explained above, and 2) rename, would just rename it like the option does when it is true.

cc @alibektas, because you were the one that implemented rust-analyzer.rename.allowExternalItems; do you think this is a good idea?

---

As a side note, I don't have a problem having an assist (code action) to do this, we could provide both options like @Veykril proposed above.

[alibektas]

👋 I know the day would soon come where I would say to myself, I'd better made that bool an enum, I just wasn't expecting it to happen to soon 😄. But jokes aside I think we should avoid changing the said config. I don't think that we should be using type aliasing as if it were a fallback for renaming. I think it's justified to have a "special" renaming function around import statements. So what I am trying to say is that renaming on an import should always produce type aliases and I am not too sure about what I am going to say now but I don't think that there is a party that wouldn't welcome this.

Why don't you implement this feature first only as an assist but instead of having the actual function that does the heavy work implemented in ide-assist crate have it somewhere in ide-db ( probably ide-db::rename idk)?

[UserIsntAvailable]

@alibektas

I know the day would soon come where I would say to myself, I'd better made that bool an enum, I just wasn't expecting it to happen to soon

I imagine that you would have that reaction when I realized that the feature was implemented 2 weaks ago 🤣

Why don't you implement this feature first only as an assist but instead of having the actual function that does the heavy work implemented in ide-assist crate have it somewhere in ide-db ( probably ide-db::rename idk)?

Ok sounds good, however I have a pretty big question:

ide-assist handlers can only use "predefined" values, as in, users can not pass values to it? (It doesn't seem that AssistContext has nothing related with that) So if I implement this, the assist is just gonna create a "dummy" type alias for you?

like:

use std::io::Result; // <- assist "generate_type_alias"

fn main() -> Result<()> {
    Ok(())
}

to

use std::io::Result as TypeAlias;

fn main() -> TypeAlias<()> {
    Ok(())
}

If the answer is no (you can provide values), how is this done? I checked similar, assists and they seem to just hard-code the names.

If the answer is yes, then this is problematic, because renaming aliases is currently unsupported. As a side note, an code action for this wouldn't be necessary if you were able to rename aliases, because you could create an alias with the same name of the qualified type (e.g use std::io::Result as Result;) then rename the type alias to the name you want and it should work all the time?

---

Maybe I'm overthinking this too much? Another alternative that I see is:

// User wants to rename `Result` to `IoResult`.
use std::io::Result;

fn main() -> Result<()> {
    Ok(())
}

So they change it like so:

use std::io::Result as IoResult;

fn main() -> Result<()> {
    Ok(())
}

Then a supposedly (apply_alias?) gonna find all the usages then apply those changes automatically?

[alibektas]

So the answer is sth in between the two. User can provide a value directly iff

1. the client has SnippetCapability which essentially lets us tell the text editor that we want a certain range in text to be edited by the user so the text editor should better put the cursor on this range
2. We are actually making use of this said capability 😄 ( An example )

So as an example an editor like VSCode does support snippets but another like Helix (IIRC) may not, which brings us to why you asked this question in the first place. You have absolutely a fair argument , once the user invokes the assist and edits alias's name one way or another, all the defs that refer to this by its previous name will now be invalid. And no we don't wait for user to edit to exit the assist routine : Once we created the text use std::io::Result as DummyAlias; we are done, rest is client's job to handle.
As a matter of fact now I am not so sure if this assist is possible at all.

The only way making this possible is if client supports multi cursor snippets which basically means putting cursor in front of every occurence of the aliased definition which is ofc ridiculous and I don't think that even VSCode supports this.

[alibektas]

To wrap things up : If you still want to do renaming you are most welcomed to do so. It may be a little more challenging than implementing an assist but it is a greenfield with regards to the allowExternalItems config. Every renaming on a use statement will be a aliasing and depending on the value of the said config it will either be possible or we will bail!("Cannot rename a non-local definition) like we do now

[UserIsntAvailable]

We are actually making use of this said capability 😄 ( An example )

Oh, I completely missed that, it was also used on extract_function...

So as an example an editor like VSCode does support snippets but another like Helix (IIRC) may not,

Weirdly enough, I installed a vscode to test if it would prompt me for a name (extract_function), but it just created the default identifier (fun_name)?

As as side note, on vscode extract_type_alias doesn't work with the handler's example (I'm getting Overlapping ranges are not allowed!), but works fine on neovim. Not that important for this conversation, just thought that it might be interesting to know.

You have absolutely a fair argument , once the user invokes the assist and edits alias's name one way or another, all the defs that refer to this by its previous name will now be invalid.

Yeah I had the suspicion that there was something wrong regarding this. This is my first my time contributing to RA so I wasn't really sure if I was missing something.

To wrap things up : If you still want to do renaming you are most welcomed to do so. It may be a little more challenging than implementing an assist but it is a greenfield with regards to the allowExternalItems config. Every renaming on a use statement will be a aliasing and depending on the value of the said config it will either be possible or we will bail!("Cannot rename a non-local definition) like we do now

So if I understand correctly, we keep the option (allowExternalItems), but instead of doing a "rename" we would create an alias for said use statement if the option is true, otherwise just error out?

[alibektas]

Weirdly enough, I installed a vscode to test if it would prompt me for a name (extract_function), but it just created the default identifier (fun_name)?

It won't prompt, it will rather put the cursor on fun_name ( we denote this with fun_$0name where $0 is the cursor ). But I see now that I sent you the wrong example. Just try Generate abc function and see how cursor moves onto the todo!() using

fn main() -> () {
    let i : i32 = abc(3);
    ()
}

As as side note, on vscode extract_type_alias doesn't work with the handler's example (I'm getting Overlapping ranges are not allowed!), but works fine on neovim. Not that important for this conversation, just thought that it might be interesting to know.

I didn't understand what you meant by handler's example.

So if I understand correctly, we keep the option (allowExternalItems), but instead of doing a "rename" we would create an alias for said use statement if the option is true, otherwise just error out?

This is a little vague but I am sure you mean what I meant but just to be clear. If we are looking at a renaming in a non-local create and the config has allowExternalItems set to true then we get to rename, otherwise we bail. If the create is local we have nothing to worry about at all.

Feel free to contact me if you have any questions.

[UserIsntAvailable]

Just try Generate abc function and see how cursor moves onto the todo!() using

Ok now I get it; thanks for the example.

I didn't understand what you meant by handler's example.

The example here, didn't work with vscode:

struct S {
    field: (u8, u8, u8),
}

If I select from "(u8, u8, u8)", then select "Extract type as alias", I get Overlapping ranges are not allowed!; I probably should open this as a separate issue, gonna see if I can replicate this in another machine.

This is a little vague but I am sure you mean what I meant but just to be clear. If we are looking at a renaming in a non-local create and the config has allowExternalItems set to true then we get to rename, otherwise we bail. If the create is local we have nothing to worry about at all.

Ok so:

allowExternalItems = true and it is indeed an external item => rename
allowExternalItems = false and it is indeed an external item => bail!
allowExternalItems = true|false and it is a local item => alias

right?

Feel free to contact me if you have any questions.

Will do; I kinda have an idea how to implement it, it just kinda confusing what would be the behavior that we want to go with 😅.

[Veykril]

(nit, you guys are talking about import aliases, not type aliases, those are different things)
For the assist idea to be really useful we'd need multi-cursor snippet support which I don't think we have (that way we can insert a new alias, rename all uses and give a cursor to the alias and all renamed usages).

Implementing the rename idea is nicer UX wise until then. You just need to check if the rename happens on a name inside a use tree and insert an alias and adjust the search scope for usage appropriately to just the module (there is a bit more to that in terms of other modules using that import but that is something that can be fixed as a next step).

[UserIsntAvailable]

@alibektas 👋

I went ahead and implemented the solution that Veykril suggested above. I got pretty far with it (I can even rename items in other modules!), but I have problem when I need to pick the references that needs to be renamed.

With the current implementation, all references, qualified and non-qualified, are being renamed, so:

pub mod foo { pub struct Foo; }

use foo::Foo$0;

fn main() {
    let _: Foo;
    let _: foo::Foo;
}

If renamed with Bar, then:

pub mod foo { pub struct Foo; }

use foo::Foo as Bar;

fn main() {
    let _: Bar;
    let _: foo::Bar; // <- wrong, should have continue being "foo::Foo".
}

(funny enough, I also ran on this problem when changing use foo::Foo;; it was also changing it to use foo::Bar as Bar. I'm actually special casing this on the current implementation, to just make it work.)

---

How should I approach this? Can I "configure" usages to be more "smart" about which references it needs to pick? Or I need to manually check if the provided reference has any PathSegment attached to it?

Thanks in advance.