fix(lsp): force correct media type detection from tsc (#20562)

Author: nayeemrmn

cli/lsp/tsc.rs

@@ -31,6 +31,7 @@ use crate::tsc::ResolveArgs;
 use crate::util::path::relative_specifier;
 use crate::util::path::specifier_to_file_path;
 
+use deno_ast::MediaType;
 use deno_core::anyhow::anyhow;
 use deno_core::error::custom_error;
 use deno_core::error::AnyError;
@@ -3167,7 +3168,8 @@ struct State {
   performance: Arc<Performance>,
   response: Option<Response>,
   state_snapshot: Arc<StateSnapshot>,
-  specifiers: HashMap<String, String>,
+  normalized_specifiers: HashMap<String, ModuleSpecifier>,
+  denormalized_specifiers: HashMap<ModuleSpecifier, String>,
   token: CancellationToken,
 }
 
@@ -3181,36 +3183,55 @@ impl State {
       performance,
       response: None,
       state_snapshot,
-      specifiers: HashMap::default(),
+      normalized_specifiers: HashMap::default(),
+      denormalized_specifiers: HashMap::default(),
       token: Default::default(),
     }
   }
 
-  /// If a normalized version of the specifier has been stored for tsc, this
-  /// will "restore" it for communicating back to the tsc language server,
-  /// otherwise it will just convert the specifier to a string.
-  fn denormalize_specifier(&self, specifier: &ModuleSpecifier) -> String {
-    let specifier_str = specifier.to_string();
-    self
-      .specifiers
-      .get(&specifier_str)
-      .unwrap_or(&specifier_str)
-      .to_string()
+  /// Convert the specifier to one compatible with tsc. Cache the resulting
+  /// mapping in case it needs to be reversed.
+  fn denormalize_specifier(&mut self, specifier: &ModuleSpecifier) -> String {
+    let original = specifier;
+    if let Some(specifier) = self.denormalized_specifiers.get(original) {
+      return specifier.to_string();
+    }
+    let mut specifier = original.to_string();
+    let media_type = MediaType::from_specifier(original);
+    // If the URL-inferred media type doesn't correspond to tsc's path-inferred
+    // media type, force it to be the same by appending an extension.
+    if MediaType::from_path(Path::new(specifier.as_str())) != media_type {
+      specifier += media_type.as_ts_extension();
+    }
+    if specifier != original.as_str() {
+      self
+        .normalized_specifiers
+        .insert(specifier.clone(), original.clone());
+    }
+    specifier
   }
 
-  /// In certain situations, tsc can request "invalid" specifiers and this will
-  /// normalize and memoize the specifier.
+  /// Convert the specifier from one compatible with tsc. Cache the resulting
+  /// mapping in case it needs to be reversed.
   fn normalize_specifier<S: AsRef<str>>(
     &mut self,
     specifier: S,
   ) -> Result<ModuleSpecifier, AnyError> {
-    let specifier_str = specifier.as_ref().replace(".d.ts.d.ts", ".d.ts");
-    if specifier_str != specifier.as_ref() {
+    let original = specifier.as_ref();
+    if let Some(specifier) = self.normalized_specifiers.get(original) {
+      return Ok(specifier.clone());
+    }
+    let specifier_str = original.replace(".d.ts.d.ts", ".d.ts");
+    let specifier = match ModuleSpecifier::parse(&specifier_str) {
+      Ok(s) => s,
+      Err(err) => return Err(err.into()),
+    };
+    if specifier.as_str() != original {
       self
-        .specifiers
-        .insert(specifier_str.clone(), specifier.as_ref().to_string());
+        .denormalized_specifiers
+        .insert(specifier.clone(), original.to_string());
     }
-    ModuleSpecifier::parse(&specifier_str).map_err(|err| err.into())
+    Ok(specifier)
   }
 
   fn get_asset_or_document(
@@ -3324,7 +3345,12 @@ fn op_resolve(
         resolved
           .into_iter()
           .map(|o| {
-            o.map(|(s, mt)| (s.to_string(), mt.as_ts_extension().to_string()))
+            o.map(|(s, mt)| {
+              (
+                state.denormalize_specifier(&s),
+                mt.as_ts_extension().to_string(),
+              )
+            })
           })
           .collect(),
       )
@@ -3861,7 +3887,7 @@ enum RequestMethod {
 }
 
 impl RequestMethod {
-  fn to_value(&self, state: &State, id: usize) -> Value {
+  fn to_value(&self, state: &mut State, id: usize) -> Value {
     match self {
       RequestMethod::Configure(config) => json!({
         "id": id,

cli/tests/integration/lsp_tests.rs

@@ -7847,6 +7847,40 @@ fn lsp_json_no_diagnostics() {
   client.shutdown();
 }
 
+#[test]
+fn lsp_json_import_with_query_string() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  temp_dir.write("data.json", r#"{"k": "v"}"#);
+  temp_dir.write(
+    "main.ts",
+    r#"
+      import data from "./data.json?1" with { type: "json" };
+      console.log(data);
+    "#,
+  );
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("data.json").unwrap(),
+      "languageId": "json",
+      "version": 1,
+      "text": temp_dir.read_to_string("data.json"),
+    }
+  }));
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("main.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": temp_dir.read_to_string("main.ts"),
+    }
+  }));
+  assert_eq!(diagnostics.all(), vec![]);
+  client.shutdown();
+}
+
 #[test]
 fn lsp_format_markdown() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
@@ -9198,7 +9232,7 @@ fn lsp_data_urls_with_jsx_compiler_option() {
         "end": { "line": 1, "character": 1 }
       }
     }, {
-      "uri": "deno:/ed0224c51f7e2a845dfc0941ed6959675e5e3e3d2a39b127f0ff569c1ffda8d8/data_url.ts",
+      "uri": "deno:/5c42b5916c4a3fb55be33fdb0c3b1f438639420592d150fca1b6dc043c1df3d9/data_url.ts",
       "range": {
         "start": { "line": 0, "character": 7 },
         "end": {"line": 0, "character": 14 },