Lazy fixes resolving

[popzxc]

Resolves #6433

Currently, fixes for diagnostics are resolved eagerly, which is not quite a good idea: when we collect diagnostics, we report all of them to user.
Fixes though are reported only if cursor is within the fix trigger range, otherwise they're just skipped.
Given the fact that resolving a fix may be an expensive operation (incorrect case diagnostic is a good example: in order to provide fix, we must track all the places where diagnostic is used), doing it just to be rejected because of cursor position is incorrect.

What this PR does: instead of providing a full fix, only the trigger range is provided by default. And only if rust-analyzer sees that fix exists and its range is suitable, it will try to resolve the fix.

@matklad I feel weird about Analysis::with_semantics method and having to call parse manually in the missing_record_expr_field_fix. It just doesn't feel right. Could you please tell whether the direction is at least correct?

Also, unfortunately I'm not able to reproduce the issue even with 1ms auto-save enabled, thus will be grateful if someone will check whether this fix works or not. In theory it should, but who knows.

cc @neilyoung @lnicola

[kjeremy]

Would #6317 #6370 help here? We can now pass data between publishDiagnostics and textDocument/codeAction (and then later codeAction/resolve which we do not implement yet but easily could).

[kjeremy]

@kjeremy wrong issue?

Good catch! fixed.

[matklad]

I think this moves roughly in the right direction, but I think the specifics of API should be different.

Specifically, I think this should be close to how code action works.

• In terms of client implementation, we should make use of new "resolve code action" facility. We currently implement it in a hacky way, we should change that to an upstream impl
• We should compute the edit when the user requests the fix. This is a slightly different logic than in this PR: we should compute ranges and names for all fixes, but compute an edit in a separate request.
• API-wise, rather than using closures, we should use the unresovled / resolved pattern of the assists. What might happen is that we compute diagnostics on one iteration of event loop, but apply (ie, compute the edit) on another event loop iteration.

[matklad]

So, I htink we should add something along this lines

impl Analysis {
    pub fn resolve_diagnostic_fix(
        &self,
        config: &DiagnosticsConfig,
        file_id: FileId,
        id: DiagnosticId
    ) -> Cancelable<Option<Fix>> {
    }
}

[popzxc]

I think I got the idea. I will try to implement it in the suggested way, but unfortunately I'm currently a bit overwhelmed with my main job, thus cannot provide any guarantees on how long it will take.

[popzxc]

Unfortunately, it doesn't seem that I'll be able to work on the rust analyzer at least 'till the end of the year.

Since this PR doesn't provide a proper fix, I'm closing it to not provide a stale entry in the list.

[matklad]

No worries, I’ll look into this soon-ish :) Thanks formally your awesome contributions, and we’ll be waiting for you next year ;-)

…

On Saturday, 21 November 2020, Igor Aleksanov ***@***.***> wrote: Unfortunately, it doesn't seem that I'll be able to work on the rust analyzer at least 'till the end of the year. Since this PR doesn't provide a proper fix, I'm closing it to not provide a stale entry in the list. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6466 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANB3M4Z5DRFUJ2IOK64GRTSQ6CHJANCNFSM4TKDHHKA> .