"convert the identifier to uppercase" only renames binding

rust-lang / rust-analyzer

A Rust compiler front-end for IDEs

https://rust-analyzer.github.io/

Apache License 2.0

13.84k stars 1.53k forks source link

"convert the identifier to uppercase" only renames binding #8645

Open stanciuadrian opened 3 years ago

stanciuadrian commented 3 years ago

Input:

fn main() {
    const a: u32 = 256; 
    println!("{}", a);
}

2 fixes are suggested:

The first one renames the binding and misses the reference.

The second one works fine.

Veykril commented 3 years ago

The first quickfix is emitted by rustc which only renames the item, the second one is the one from rust-analyzer which fixed the usages. We should make it clearer that the former isn't the quickfix one should be using.

SomeoneToIgnore commented 3 years ago

Afaik we've already started excluding the rustc quickfixes for what we have a decent alternative? At least that should be the endgoal anyway, so we might just filter that one out too.

Veykril commented 3 years ago

I don't think we are filtering them yet, unless I missed that

SomeoneToIgnore commented 3 years ago

Indeed, the Cargo messages are skipped, not the rustc ones now:

https://github.com/rust-analyzer/rust-analyzer/blob/b3a7953cae4f14f74934cc64ba7ccf7b3eb6cd08/crates/flycheck/src/lib.rs#L326-L344

matklad commented 3 years ago

We should implement "prefer our fixes to rustc" logic.

I think the steps are:

assign codes to our diagnostics to match rustc
in https://github.com/rust-analyzer/rust-analyzer/blob/eee50b6921662dee766c987e1a0b15b371e141d1/crates/rust-analyzer/src/handlers.rs#L1020-L1029, filter out fixes with matching error codes
in https://github.com/rust-analyzer/rust-analyzer/blob/eee50b6921662dee766c987e1a0b15b371e141d1/crates/rust-analyzer/src/config.rs#L96-L116, add a setting to override this behavior.

pitaj commented 2 years ago

I would like to try my hand at this.

pitaj commented 2 years ago

Okay, I've looked around a bit.

It appears that diagnostics-related code is spread about various crates:

ide_diagnostics
hir
hir_ty
hir_def
rust_analyzer

So I'm not exactly sure what a good place would be to assign check codes (from rustc or clippy) to our diags. And it appears that in some cases (like this one) there are multiple rustc lints covered by a single native diag. So it may make more sense to have a central map of (check diag code => native diag code) somewhere (rust_analyzer crate probably?).

In fact, I'm wondering if we could just have a list of "covered fixes" where, if it's known that native diags cover the fix, we just discard it without checking if a fix was actually provided by native diags. That wouldn't cover a case where a check fix is provided but the native diag misses the fix, but I have no idea how common such a case is.