Repository: denoland/deno
Pull request: fix(lsp): improve renaming a file with bytes or text imports

Also actually goes to the file now when doing "go to definition" on the specifier.

Changed files:
filename: cli/lsp/documents.rs
patch: @@ -1596,32 +1596,22 @@ impl DocumentModules {
         true => ResolutionMode::Require,
         false => ResolutionMode::Import,
       };
-      if raw_specifier.starts_with("asset:") {
+      let result = if raw_specifier.starts_with("asset:") {
         if let Ok(specifier) = resolve_url(raw_specifier) {
           let media_type = MediaType::from_specifier(&specifier);
-          results.push(Some((specifier, media_type)));
+          Some((specifier, media_type))
         } else {
-          results.push(None);
+          None
         }
       } else if let Some(dep) =
         dependencies.as_ref().and_then(|d| d.get(raw_specifier))
       {
         if let Some(specifier) = dep.maybe_type.maybe_specifier() {
-          results.push(self.resolve_dependency(
-            specifier,
-            referrer,
-            resolution_mode,
-            scope,
-          ));
+          self.resolve_dependency(specifier, referrer, resolution_mode, scope)
         } else if let Some(specifier) = dep.maybe_code.maybe_specifier() {
-          results.push(self.resolve_dependency(
-            specifier,
-            referrer,
-            resolution_mode,
-            scope,
-          ));
+          self.resolve_dependency(specifier, referrer, resolution_mode, scope)
         } else {
-          results.push(None);
+          None
         }
       } else {
         match scoped_resolver.as_cli_resolver().resolve(
@@ -1631,19 +1621,16 @@ impl DocumentModules {
           resolution_mode,
           NodeResolutionKind::Types,
         ) {
-          Ok(specifier) => {
-            results.push(self.resolve_dependency(
-              &specifier,
-              referrer,
-              resolution_mode,
-              scope,
-            ));
-          }
-          _ => {
-            results.push(None);
-          }
+          Ok(specifier) => self.resolve_dependency(
+            &specifier,
+            referrer,
+            resolution_mode,
+            scope,
+          ),
+          _ => None,
         }
-      }
+      };
+      results.push(result);
     }
     results
   }

filename: cli/lsp/tsc.rs
patch: @@ -4569,7 +4569,7 @@ impl TscSpecifierMap {
     }
     // If the module's 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 {
+    if MediaType::from_path(Path::new(&specifier)) != media_type {
       specifier += media_type.as_ts_extension();
     }
     if specifier != original.as_str() {
@@ -4733,26 +4733,38 @@ fn op_load<'s>(
     .performance
     .mark_with_args("tsc.op.op_load", specifier);
   let specifier = state.specifier_map.normalize(specifier)?;
-  let module = if specifier.as_str() == MISSING_DEPENDENCY_SPECIFIER {
-    None
-  } else {
-    state.get_module(&specifier)
-  };
-  let maybe_load_response = module.as_ref().map(|m| {
-    let data = if m.media_type == MediaType::Json && m.text.len() > 10_000_000 {
-      // VSCode's TS server types large JSON files this way.
-      DocumentText::Static("{}\n")
+  let maybe_load_response =
+    if let Some(source) = crate::tsc::load_raw_import_source(&specifier) {
+      Some(LoadResponse {
+        data: DocumentText::Static(source),
+        script_kind: crate::tsc::as_ts_script_kind(MediaType::TypeScript),
+        version: Some("1".to_string()),
+        is_cjs: false,
+        is_classic_script: false,
+      })
     } else {
-      m.text.clone()
+      let module = if specifier.as_str() == MISSING_DEPENDENCY_SPECIFIER {
+        None
+      } else {
+        state.get_module(&specifier)
+      };
+      module.as_ref().map(|m| {
+        let data =
+          if m.media_type == MediaType::Json && m.text.len() > 10_000_000 {
+            // VSCode's TS server types large JSON files this way.
+            DocumentText::Static("{}\n")
+          } else {
+            m.text.clone()
+          };
+        LoadResponse {
+          data,
+          script_kind: crate::tsc::as_ts_script_kind(m.media_type),
+          version: state.script_version(&specifier),
+          is_cjs: m.resolution_mode == ResolutionMode::Require,
+          is_classic_script: m.notebook_uri.is_some(),
+        }
+      })
     };
-    LoadResponse {
-      data,
-      script_kind: crate::tsc::as_ts_script_kind(m.media_type),
-      version: state.script_version(&specifier),
-      is_cjs: m.resolution_mode == ResolutionMode::Require,
-      is_classic_script: m.notebook_uri.is_some(),
-    }
-  });
   let serialized = serde_v8::to_v8(scope, maybe_load_response)?;
   state.performance.measure(mark);
   Ok(serialized)

filename: cli/tsc/97_ts_host.js
patch: @@ -600,31 +600,21 @@ const hostImpl = {
   ) {
     const specifiers = moduleLiterals.map((literal) => {
       const rawKind = getModuleLiteralImportKind(literal);
+      let lookupSpecifier = literal.text;
       if (rawKind != null) {
-        // hack to get the specifier keyed differently until
-        // https://github.com/microsoft/TypeScript/issues/61941 is resolved
-        if (!literal.text.includes("?")) {
-          literal.text += `?${rawKind}`;
-        } else if (
-          !literal.text.includes(`?${rawKind}`) &&
-          !literal.text.includes(`&${rawKind}`)
-        ) {
-          literal.text += `&${rawKind}`;
+        if ((/** @type any */ (literal)).__originalText == null) {
+          (/** @type any */ (literal)).__originalText = literal.text;
+          literal.text = appendRawImportFragment(literal.text, rawKind);
         }
-        return [
-          false,
-          rawKind === "text"
-            ? "asset:///text_import.d.ts"
-            : "asset:///bytes_import.d.ts",
-        ];
+        lookupSpecifier = (/** @type any */ (literal)).__originalText;
       }
       return [
         ts.getModeForUsageLocation(
           containingSourceFile,
           literal,
           compilerOptions,
         ) === ts.ModuleKind.CommonJS,
-        literal.text,
+        lookupSpecifier,
       ];
     });
     if (logDebug) {
@@ -639,17 +629,29 @@ const hostImpl = {
     );
     if (resolved) {
       /** @type {Array<ts.ResolvedModuleWithFailedLookupLocations>} */
-      const result = resolved.map((item) => {
-        if (item && item[1]) {
-          const [resolvedFileName, extension] = item;
-          return {
-            resolvedModule: {
+      const result = resolved.map((item, i) => {
+        if (item) {
+          let [resolvedFileName, extension] = item;
+          // hack to get the specifier keyed differently until
+          // https://github.com/microsoft/TypeScript/issues/61941 is resolved
+          const rawKind = getModuleLiteralImportKind(moduleLiterals[i]);
+          if (rawKind != null) {
+            resolvedFileName = appendRawImportFragment(
               resolvedFileName,
-              extension,
-              // todo(dsherret): we should probably be setting this
-              isExternalLibraryImport: false,
-            },
-          };
+              rawKind,
+            );
+            extension = ts.Extension.Ts;
+          }
+          if (extension) {
+            return {
+              resolvedModule: {
+                resolvedFileName,
+                extension,
+                // todo(dsherret): we should probably be setting this
+                isExternalLibraryImport: false,
+              },
+            };
+          }
         }
         return {
           resolvedModule: undefined,
@@ -992,6 +994,24 @@ function getModuleLiteralImportKind(node) {
   }
 }
 
+/**
+ * @param {string} specifier
+ * @param {"bytes" | "text"} rawKind
+ */
+function appendRawImportFragment(specifier, rawKind) {
+  const fragmentIndex = specifier.indexOf("#");
+  if (fragmentIndex === -1) {
+    specifier += `#denoRawImport=${rawKind}.ts`;
+  } else if (
+    !specifier.substring(fragmentIndex).includes(
+      `denoRawImport=${rawKind}.ts`,
+    )
+  ) {
+    specifier += `&denoRawImport=${rawKind}.ts`;
+  }
+  return specifier;
+}
+
 /** @param {ts.ImportAttribute} node */
 function getRawImportAttributeValue(node) {
   if (!isStrOrIdentWithText(node.name, "type")) {

filename: cli/tsc/mod.rs
patch: @@ -311,24 +311,6 @@ pub static LAZILY_LOADED_STATIC_ASSETS: Lazy<
         ),
       },
     ),
-    (
-      "text_import.d.ts",
-      StaticAsset {
-        is_lib: false,
-        source: StaticAssetSource::Uncompressed(
-          "export const data: string;\nexport default data;\n",
-        ),
-      },
-    ),
-    (
-      "bytes_import.d.ts",
-      StaticAsset {
-        is_lib: false,
-        source: StaticAssetSource::Uncompressed(
-          "const data: Uint8Array<ArrayBuffer>;\nexport default data;\n",
-        ),
-      },
-    ),
   ])
 });
 
@@ -740,6 +722,13 @@ fn op_load_inner(
     hash = get_maybe_hash(maybe_source, state.hash_data);
     media_type = MediaType::from_str(load_specifier);
     maybe_source.map(FastString::from_static)
+  } else if let Some(source) = load_raw_import_source(&specifier) {
+    return Ok(Some(LoadResponse {
+      data: FastString::from_static(source),
+      version: Some("1".to_string()),
+      script_kind: as_ts_script_kind(MediaType::TypeScript),
+      is_cjs: false,
+    }));
   } else {
     let specifier = if let Some(remapped_specifier) =
       state.maybe_remapped_specifier(load_specifier)
@@ -838,6 +827,43 @@ fn op_load_inner(
   }))
 }
 
+pub fn load_raw_import_source(specifier: &Url) -> Option<&'static str> {
+  let raw_import = get_specifier_raw_import(specifier)?;
+  let source = match raw_import {
+    RawImportKind::Bytes => {
+      "const data: Uint8Array<ArrayBuffer>;\nexport default data;\n"
+    }
+    RawImportKind::Text => "export const data: string;\nexport default data;\n",
+  };
+  Some(source)
+}
+
+enum RawImportKind {
+  Bytes,
+  Text,
+}
+
+/// We store the raw import kind in the fragment of the Url
+/// like `#denoRawImport=text`. This is necessary because
+/// TypeScript can't handle different modules at the same
+/// specifier.
+fn get_specifier_raw_import(specifier: &Url) -> Option<RawImportKind> {
+  // this is purposefully relaxed about matching in order to keep the
+  // code less complex. If someone is doing something to cause this to
+  // incorrectly match then they most likely deserve the bug they sought.
+  let fragment = specifier.fragment()?;
+  let key_text = "denoRawImport=";
+  let raw_import_index = fragment.find(key_text)?;
+  let remaining = &fragment[raw_import_index + key_text.len()..];
+  if remaining.starts_with("text") {
+    Some(RawImportKind::Text)
+  } else if remaining.starts_with("bytes") {
+    Some(RawImportKind::Bytes)
+  } else {
+    None
+  }
+}
+
 #[derive(Debug, Error, deno_error::JsError)]
 pub enum ResolveError {
   #[class(inherit)]

filename: tests/integration/lsp_tests.rs
patch: @@ -18953,3 +18953,243 @@ fn lsp_import_json_module_import_attribute_variations() {
     }])
   );
 }
+
+#[test]
+#[timeout(300_000)]
+fn lsp_import_raw_imports() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir().path();
+  temp_dir.join("deno.json").write_json(&json!({
+    "unstable": ["raw-imports"]
+  }));
+  temp_dir.join("data.txt").write("");
+  temp_dir
+    .join("hello.ts")
+    .write("export function hello() { return 'Hello'; }\n");
+  let text = r#"import { hello } from "./hello.ts";
+import helloBytes from "./hello.ts" with { type: "bytes" };
+import helloText from "./hello.ts" with { type: "text" };
+import dataBytes from "./data.txt" with { type: "bytes" };
+import dataText from "./data.txt" with { type: "text" };
+
+const { default: dynamicHelloBytes } = await import("./hello.ts", {
+  with: { "type": "bytes" },
+});
+const { default: dynamicHelloText } = await import("./hello.ts", {
+  with: { "type": "text" },
+});
+const { default: dynamicDataBytes } = await import("./data.txt", {
+  with: { "type": "bytes" },
+});
+const { default: dynamicDataText } = await import("./data.txt", {
+  with: { "type": "text" },
+});
+
+let validBytes: Uint8Array<ArrayBuffer>;
+let validText: string;
+
+validBytes = helloBytes;
+validBytes = dataBytes;
+validBytes = dynamicHelloBytes;
+validBytes = dynamicDataBytes;
+
+validText = helloText;
+validText = hello();
+validText = dataText;
+validText = dynamicHelloText;
+validText = dynamicDataText;
+
+let invalid: number;
+invalid = helloBytes;
+invalid = dataBytes;
+invalid = dynamicHelloBytes;
+invalid = dynamicDataBytes;
+invalid = helloText;
+invalid = hello();
+invalid = dataText;
+invalid = dynamicHelloText;
+invalid = dynamicDataText;
+
+console.log(invalid, validBytes, validText);
+"#;
+  temp_dir.join("main.ts").write(text);
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.join("main.ts").uri_file(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": text,
+    }
+  }));
+  assert_eq!(
+    json!(diagnostics.all()),
+    json!([
+      {
+      "range": {
+        "start": {
+          "line": 34,
+          "character": 0
+        },
+        "end": {
+          "line": 34,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'Uint8Array<ArrayBuffer>' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 35,
+          "character": 0
+        },
+        "end": {
+          "line": 35,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'Uint8Array<ArrayBuffer>' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 36,
+          "character": 0
+        },
+        "end": {
+          "line": 36,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'Uint8Array<ArrayBuffer>' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 37,
+          "character": 0
+        },
+        "end": {
+          "line": 37,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'Uint8Array<ArrayBuffer>' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 38,
+          "character": 0
+        },
+        "end": {
+          "line": 38,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'string' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 39,
+          "character": 0
+        },
+        "end": {
+          "line": 39,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'string' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 40,
+          "character": 0
+        },
+        "end": {
+          "line": 40,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'string' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 41,
+          "character": 0
+        },
+        "end": {
+          "line": 41,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'string' is not assignable to type 'number'."
+    },
+    {
+      "range": {
+        "start": {
+          "line": 42,
+          "character": 0
+        },
+        "end": {
+          "line": 42,
+          "character": 7
+        }
+      },
+      "severity": 1,
+      "code": 2322,
+      "source": "deno-ts",
+      "message": "Type 'string' is not assignable to type 'number'."
+    }
+    ])
+  );
+
+  // ensure the specifiers aren't modified when renaming
+  let res = client.write_request(
+    "workspace/willRenameFiles",
+    json!({
+      "files": [
+        {
+          "oldUri": temp_dir.join("main.ts").uri_file(),
+          "newUri": temp_dir.join("renamed.ts").uri_file(),
+        },
+      ],
+    }),
+  );
+  assert_eq!(
+    res,
+    json!({
+      "documentChanges": [],
+    }),
+  );
+  client.shutdown();
+}