Merge #8494

8494: internal: unfork code paths for unresolved and resolved assist r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

Author: bors[bot]

crates/ide_assists/src/lib.rs

@@ -28,7 +28,9 @@ pub use assist_config::AssistConfig;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum AssistKind {
+    // FIXME: does the None variant make sense? Probably not.
     None,
+
     QuickFix,
     Generate,
     Refactor,

crates/rust-analyzer/src/from_proto.rs

@@ -42,27 +42,27 @@ pub(crate) fn text_range(line_index: &LineIndex, range: lsp_types::Range) -> Tex
     TextRange::new(start, end)
 }
 
-pub(crate) fn file_id(world: &GlobalStateSnapshot, url: &lsp_types::Url) -> Result<FileId> {
-    world.url_to_file_id(url)
+pub(crate) fn file_id(snap: &GlobalStateSnapshot, url: &lsp_types::Url) -> Result<FileId> {
+    snap.url_to_file_id(url)
 }
 
 pub(crate) fn file_position(
-    world: &GlobalStateSnapshot,
+    snap: &GlobalStateSnapshot,
     tdpp: lsp_types::TextDocumentPositionParams,
 ) -> Result<FilePosition> {
-    let file_id = file_id(world, &tdpp.text_document.uri)?;
-    let line_index = world.file_line_index(file_id)?;
+    let file_id = file_id(snap, &tdpp.text_document.uri)?;
+    let line_index = snap.file_line_index(file_id)?;
     let offset = offset(&line_index, tdpp.position);
     Ok(FilePosition { file_id, offset })
 }
 
 pub(crate) fn file_range(
-    world: &GlobalStateSnapshot,
+    snap: &GlobalStateSnapshot,
     text_document_identifier: lsp_types::TextDocumentIdentifier,
     range: lsp_types::Range,
 ) -> Result<FileRange> {
-    let file_id = file_id(world, &text_document_identifier.uri)?;
-    let line_index = world.file_line_index(file_id)?;
+    let file_id = file_id(snap, &text_document_identifier.uri)?;
+    let line_index = snap.file_line_index(file_id)?;
     let range = text_range(&line_index, range);
     Ok(FileRange { file_id, range })
 }
@@ -82,7 +82,7 @@ pub(crate) fn assist_kind(kind: lsp_types::CodeActionKind) -> Option<AssistKind>
 }
 
 pub(crate) fn annotation(
-    world: &GlobalStateSnapshot,
+    snap: &GlobalStateSnapshot,
     code_lens: lsp_types::CodeLens,
 ) -> Result<Annotation> {
     let data = code_lens.data.unwrap();
@@ -91,25 +91,25 @@ pub(crate) fn annotation(
     match resolve {
         lsp_ext::CodeLensResolveData::Impls(params) => {
             let file_id =
-                world.url_to_file_id(&params.text_document_position_params.text_document.uri)?;
-            let line_index = world.file_line_index(file_id)?;
+                snap.url_to_file_id(&params.text_document_position_params.text_document.uri)?;
+            let line_index = snap.file_line_index(file_id)?;
 
             Ok(Annotation {
                 range: text_range(&line_index, code_lens.range),
                 kind: AnnotationKind::HasImpls {
-                    position: file_position(world, params.text_document_position_params)?,
+                    position: file_position(snap, params.text_document_position_params)?,
                     data: None,
                 },
             })
         }
         lsp_ext::CodeLensResolveData::References(params) => {
-            let file_id = world.url_to_file_id(&params.text_document.uri)?;
-            let line_index = world.file_line_index(file_id)?;
+            let file_id = snap.url_to_file_id(&params.text_document.uri)?;
+            let line_index = snap.file_line_index(file_id)?;
 
             Ok(Annotation {
                 range: text_range(&line_index, code_lens.range),
                 kind: AnnotationKind::HasReferences {
-                    position: file_position(world, params)?,
+                    position: file_position(snap, params)?,
                     data: None,
                 },
             })

crates/rust-analyzer/src/handlers.rs

@@ -8,16 +8,16 @@ use std::{
 };
 
 use ide::{
-    AnnotationConfig, FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData, Query,
-    RangeInfo, Runnable, RunnableKind, SearchScope, SourceChange, TextEdit,
+    AnnotationConfig, AssistKind, FileId, FilePosition, FileRange, HoverAction, HoverGotoTypeData,
+    Query, RangeInfo, Runnable, RunnableKind, SearchScope, SourceChange, TextEdit,
 };
 use ide_db::SymbolKind;
 use itertools::Itertools;
 use lsp_server::ErrorCode;
 use lsp_types::{
     CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem,
     CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams,
-    CodeActionKind, CodeLens, CompletionItem, Diagnostic, DiagnosticTag, DocumentFormattingParams,
+    CodeLens, CompletionItem, Diagnostic, DiagnosticTag, DocumentFormattingParams,
     DocumentHighlight, FoldingRange, FoldingRangeParams, HoverContents, Location, NumberOrString,
     Position, PrepareRenameResponse, Range, RenameParams, SemanticTokensDeltaParams,
     SemanticTokensFullDeltaResult, SemanticTokensParams, SemanticTokensRangeParams,
@@ -36,7 +36,7 @@ use crate::{
     diff::diff,
     from_proto,
     global_state::{GlobalState, GlobalStateSnapshot},
-    line_index::{LineEndings, LineIndex},
+    line_index::LineEndings,
     lsp_ext::{self, InlayHint, InlayHintsParams},
     lsp_utils::all_edits_are_disjoint,
     to_proto, LspError, Result,
@@ -982,86 +982,66 @@ pub(crate) fn handle_code_action(
     params: lsp_types::CodeActionParams,
 ) -> Result<Option<Vec<lsp_ext::CodeAction>>> {
     let _p = profile::span("handle_code_action");
-    // We intentionally don't support command-based actions, as those either
-    // requires custom client-code anyway, or requires server-initiated edits.
-    // Server initiated edits break causality, so we avoid those as well.
+
     if !snap.config.code_action_literals() {
+        // We intentionally don't support command-based actions, as those either
+        // require either custom client-code or server-initiated edits. Server
+        // initiated edits break causality, so we avoid those.
         return Ok(None);
     }
 
-    let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
-    let line_index = snap.file_line_index(file_id)?;
-    let range = from_proto::text_range(&line_index, params.range);
-    let frange = FileRange { file_id, range };
+    let line_index =
+        snap.file_line_index(from_proto::file_id(&snap, &params.text_document.uri)?)?;
+    let frange = from_proto::file_range(&snap, params.text_document.clone(), params.range)?;
 
     let mut assists_config = snap.config.assist();
     assists_config.allowed = params
-        .clone()
         .context
         .only
+        .clone()
         .map(|it| it.into_iter().filter_map(from_proto::assist_kind).collect());
 
     let mut res: Vec<lsp_ext::CodeAction> = Vec::new();
 
-    let include_quick_fixes = match &params.context.only {
-        Some(v) => v.iter().any(|it| {
-            it == &lsp_types::CodeActionKind::EMPTY || it == &lsp_types::CodeActionKind::QUICKFIX
-        }),
+    let include_quick_fixes = match &assists_config.allowed {
+        Some(v) => v.iter().any(|it| it == &AssistKind::None || it == &AssistKind::QuickFix),
         None => true,
     };
-    if include_quick_fixes {
-        add_quick_fixes(&snap, frange, &line_index, &mut res)?;
-    }
-
-    if snap.config.code_action_resolve() {
-        for (index, assist) in
-            snap.analysis.assists(&assists_config, false, frange)?.into_iter().enumerate()
-        {
-            res.push(to_proto::unresolved_code_action(&snap, params.clone(), assist, index)?);
-        }
-    } else {
-        for assist in snap.analysis.assists(&assists_config, true, frange)?.into_iter() {
-            res.push(to_proto::resolved_code_action(&snap, assist)?);
-        }
-    }
+    let code_action_resolve_cap = snap.config.code_action_resolve();
 
-    Ok(Some(res))
-}
+    let mut assists = Vec::new();
 
-fn add_quick_fixes(
-    snap: &GlobalStateSnapshot,
-    frange: FileRange,
-    line_index: &LineIndex,
-    acc: &mut Vec<lsp_ext::CodeAction>,
-) -> Result<()> {
-    let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics(), frange.file_id)?;
+    // Fixes from native diagnostics.
+    if include_quick_fixes {
+        let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics(), frange.file_id)?;
+        assists.extend(
+            diagnostics
+                .into_iter()
+                .filter_map(|d| d.fix)
+                .filter(|fix| fix.target.intersect(frange.range).is_some()),
+        )
+    }
 
-    for fix in diagnostics
-        .into_iter()
-        .filter_map(|d| d.fix)
-        .filter(|fix| fix.target.intersect(frange.range).is_some())
-    {
-        if let Some(source_change) = fix.source_change {
-            let edit = to_proto::snippet_workspace_edit(&snap, source_change)?;
-            let action = lsp_ext::CodeAction {
-                title: fix.label.to_string(),
-                group: None,
-                kind: Some(CodeActionKind::QUICKFIX),
-                edit: Some(edit),
-                is_preferred: Some(false),
-                data: None,
-            };
-            acc.push(action);
-        }
+    // Assists proper.
+    assists.extend(snap.analysis.assists(&assists_config, !code_action_resolve_cap, frange)?);
+    for (index, assist) in assists.into_iter().enumerate() {
+        let resolve_data =
+            if code_action_resolve_cap { Some((index, params.clone())) } else { None };
+        let code_action = to_proto::code_action(&snap, assist, resolve_data)?;
+        res.push(code_action)
     }
 
+    // Fixes from `cargo check`.
     for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() {
+        // FIXME: this mapping is awkward and shouldn't exist. Refactor
+        // `snap.check_fixes` to not convert to LSP prematurely.
         let fix_range = from_proto::text_range(&line_index, fix.range);
         if fix_range.intersect(frange.range).is_some() {
-            acc.push(fix.action.clone());
+            res.push(fix.action.clone());
         }
     }
-    Ok(())
+
+    Ok(Some(res))
 }
 
 pub(crate) fn handle_code_action_resolve(
@@ -1091,7 +1071,7 @@ pub(crate) fn handle_code_action_resolve(
     let index = index.parse::<usize>().unwrap();
     let assist = &assists[index];
     assert!(assist.id.0 == id);
-    let edit = to_proto::resolved_code_action(&snap, assist.clone())?.edit;
+    let edit = to_proto::code_action(&snap, assist.clone(), None)?.edit;
     code_action.edit = edit;
     Ok(code_action)
 }

crates/rust-analyzer/src/to_proto.rs

@@ -843,40 +843,31 @@ pub(crate) fn code_action_kind(kind: AssistKind) -> lsp_types::CodeActionKind {
     }
 }
 
-pub(crate) fn unresolved_code_action(
+pub(crate) fn code_action(
     snap: &GlobalStateSnapshot,
-    code_action_params: lsp_types::CodeActionParams,
     assist: Assist,
-    index: usize,
+    resolve_data: Option<(usize, lsp_types::CodeActionParams)>,
 ) -> Result<lsp_ext::CodeAction> {
-    assert!(assist.source_change.is_none());
-    let res = lsp_ext::CodeAction {
+    let mut res = lsp_ext::CodeAction {
         title: assist.label.to_string(),
         group: assist.group.filter(|_| snap.config.code_action_group()).map(|gr| gr.0),
         kind: Some(code_action_kind(assist.id.1)),
         edit: None,
         is_preferred: None,
-        data: Some(lsp_ext::CodeActionData {
-            id: format!("{}:{}", assist.id.0, index.to_string()),
-            code_action_params,
-        }),
-    };
-    Ok(res)
-}
-
-pub(crate) fn resolved_code_action(
-    snap: &GlobalStateSnapshot,
-    assist: Assist,
-) -> Result<lsp_ext::CodeAction> {
-    let change = assist.source_change.unwrap();
-    let res = lsp_ext::CodeAction {
-        edit: Some(snippet_workspace_edit(snap, change)?),
-        title: assist.label.to_string(),
-        group: assist.group.filter(|_| snap.config.code_action_group()).map(|gr| gr.0),
-        kind: Some(code_action_kind(assist.id.1)),
-        is_preferred: None,
         data: None,
     };
+    match (assist.source_change, resolve_data) {
+        (Some(it), _) => res.edit = Some(snippet_workspace_edit(snap, it)?),
+        (None, Some((index, code_action_params))) => {
+            res.data = Some(lsp_ext::CodeActionData {
+                id: format!("{}:{}", assist.id.0, index.to_string()),
+                code_action_params,
+            });
+        }
+        (None, None) => {
+            stdx::never!("assist should always be resolved if client can't do lazy resolving")
+        }
+    };
     Ok(res)
 }
 

crates/rust-analyzer/tests/rust-analyzer/main.rs

@@ -340,7 +340,6 @@ fn main() {}
                 }
               ]
             },
-            "isPreferred": false,
             "kind": "quickfix",
             "title": "Create module"
         }]),
@@ -411,7 +410,6 @@ fn main() {{}}
                 }
               ]
             },
-            "isPreferred": false,
             "kind": "quickfix",
             "title": "Create module"
         }]),