refactor(lsp): refactor completions and add tests (denoland#9789)

Author: kitsonk

cli/lsp/capabilities.rs

@@ -55,7 +55,12 @@ pub fn server_capabilities(
     )),
     hover_provider: Some(HoverProviderCapability::Simple(true)),
     completion_provider: Some(CompletionOptions {
-      all_commit_characters: None,
+      all_commit_characters: Some(vec![
+        ".".to_string(),
+        ",".to_string(),
+        ";".to_string(),
+        "(".to_string(),
+      ]),
       trigger_characters: Some(vec![
         ".".to_string(),
         "\"".to_string(),
@@ -66,7 +71,7 @@ pub fn server_capabilities(
         "<".to_string(),
         "#".to_string(),
       ]),
-      resolve_provider: None,
+      resolve_provider: Some(true),
       work_done_progress_options: WorkDoneProgressOptions {
         work_done_progress: None,
       },
@@ -77,7 +82,7 @@ pub fn server_capabilities(
         "(".to_string(),
         "<".to_string(),
       ]),
-      retrigger_characters: None,
+      retrigger_characters: Some(vec![")".to_string()]),
       work_done_progress_options: WorkDoneProgressOptions {
         work_done_progress: None,
       },

cli/lsp/config.rs

@@ -15,7 +15,7 @@ pub struct ClientCapabilities {
   pub workspace_did_change_watched_files: bool,
 }
 
-#[derive(Debug, Default, Clone, Deserialize)]
+#[derive(Debug, Clone, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CodeLensSettings {
   /// Flag for providing implementation code lenses.
@@ -30,13 +30,50 @@ pub struct CodeLensSettings {
   pub references_all_functions: bool,
 }
 
+impl Default for CodeLensSettings {
+  fn default() -> Self {
+    Self {
+      implementations: false,
+      references: false,
+      references_all_functions: false,
+    }
+  }
+}
+
+#[derive(Debug, Clone, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct CompletionSettings {
+  #[serde(default)]
+  pub complete_function_calls: bool,
+  #[serde(default)]
+  pub names: bool,
+  #[serde(default)]
+  pub paths: bool,
+  #[serde(default)]
+  pub auto_imports: bool,
+}
+
+impl Default for CompletionSettings {
+  fn default() -> Self {
+    Self {
+      complete_function_calls: false,
+      names: true,
+      paths: true,
+      auto_imports: true,
+    }
+  }
+}
+
 #[derive(Debug, Default, Clone, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct WorkspaceSettings {
   pub enable: bool,
   pub config: Option<String>,
   pub import_map: Option<String>,
-  pub code_lens: Option<CodeLensSettings>,
+  #[serde(default)]
+  pub code_lens: CodeLensSettings,
+  #[serde(default)]
+  pub suggest: CompletionSettings,
 
   #[serde(default)]
   pub lint: bool,
@@ -48,36 +85,7 @@ impl WorkspaceSettings {
   /// Determine if any code lenses are enabled at all.  This allows short
   /// circuiting when there are no code lenses enabled.
   pub fn enabled_code_lens(&self) -> bool {
-    if let Some(code_lens) = &self.code_lens {
-      // This should contain all the "top level" code lens references
-      code_lens.implementations || code_lens.references
-    } else {
-      false
-    }
-  }
-
-  pub fn enabled_code_lens_implementations(&self) -> bool {
-    if let Some(code_lens) = &self.code_lens {
-      code_lens.implementations
-    } else {
-      false
-    }
-  }
-
-  pub fn enabled_code_lens_references(&self) -> bool {
-    if let Some(code_lens) = &self.code_lens {
-      code_lens.references
-    } else {
-      false
-    }
-  }
-
-  pub fn enabled_code_lens_references_all_functions(&self) -> bool {
-    if let Some(code_lens) = &self.code_lens {
-      code_lens.references_all_functions
-    } else {
-      false
-    }
+    self.code_lens.implementations || self.code_lens.references
   }
 }
 

cli/lsp/language_server.rs

@@ -917,7 +917,7 @@ impl Inner {
       let mut code_lenses = cl.borrow_mut();
 
       // TSC Implementations Code Lens
-      if self.config.settings.enabled_code_lens_implementations() {
+      if self.config.settings.code_lens.implementations {
         let source = CodeLensSource::Implementations;
         match i.kind {
           tsc::ScriptElementKind::InterfaceElement => {
@@ -941,7 +941,7 @@ impl Inner {
       }
 
       // TSC References Code Lens
-      if self.config.settings.enabled_code_lens_references() {
+      if self.config.settings.code_lens.references {
         let source = CodeLensSource::References;
         if let Some(parent) = &mp {
           if parent.kind == tsc::ScriptElementKind::EnumElement {
@@ -950,11 +950,7 @@ impl Inner {
         }
         match i.kind {
           tsc::ScriptElementKind::FunctionElement => {
-            if self
-              .config
-              .settings
-              .enabled_code_lens_references_all_functions()
-            {
+            if self.config.settings.code_lens.references_all_functions {
               code_lenses.push(i.to_code_lens(
                 &line_index,
                 &specifier,
@@ -1358,7 +1354,6 @@ impl Inner {
     let specifier = self
       .url_map
       .normalize_url(&params.text_document_position.text_document.uri);
-    // TODO(lucacasonato): handle error correctly
     let line_index =
       if let Some(line_index) = self.get_line_index_sync(&specifier) {
         line_index
@@ -1368,13 +1363,22 @@ impl Inner {
           specifier
         )));
       };
+    let trigger_character = if let Some(context) = &params.context {
+      context.trigger_character.clone()
+    } else {
+      None
+    };
+    let position =
+      line_index.offset_tsc(params.text_document_position.position)?;
     let req = tsc::RequestMethod::GetCompletions((
-      specifier,
-      line_index.offset_tsc(params.text_document_position.position)?,
-      tsc::UserPreferences {
-        // TODO(lucacasonato): enable this. see https://github.com/denoland/deno/pull/8651
-        include_completions_with_insert_text: Some(false),
-        ..Default::default()
+      specifier.clone(),
+      position,
+      tsc::GetCompletionsAtPositionOptions {
+        user_preferences: tsc::UserPreferences {
+          include_completions_with_insert_text: Some(true),
+          ..Default::default()
+        },
+        trigger_character,
       },
     ));
     let maybe_completion_info: Option<tsc::CompletionInfo> = self
@@ -1387,7 +1391,12 @@ impl Inner {
       })?;
 
     if let Some(completions) = maybe_completion_info {
-      let results = completions.into_completion_response(&line_index);
+      let results = completions.as_completion_response(
+        &line_index,
+        &self.config.settings.suggest,
+        &specifier,
+        position,
+      );
       self.performance.measure(mark);
       Ok(Some(results))
     } else {
@@ -1396,6 +1405,47 @@ impl Inner {
     }
   }
 
+  async fn completion_resolve(
+    &mut self,
+    params: CompletionItem,
+  ) -> LspResult<CompletionItem> {
+    let mark = self.performance.mark("completion_resolve");
+    if let Some(data) = &params.data {
+      let data: tsc::CompletionItemData = serde_json::from_value(data.clone())
+        .map_err(|err| {
+          error!("{}", err);
+          LspError::invalid_params(
+            "Could not decode data field of completion item.",
+          )
+        })?;
+      let req = tsc::RequestMethod::GetCompletionDetails(data.into());
+      let maybe_completion_info: Option<tsc::CompletionEntryDetails> = self
+        .ts_server
+        .request(self.snapshot(), req)
+        .await
+        .map_err(|err| {
+          error!("Unable to get completion info from TypeScript: {}", err);
+          LspError::internal_error()
+        })?;
+      if let Some(completion_info) = maybe_completion_info {
+        let completion_item = completion_info.as_completion_item(&params);
+        self.performance.measure(mark);
+        Ok(completion_item)
+      } else {
+        error!(
+          "Received an undefined response from tsc for completion details."
+        );
+        self.performance.measure(mark);
+        Ok(params)
+      }
+    } else {
+      self.performance.measure(mark);
+      Err(LspError::invalid_params(
+        "The completion item is missing the data field.",
+      ))
+    }
+  }
+
   async fn goto_implementation(
     &mut self,
     params: GotoImplementationParams,
@@ -1715,6 +1765,13 @@ impl lspower::LanguageServer for LanguageServer {
     self.0.lock().await.completion(params).await
   }
 
+  async fn completion_resolve(
+    &self,
+    params: CompletionItem,
+  ) -> LspResult<CompletionItem> {
+    self.0.lock().await.completion_resolve(params).await
+  }
+
   async fn goto_implementation(
     &self,
     params: GotoImplementationParams,
@@ -2740,6 +2797,58 @@ mod tests {
     harness.run().await;
   }
 
+  #[derive(Deserialize)]
+  struct CompletionResult {
+    pub result: Option<CompletionResponse>,
+  }
+
+  #[tokio::test]
+  async fn test_completions() {
+    let mut harness = LspTestHarness::new(vec![
+      ("initialize_request.json", LspResponse::RequestAny),
+      ("initialized_notification.json", LspResponse::None),
+      ("did_open_notification_completions.json", LspResponse::None),
+      (
+        "completion_request.json",
+        LspResponse::RequestAssert(|value| {
+          let response: CompletionResult =
+            serde_json::from_value(value).unwrap();
+          let result = response.result.unwrap();
+          match result {
+            CompletionResponse::List(list) => {
+              // there should be at least 90 completions for `Deno.`
+              assert!(list.items.len() > 90);
+            }
+            _ => panic!("unexpected result"),
+          }
+        }),
+      ),
+      (
+        "completion_resolve_request.json",
+        LspResponse::Request(
+          4,
+          json!({
+            "label": "build",
+            "kind": 6,
+            "detail": "const Deno.build: {\n    target: string;\n    arch: \"x86_64\";\n    os: \"darwin\" | \"linux\" | \"windows\";\n    vendor: string;\n    env?: string | undefined;\n}",
+            "documentation": {
+              "kind": "markdown",
+              "value": "Build related information."
+            },
+            "sortText": "1",
+            "insertTextFormat": 1,
+          }),
+        ),
+      ),
+      (
+        "shutdown_request.json",
+        LspResponse::Request(3, json!(null)),
+      ),
+      ("exit_notification.json", LspResponse::None),
+    ]);
+    harness.run().await;
+  }
+
   #[derive(Deserialize)]
   struct PerformanceAverages {
     averages: Vec<PerformanceAverage>,