rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.28k stars 1.61k forks source link

Import item suggestion should come before did-you-mean suggestion #11862

Open jplatte opened 2 years ago

jplatte commented 2 years ago

I've hit this case a few times now: I do some regex-based mass renaming that only works for expressions, not imports, then manually clean up the old broken import and want to import the new thing using the import suggstion whereever a new name needs importing.

This usually works fine, until there is an item already in scope with a similar name to what I need to import. My muscle memory then sometimes leads me to apply the first suggestion of "there's a thing with a similar name in scope, did you mean that?" when that is not what I want.

I think when there is an exact match for an unresolved name, the suggestion to import it should be higher up in the list of suggestions than pretty much anything else.

timstr commented 1 year ago

This impacts me too on a regular basis. I would happily disable the similar name suggestions altogether if there was an option.

The following is a basic example of why the name suggestions coming first is counterproductive. I've just cut/pasted two functions serialize_object_id and deserialize_object_id to a separate source file and now I'm doing a pass to add imports to those functions where they're used. First, I find a use of serialize_object_id with error squiggles and Rust-Analyzer kindly offers to import it as the first item. This make it very easy to fix such imports with mostly muscle memory.

image

Unfortunately, after I've added this import, Rust-Analyzer now seems to think that deserialize_object_id is more likely to be a typo than a real function. Listing the false spelling correction first means I can't rely on muscle memory any more and could easily introduce bugs if I'm not being vigilant.

image

Please allow me to disable spelling-based suggestions.

lnicola commented 1 year ago

@timstr that looks like a rustc suggestion. Does it show up in cargo check?

timstr commented 1 year ago

@lnicola You are correct! Thanks for that suggestion, I was not aware of that

cargo check indeed says:

error[E0425]: cannot find function `deserialize_object_id` in this scope
   --> src/ui_core/graph_ui_state.rs:237:22
    |
237 |               let id = deserialize_object_id(&mut d1, idmap)?;
    |                        ^^^^^^^^^^^^^^^^^^^^^
    |
   ::: src/core/graphserialization.rs:494:1
    |
494 | / pub(crate) fn serialize_object_id(
495 | |     id: ObjectId,
496 | |     serializer: &mut Serializer,
497 | |     idmap: &ForwardGraphIdMap,
...   |
508 | |     }
509 | | }
    | |_- similarly named function `serialize_object_id` defined here
    |
help: a function with a similar name exists
    |
237 |             let id = serialize_object_id(&mut d1, idmap)?;
    |                      ~~~~~~~~~~~~~~~~~~~
help: consider importing this function
    |
1   | use crate::core::graphserialization::deserialize_object_id;
timstr commented 1 year ago

Nonetheless, are you aware of any way to disable or at least demote the spelling-based suggestion? I'm having trouble finding discussion about this rustc suggestion. Thanks in advance for any guidance

lnicola commented 1 year ago

No, not really. I guess you can disable cargo check and run it when needed (there's a rust-analyzer: Run command), possibly relying on our experimental diagnostics and the unresolved symbol highlighting:

{
    "editor.semanticTokenColorCustomizations": {
        "rules": {
            "unresolvedReference": "#ff0000",
            "*.unsafe:rust": "#eb5046"
        },
    },
    "rust-analyzer.checkOnSave": false,
    "rust-analyzer.diagnostics.experimental.enable": true
}

But these aren't super-complete or precise.

timstr commented 1 year ago

Thanks a lot for your help @lnicola. I tried those settings and sadly it made most of the diagnostics and error squiggles I was relying on go away. I can live with the spelling suggestions