fix(lsp): textDocument/references should respect includeDeclaration (#18496)

cli/lsp/code_lens.rs

@@ -297,70 +297,72 @@ async fn resolve_references_code_lens(
   data: CodeLensData,
   language_server: &language_server::Inner,
 ) -> Result<lsp::CodeLens, AnyError> {
-  let asset_or_document =
+  fn get_locations(
-    language_server.get_asset_or_document(&data.specifier)?;
+    maybe_referenced_symbols: Option<Vec<tsc::ReferencedSymbol>>,
-  let line_index = asset_or_document.line_index();
+    language_server: &language_server::Inner,
-  let req = tsc::RequestMethod::GetReferences((
+  ) -> Result<Vec<lsp::Location>, AnyError> {
-    data.specifier.clone(),
+    let symbols = match maybe_referenced_symbols {
-    line_index.offset_tsc(code_lens.range.start)?,
+      Some(symbols) => symbols,
-  ));
+      None => return Ok(Vec::new()),
-  let snapshot = language_server.snapshot();
+    };
-  let maybe_references: Option<Vec<tsc::ReferenceEntry>> =
-    language_server.ts_server.request(snapshot, req).await?;
-  if let Some(references) = maybe_references {
     let mut locations = Vec::new();
-    for reference in references {
+    for reference in symbols.iter().flat_map(|s| &s.references) {
       if reference.is_definition {
         continue;
       }
       let reference_specifier =
-        resolve_url(&reference.document_span.file_name)?;
+        resolve_url(&reference.entry.document_span.file_name)?;
       let asset_or_doc =
         language_server.get_asset_or_document(&reference_specifier)?;
       locations.push(
         reference
+          .entry
           .to_location(asset_or_doc.line_index(), &language_server.url_map),
       );
     }
-    let command = if !locations.is_empty() {
+    Ok(locations)
-      let title = if locations.len() > 1 {
-        format!("{} references", locations.len())
-      } else {
-        "1 reference".to_string()
-      };
-      lsp::Command {
-        title,
-        command: "deno.showReferences".to_string(),
-        arguments: Some(vec![
-          json!(data.specifier),
-          json!(code_lens.range.start),
-          json!(locations),
-        ]),
-      }
-    } else {
-      lsp::Command {
-        title: "0 references".to_string(),
-        command: "".to_string(),
-        arguments: None,
-      }
-    };
-    Ok(lsp::CodeLens {
-      range: code_lens.range,
-      command: Some(command),
-      data: None,
-    })
-  } else {
-    let command = lsp::Command {
-      title: "0 references".to_string(),
-      command: "".to_string(),
-      arguments: None,
-    };
-    Ok(lsp::CodeLens {
-      range: code_lens.range,
-      command: Some(command),
-      data: None,
-    })
   }
+
+  let asset_or_document =
+    language_server.get_asset_or_document(&data.specifier)?;
+  let line_index = asset_or_document.line_index();
+  let snapshot = language_server.snapshot();
+  let maybe_referenced_symbols = language_server
+    .ts_server
+    .find_references(
+      snapshot,
+      &data.specifier,
+      line_index.offset_tsc(code_lens.range.start)?,
+    )
+    .await?;
+  let locations = get_locations(maybe_referenced_symbols, language_server)?;
+  let title = if locations.len() == 1 {
+    "1 reference".to_string()
+  } else {
+    format!("{} references", locations.len())
+  };
+  let command = if locations.is_empty() {
+    lsp::Command {
+      title,
+      command: String::new(),
+      arguments: None,
+    }
+  } else {
+    lsp::Command {
+      title,
+      command: "deno.showReferences".to_string(),
+      arguments: Some(vec![
+        json!(data.specifier),
+        json!(code_lens.range.start),
+        json!(locations),
+      ]),
+    }
+  };
+  Ok(lsp::CodeLens {
+    range: code_lens.range,
+    command: Some(command),
+    data: None,
+  })
 }
 
 pub async fn resolve_code_lens(

cli/lsp/language_server.rs

@@ -1953,27 +1953,23 @@ impl Inner {
     let mark = self.performance.mark("references", Some(&params));
     let asset_or_doc = self.get_asset_or_document(&specifier)?;
     let line_index = asset_or_doc.line_index();
-    let req = tsc::RequestMethod::GetReferences((
+    let maybe_referenced_symbols = self
-      specifier.clone(),
-      line_index.offset_tsc(params.text_document_position.position)?,
-    ));
-    let maybe_references: Option<Vec<tsc::ReferenceEntry>> = self
       .ts_server
-      .request(self.snapshot(), req)
+      .find_references(
-      .await
+        self.snapshot(),
-      .map_err(|err| {
+        &specifier,
-        error!("Unable to get references from TypeScript: {}", err);
+        line_index.offset_tsc(params.text_document_position.position)?,
-        LspError::internal_error()
+      )
-      })?;
+      .await?;
 
-    if let Some(references) = maybe_references {
+    if let Some(symbols) = maybe_referenced_symbols {
       let mut results = Vec::new();
-      for reference in references {
+      for reference in symbols.iter().flat_map(|s| &s.references) {
         if !params.context.include_declaration && reference.is_definition {
           continue;
         }
         let reference_specifier =
-          resolve_url(&reference.document_span.file_name).unwrap();
+          resolve_url(&reference.entry.document_span.file_name).unwrap();
         let reference_line_index = if reference_specifier == specifier {
           line_index.clone()
         } else {
@@ -1981,8 +1977,11 @@ impl Inner {
             self.get_asset_or_document(&reference_specifier)?;
           asset_or_doc.line_index()
         };
-        results
+        results.push(
-          .push(reference.to_location(reference_line_index, &self.url_map));
+          reference
+            .entry
+            .to_location(reference_line_index, &self.url_map),
+        );
       }
 
       self.performance.measure(mark);

cli/lsp/tsc.rs

@@ -149,7 +149,28 @@ impl TsServer {
     if self.0.send((req, snapshot, tx, token)).is_err() {
       return Err(anyhow!("failed to send request to tsc thread"));
     }
-    rx.await?.map(|v| serde_json::from_value::<R>(v).unwrap())
+    let value = rx.await??;
+    Ok(serde_json::from_value::<R>(value)?)
+  }
+
+  // todo(dsherret): refactor the rest of the request methods to have
+  // methods to call on this struct, then make `RequestMethod` and
+  // friends internal
+
+  pub async fn find_references(
+    &self,
+    snapshot: Arc<StateSnapshot>,
+    specifier: &ModuleSpecifier,
+    position: u32,
+  ) -> Result<Option<Vec<ReferencedSymbol>>, LspError> {
+    let req = RequestMethod::FindReferences {
+      specifier: specifier.clone(),
+      position,
+    };
+    self.request(snapshot, req).await.map_err(|err| {
+      log::error!("Unable to get references from TypeScript: {}", err);
+      LspError::internal_error()
+    })
   }
 }
 
@@ -1688,10 +1709,31 @@ pub struct CombinedCodeActions {
 
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
-pub struct ReferenceEntry {
+pub struct ReferencedSymbol {
-  // is_write_access: bool,
+  pub definition: ReferencedSymbolDefinitionInfo,
+  pub references: Vec<ReferencedSymbolEntry>,
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ReferencedSymbolDefinitionInfo {
+  #[serde(flatten)]
+  pub definition_info: DefinitionInfo,
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ReferencedSymbolEntry {
   #[serde(default)]
   pub is_definition: bool,
+  #[serde(flatten)]
+  pub entry: ReferenceEntry,
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ReferenceEntry {
+  // is_write_access: bool,
   // is_in_string: Option<bool>,
   #[serde(flatten)]
   pub document_span: DocumentSpan,
@@ -3178,8 +3220,11 @@ pub enum RequestMethod {
   GetOutliningSpans(ModuleSpecifier),
   /// Return quick info at position (hover information).
   GetQuickInfo((ModuleSpecifier, u32)),
-  /// Get document references for a specific position.
+  /// Finds the document references for a specific position.
-  GetReferences((ModuleSpecifier, u32)),
+  FindReferences {
+    specifier: ModuleSpecifier,
+    position: u32,
+  },
   /// Get signature help items for a specific position.
   GetSignatureHelpItems((ModuleSpecifier, u32, SignatureHelpItemsOptions)),
   /// Get a selection range for a specific position.
@@ -3349,9 +3394,12 @@ impl RequestMethod {
         "specifier": state.denormalize_specifier(specifier),
         "position": position,
       }),
-      RequestMethod::GetReferences((specifier, position)) => json!({
+      RequestMethod::FindReferences {
+        specifier,
+        position,
+      } => json!({
         "id": id,
-        "method": "getReferences",
+        "method": "findReferences",
         "specifier": state.denormalize_specifier(specifier),
         "position": position,
       }),

cli/tests/integration/lsp_tests.rs

@@ -2367,16 +2367,32 @@ fn lsp_semantic_tokens() {
 fn lsp_code_lens() {
   let mut client = LspClientBuilder::new().build();
   client.initialize_default();
-  client.did_open(
+  client.did_open(json!({
-    json!({
+    "textDocument": {
-      "textDocument": {
+      "uri": "file:///a/file.ts",
-        "uri": "file:///a/file.ts",
+      "languageId": "typescript",
-        "languageId": "typescript",
+      "version": 1,
-        "version": 1,
+      "text": concat!(
-        "text": "class A {\n  a = \"a\";\n\n  b() {\n    console.log(this.a);\n  }\n\n  c() {\n    this.a = \"c\";\n  }\n}\n\nconst a = new A();\na.b();\n"
+        "class A {\n",
-      }
+        "  a = \"a\";\n",
-    }),
+        "\n",
-  );
+        "  b() {\n",
+        "    console.log(this.a);\n",
+        "  }\n",
+        "\n",
+        "  c() {\n",
+        "    this.a = \"c\";\n",
+        "  }\n",
+        "}\n",
+        "\n",
+        "const a = new A();\n",
+        "a.b();\n",
+        "const b = 2;\n",
+        "const c = 3;\n",
+        "c; c;",
+      ),
+    }
+  }));
   let res = client.write_request(
     "textDocument/codeLens",
     json!({
@@ -2428,18 +2444,12 @@ fn lsp_code_lens() {
         "end": { "line": 0, "character": 7 }
       },
       "command": {
-        "title": "2 references",
+        "title": "1 reference",
         "command": "deno.showReferences",
         "arguments": [
           "file:///a/file.ts",
           { "line": 0, "character": 6 },
           [{
-            "uri": "file:///a/file.ts",
-            "range": {
-              "start": { "line": 0, "character": 6 },
-              "end": { "line": 0, "character": 7 }
-            }
-          }, {
             "uri": "file:///a/file.ts",
             "range": {
               "start": { "line": 12, "character": 14 },
@@ -2450,6 +2460,80 @@ fn lsp_code_lens() {
       }
     })
   );
+
+  // 0 references
+  let res = client.write_request(
+    "codeLens/resolve",
+    json!({
+      "range": {
+        "start": { "line": 14, "character": 6 },
+        "end": { "line": 14, "character": 7 }
+      },
+      "data": {
+        "specifier": "file:///a/file.ts",
+        "source": "references"
+      }
+    }),
+  );
+  assert_eq!(
+    res,
+    json!({
+      "range": {
+        "start": { "line": 14, "character": 6 },
+        "end": { "line": 14, "character": 7 }
+      },
+      "command": {
+        "title": "0 references",
+        "command": "",
+      }
+    })
+  );
+
+  // 2 references
+  let res = client.write_request(
+    "codeLens/resolve",
+    json!({
+      "range": {
+        "start": { "line": 15, "character": 6 },
+        "end": { "line": 15, "character": 7 }
+      },
+      "data": {
+        "specifier": "file:///a/file.ts",
+        "source": "references"
+      }
+    }),
+  );
+  assert_eq!(
+    res,
+    json!({
+      "range": {
+        "start": { "line": 15, "character": 6 },
+        "end": { "line": 15, "character": 7 }
+      },
+      "command": {
+        "title": "2 references",
+        "command": "deno.showReferences",
+        "arguments": [
+          "file:///a/file.ts",
+          { "line": 15, "character": 6 },
+          [{
+            "uri": "file:///a/file.ts",
+            "range": {
+              "start": { "line": 16, "character": 0 },
+              "end": { "line": 16, "character": 1 }
+            }
+          },{
+            "uri": "file:///a/file.ts",
+            "range": {
+              "start": { "line": 16, "character": 3 },
+              "end": { "line": 16, "character": 4 }
+            }
+          }]
+        ]
+      }
+    })
+  );
+
   client.shutdown();
 }
 
@@ -3091,6 +3175,114 @@ fn lsp_nav_tree_updates() {
   client.shutdown();
 }
 
+#[test]
+fn lsp_find_references() {
+  let mut client = LspClientBuilder::new().build();
+  client.initialize_default();
+  client.did_open(json!({
+    "textDocument": {
+      "uri": "file:///a/mod.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"export const a = 1;\nconst b = 2;"#
+    }
+  }));
+  client.did_open(json!({
+    "textDocument": {
+      "uri": "file:///a/mod.test.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"import { a } from './mod.ts'; console.log(a);"#
+    }
+  }));
+
+  // test without including the declaration
+  let res = client.write_request(
+    "textDocument/references",
+    json!({
+      "textDocument": {
+        "uri": "file:///a/mod.ts",
+      },
+      "position": { "line": 0, "character": 13 },
+      "context": {
+        "includeDeclaration": false
+      }
+    }),
+  );
+
+  assert_eq!(
+    res,
+    json!([{
+      "uri": "file:///a/mod.test.ts",
+      "range": {
+        "start": { "line": 0, "character": 9 },
+        "end": { "line": 0, "character": 10 }
+      }
+    }, {
+      "uri": "file:///a/mod.test.ts",
+      "range": {
+        "start": { "line": 0, "character": 42 },
+        "end": { "line": 0, "character": 43 }
+      }
+    }])
+  );
+
+  // test with including the declaration
+  let res = client.write_request(
+    "textDocument/references",
+    json!({
+      "textDocument": {
+        "uri": "file:///a/mod.ts",
+      },
+      "position": { "line": 0, "character": 13 },
+      "context": {
+        "includeDeclaration": true
+      }
+    }),
+  );
+
+  assert_eq!(
+    res,
+    json!([{
+      "uri": "file:///a/mod.ts",
+      "range": {
+        "start": { "line": 0, "character": 13 },
+        "end": { "line": 0, "character": 14 }
+      }
+    }, {
+      "uri": "file:///a/mod.test.ts",
+      "range": {
+        "start": { "line": 0, "character": 9 },
+        "end": { "line": 0, "character": 10 }
+      }
+    }, {
+      "uri": "file:///a/mod.test.ts",
+      "range": {
+        "start": { "line": 0, "character": 42 },
+        "end": { "line": 0, "character": 43 }
+      }
+    }])
+  );
+
+  // test 0 references
+  let res = client.write_request(
+    "textDocument/references",
+    json!({
+      "textDocument": {
+        "uri": "file:///a/mod.ts",
+      },
+      "position": { "line": 1, "character": 6 },
+      "context": {
+        "includeDeclaration": false
+      }
+    }),
+  );
+
+  assert_eq!(res, json!(null)); // seems it always returns null for this, which is ok
+
+  client.shutdown();
+}
+
 #[test]
 fn lsp_signature_help() {
   let mut client = LspClientBuilder::new().build();

cli/tests/integration/repl_tests.rs

@@ -905,7 +905,7 @@ fn package_json_uncached_no_error() {
   );
   test_context.new_command().with_pty(|mut console| {
     console.write_line("console.log(123 + 456);");
-    console.expect("579");
+    console.expect_all(&["579", "undefined"]);
     assert_not_contains!(
       console.all_output(),
       "Could not set npm package requirements",
@@ -914,7 +914,7 @@ fn package_json_uncached_no_error() {
     // should support getting the package now though
     console
       .write_line("import { getValue, setValue } from '@denotest/esm-basic';");
-    console.expect("undefined");
+    console.expect_all(&["undefined", "Initialize"]);
     console.write_line("setValue(12 + 30);");
     console.expect("undefined");
     console.write_line("getValue()");

cli/tsc/99_main_compiler.js

@@ -1121,10 +1121,10 @@ delete Object.prototype.__proto__;
           ),
         );
       }
-      case "getReferences": {
+      case "findReferences": {
         return respond(
           id,
-          languageService.getReferencesAtPosition(
+          languageService.findReferences(
             request.specifier,
             request.position,
           ),

cli/tsc/compiler.d.ts

@@ -75,7 +75,7 @@ declare global {
     | GetNavigationTree
     | GetOutliningSpans
     | GetQuickInfoRequest
-    | GetReferencesRequest
+    | FindReferencesRequest
     | GetSignatureHelpItemsRequest
     | GetSmartSelectionRange
     | GetSupportedCodeFixes
@@ -212,8 +212,8 @@ declare global {
     position: number;
   }
 
-  interface GetReferencesRequest extends BaseLanguageServerRequest {
+  interface FindReferencesRequest extends BaseLanguageServerRequest {
-    method: "getReferences";
+    method: "findReferences";
     specifier: string;
     position: number;
   }