Repository: denoland/deno
Pull request: fix(lsp): move sloppy import resolution from loader to resolver

Fixes #23745
Fixes #23197

Moves sloppy import resolution from the loader to the resolver.

Also adds some test helper functions to make the lsp tests less verbose

Changed files:
filename: cli/lsp/documents.rs
patch: @@ -13,9 +13,7 @@ use super::tsc::AssetDocument;
 use crate::cache::HttpCache;
 use crate::graph_util::CliJsrUrlProvider;
 use crate::lsp::logging::lsp_warn;
-use crate::resolver::SloppyImportsFsEntry;
-use crate::resolver::SloppyImportsResolution;
-use crate::resolver::SloppyImportsResolver;
+use deno_graph::source::Resolver;
 use deno_runtime::fs_util::specifier_to_file_path;
 
 use dashmap::DashMap;
@@ -390,7 +388,7 @@ impl Document {
               d.with_new_resolver(
                 s,
                 &CliJsrUrlProvider,
-                Some(graph_resolver),
+                Some(&graph_resolver),
                 Some(npm_resolver),
               ),
             )
@@ -400,7 +398,7 @@ impl Document {
       maybe_types_dependency = self.maybe_types_dependency.as_ref().map(|d| {
         Arc::new(d.with_new_resolver(
           &CliJsrUrlProvider,
-          Some(graph_resolver),
+          Some(&graph_resolver),
           Some(npm_resolver),
         ))
       });
@@ -854,8 +852,6 @@ pub struct Documents {
   /// Gets if any document had a node: specifier such that a @types/node package
   /// should be injected.
   has_injected_types_node_package: bool,
-  /// If --unstable-sloppy-imports is enabled.
-  unstable_sloppy_imports: bool,
 }
 
 impl Documents {
@@ -869,7 +865,6 @@ impl Documents {
       resolver: Default::default(),
       npm_specifier_reqs: Default::default(),
       has_injected_types_node_package: false,
-      unstable_sloppy_imports: false,
     }
   }
 
@@ -996,54 +991,17 @@ impl Documents {
     &self,
     specifier: &ModuleSpecifier,
   ) -> Option<ModuleSpecifier> {
-    if self.unstable_sloppy_imports && specifier.scheme() == "file" {
-      Some(
-        self
-          .resolve_unstable_sloppy_import(specifier)
-          .into_specifier()
-          .into_owned(),
-      )
+    let specifier = if let Ok(jsr_req_ref) =
+      JsrPackageReqReference::from_specifier(specifier)
+    {
+      Cow::Owned(self.resolver.jsr_to_registry_url(&jsr_req_ref)?)
     } else {
-      let specifier = if let Ok(jsr_req_ref) =
-        JsrPackageReqReference::from_specifier(specifier)
-      {
-        Cow::Owned(self.resolver.jsr_to_registry_url(&jsr_req_ref)?)
-      } else {
-        Cow::Borrowed(specifier)
-      };
-      if !DOCUMENT_SCHEMES.contains(&specifier.scheme()) {
-        return None;
-      }
-      self.resolver.resolve_redirects(&specifier)
+      Cow::Borrowed(specifier)
+    };
+    if !DOCUMENT_SCHEMES.contains(&specifier.scheme()) {
+      return None;
     }
-  }
-
-  fn resolve_unstable_sloppy_import<'a>(
-    &self,
-    specifier: &'a ModuleSpecifier,
-  ) -> SloppyImportsResolution<'a> {
-    SloppyImportsResolver::resolve_with_stat_sync(
-      specifier,
-      ResolutionMode::Types,
-      |path| {
-        if let Ok(specifier) = ModuleSpecifier::from_file_path(path) {
-          if self.open_docs.contains_key(&specifier)
-            || self.cache.contains(&specifier)
-          {
-            return Some(SloppyImportsFsEntry::File);
-          }
-        }
-        path.metadata().ok().and_then(|m| {
-          if m.is_file() {
-            Some(SloppyImportsFsEntry::File)
-          } else if m.is_dir() {
-            Some(SloppyImportsFsEntry::Dir)
-          } else {
-            None
-          }
-        })
-      },
-    )
+    self.resolver.resolve_redirects(&specifier)
   }
 
   /// Return `true` if the specifier can be resolved to a document.
@@ -1226,12 +1184,7 @@ impl Documents {
   ) {
     self.config = Arc::new(config.clone());
     self.cache = cache;
-    let config_data = config.tree.root_data();
-    let config_file = config_data.and_then(|d| d.config_file.as_deref());
     self.resolver = resolver.clone();
-    self.unstable_sloppy_imports = config_file
-      .map(|c| c.has_unstable("sloppy-imports"))
-      .unwrap_or(false);
     {
       let fs_docs = &self.file_system_docs;
       // Clean up non-existent documents.
@@ -1404,7 +1357,6 @@ fn node_resolve_npm_req_ref(
 pub struct OpenDocumentsGraphLoader<'a> {
   pub inner_loader: &'a mut dyn deno_graph::source::Loader,
   pub open_docs: &'a HashMap<ModuleSpecifier, Arc<Document>>,
-  pub unstable_sloppy_imports: bool,
 }
 
 impl<'a> OpenDocumentsGraphLoader<'a> {
@@ -1426,32 +1378,6 @@ impl<'a> OpenDocumentsGraphLoader<'a> {
     }
     None
   }
-
-  fn resolve_unstable_sloppy_import<'b>(
-    &self,
-    specifier: &'b ModuleSpecifier,
-  ) -> SloppyImportsResolution<'b> {
-    SloppyImportsResolver::resolve_with_stat_sync(
-      specifier,
-      ResolutionMode::Types,
-      |path| {
-        if let Ok(specifier) = ModuleSpecifier::from_file_path(path) {
-          if self.open_docs.contains_key(&specifier) {
-            return Some(SloppyImportsFsEntry::File);
-          }
-        }
-        path.metadata().ok().and_then(|m| {
-          if m.is_file() {
-            Some(SloppyImportsFsEntry::File)
-          } else if m.is_dir() {
-            Some(SloppyImportsFsEntry::Dir)
-          } else {
-            None
-          }
-        })
-      },
-    )
-  }
 }
 
 impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> {
@@ -1460,17 +1386,9 @@ impl<'a> deno_graph::source::Loader for OpenDocumentsGraphLoader<'a> {
     specifier: &ModuleSpecifier,
     options: deno_graph::source::LoadOptions,
   ) -> deno_graph::source::LoadFuture {
-    let specifier = if self.unstable_sloppy_imports {
-      self
-        .resolve_unstable_sloppy_import(specifier)
-        .into_specifier()
-    } else {
-      Cow::Borrowed(specifier)
-    };
-
-    match self.load_from_docs(&specifier) {
+    match self.load_from_docs(specifier) {
       Some(fut) => fut,
-      None => self.inner_loader.load(&specifier, options),
+      None => self.inner_loader.load(specifier, options),
     }
   }
 
@@ -1531,7 +1449,7 @@ fn analyze_module(
         // dynamic imports like import(`./dir/${something}`) in the LSP
         file_system: &deno_graph::source::NullFileSystem,
         jsr_url_provider: &CliJsrUrlProvider,
-        maybe_resolver: Some(resolver.as_graph_resolver()),
+        maybe_resolver: Some(&resolver.as_graph_resolver()),
         maybe_npm_resolver: Some(resolver.as_graph_npm_resolver()),
       },
     )),

filename: cli/lsp/language_server.rs
patch: @@ -251,7 +251,6 @@ impl LanguageServer {
       let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader {
         inner_loader: &mut inner_loader,
         open_docs: &open_docs,
-        unstable_sloppy_imports: cli_options.unstable_sloppy_imports(),
       };
       let graph = module_graph_creator
         .create_graph_with_loader(GraphKind::All, roots.clone(), &mut loader)

filename: cli/lsp/resolver.rs
patch: @@ -21,6 +21,8 @@ use crate::npm::ManagedCliNpmResolver;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
 use crate::resolver::CliNodeResolver;
+use crate::resolver::SloppyImportsFsEntry;
+use crate::resolver::SloppyImportsResolver;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::progress_bar::ProgressBarStyle;
 use deno_cache_dir::HttpCache;
@@ -60,6 +62,7 @@ pub struct LspResolver {
   redirect_resolver: Option<Arc<RedirectResolver>>,
   graph_imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
   config: Arc<Config>,
+  unstable_sloppy_imports: bool,
 }
 
 impl Default for LspResolver {
@@ -73,6 +76,7 @@ impl Default for LspResolver {
       redirect_resolver: None,
       graph_imports: Default::default(),
       config: Default::default(),
+      unstable_sloppy_imports: false,
     }
   }
 }
@@ -140,6 +144,10 @@ impl LspResolver {
       npm_config_hash,
       redirect_resolver,
       graph_imports,
+      unstable_sloppy_imports: config_data
+        .and_then(|d| d.config_file.as_ref())
+        .map(|cf| cf.has_unstable("sloppy-imports"))
+        .unwrap_or(false),
       config: Arc::new(config.clone()),
     })
   }
@@ -162,6 +170,7 @@ impl LspResolver {
       redirect_resolver: self.redirect_resolver.clone(),
       graph_imports: self.graph_imports.clone(),
       config: self.config.clone(),
+      unstable_sloppy_imports: self.unstable_sloppy_imports,
     })
   }
 
@@ -181,8 +190,11 @@ impl LspResolver {
     Ok(())
   }
 
-  pub fn as_graph_resolver(&self) -> &dyn Resolver {
-    self.graph_resolver.as_ref()
+  pub fn as_graph_resolver(&self) -> LspGraphResolver {
+    LspGraphResolver {
+      inner: &self.graph_resolver,
+      unstable_sloppy_imports: self.unstable_sloppy_imports,
+    }
   }
 
   pub fn as_graph_npm_resolver(&self) -> &dyn NpmResolver {
@@ -307,6 +319,68 @@ impl LspResolver {
   }
 }
 
+#[derive(Debug)]
+pub struct LspGraphResolver<'a> {
+  inner: &'a CliGraphResolver,
+  unstable_sloppy_imports: bool,
+}
+
+impl<'a> Resolver for LspGraphResolver<'a> {
+  fn default_jsx_import_source(&self) -> Option<String> {
+    self.inner.default_jsx_import_source()
+  }
+
+  fn default_jsx_import_source_types(&self) -> Option<String> {
+    self.inner.default_jsx_import_source_types()
+  }
+
+  fn jsx_import_source_module(&self) -> &str {
+    self.inner.jsx_import_source_module()
+  }
+
+  fn resolve(
+    &self,
+    specifier_text: &str,
+    referrer_range: &deno_graph::Range,
+    mode: deno_graph::source::ResolutionMode,
+  ) -> Result<deno_ast::ModuleSpecifier, deno_graph::source::ResolveError> {
+    let specifier = self.inner.resolve(specifier_text, referrer_range, mode)?;
+    if self.unstable_sloppy_imports && specifier.scheme() == "file" {
+      Ok(
+        SloppyImportsResolver::resolve_with_stat_sync(
+          &specifier,
+          mode,
+          |path| {
+            path.metadata().ok().and_then(|m| {
+              if m.is_file() {
+                Some(SloppyImportsFsEntry::File)
+              } else if m.is_dir() {
+                Some(SloppyImportsFsEntry::Dir)
+              } else {
+                None
+              }
+            })
+          },
+        )
+        .into_specifier()
+        .into_owned(),
+      )
+    } else {
+      Ok(specifier)
+    }
+  }
+
+  fn resolve_types(
+    &self,
+    specifier: &deno_ast::ModuleSpecifier,
+  ) -> Result<
+    Option<(deno_ast::ModuleSpecifier, Option<deno_graph::Range>)>,
+    deno_graph::source::ResolveError,
+  > {
+    self.inner.resolve_types(specifier)
+  }
+}
+
 async fn create_npm_resolver(
   config_data: &ConfigData,
   global_cache_path: Option<&Path>,
@@ -399,9 +473,7 @@ fn create_graph_resolver(
     bare_node_builtins_enabled: config_file
       .map(|cf| cf.has_unstable("bare-node-builtins"))
       .unwrap_or(false),
-    // Don't set this for the LSP because instead we'll use the OpenDocumentsLoader
-    // because it's much easier and we get diagnostics/quick fixes about a redirected
-    // specifier for free.
+    // not used in the LSP as the LspGraphResolver handles this
     sloppy_imports_resolver: None,
   }))
 }

filename: tests/integration/lsp_tests.rs
patch: @@ -12,70 +12,13 @@ use test_util::assert_starts_with;
 use test_util::assertions::assert_json_subset;
 use test_util::deno_cmd_with_deno_dir;
 use test_util::env_vars_for_npm_tests;
+use test_util::lsp::range_of;
+use test_util::lsp::source_file;
 use test_util::lsp::LspClient;
 use test_util::testdata_path;
 use test_util::TestContextBuilder;
 use tower_lsp::lsp_types as lsp;
 
-/// Helper to get the `lsp::Range` of the `n`th occurrence of
-/// `text` in `src`. `n` is zero-based, like most indexes.
-fn range_of_nth(
-  n: usize,
-  text: impl AsRef<str>,
-  src: impl AsRef<str>,
-) -> lsp::Range {
-  let text = text.as_ref();
-
-  let src = src.as_ref();
-
-  let start = src
-    .match_indices(text)
-    .nth(n)
-    .map(|(i, _)| i)
-    .unwrap_or_else(|| panic!("couldn't find text {text} in source {src}"));
-  let end = start + text.len();
-  let mut line = 0;
-  let mut col = 0;
-  let mut byte_idx = 0;
-
-  let pos = |line, col| lsp::Position {
-    line,
-    character: col,
-  };
-
-  let mut start_pos = None;
-  let mut end_pos = None;
-  for c in src.chars() {
-    if byte_idx == start {
-      start_pos = Some(pos(line, col));
-    }
-    if byte_idx == end {
-      end_pos = Some(pos(line, col));
-      break;
-    }
-    if c == '\n' {
-      line += 1;
-      col = 0;
-    } else {
-      col += c.len_utf16() as u32;
-    }
-    byte_idx += c.len_utf8();
-  }
-  if start_pos.is_some() && end_pos.is_none() {
-    // range extends to end of string
-    end_pos = Some(pos(line, col));
-  }
-
-  let (start, end) = (start_pos.unwrap(), end_pos.unwrap());
-  lsp::Range { start, end }
-}
-
-/// Helper to get the `lsp::Range` of the first occurrence of
-/// `text` in `src`. Equivalent to `range_of_nth(0, text, src)`.
-fn range_of(text: impl AsRef<str>, src: impl AsRef<str>) -> lsp::Range {
-  range_of_nth(0, text, src)
-}
-
 #[test]
 fn lsp_startup_shutdown() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
@@ -12106,15 +12049,19 @@ fn lsp_deno_future_env_byonm() {
 }
 
 #[test]
-fn lsp_sloppy_imports_warn() {
+fn lsp_sloppy_imports() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
   let temp_dir = context.temp_dir();
   let temp_dir = temp_dir.path();
   temp_dir
     .join("deno.json")
     .write(r#"{ "unstable": ["sloppy-imports"] }"#);
-  // should work when exists on the fs and when doesn't
+  // for sloppy imports, the file must exist on the file system
+  // to be resolved correctly
   temp_dir.join("a.ts").write("export class A {}");
+  temp_dir.join("b.ts").write("export class B {}");
+  temp_dir.join("c.js").write("export class C {}");
+  temp_dir.join("c.d.ts").write("export class C {}");
   let mut client = context.new_lsp_command().build();
   client.initialize(|builder| {
     builder.set_root_uri(temp_dir.uri_dir());
@@ -12161,137 +12108,67 @@ fn lsp_sloppy_imports_warn() {
       ),
     },
   }));
-  assert_eq!(
-    diagnostics.messages_with_source("deno"),
-    lsp::PublishDiagnosticsParams {
-      uri: temp_dir.join("file.ts").uri_file(),
-      diagnostics: vec![
-        lsp::Diagnostic {
-          range: lsp::Range {
-            start: lsp::Position {
-              line: 0,
-              character: 19
-            },
-            end: lsp::Position {
-              line: 0,
-              character: 24
-            }
-          },
-          severity: Some(lsp::DiagnosticSeverity::INFORMATION),
-          code: Some(lsp::NumberOrString::String("redirect".to_string())),
-          source: Some("deno".to_string()),
-          message: format!(
-            "The import of \"{}\" was redirected to \"{}\".",
-            temp_dir.join("a").uri_file(),
-            temp_dir.join("a.ts").uri_file()
-          ),
-          data: Some(json!({
-            "specifier": temp_dir.join("a").uri_file(),
-            "redirect": temp_dir.join("a.ts").uri_file()
-          })),
-          ..Default::default()
-        },
-        lsp::Diagnostic {
-          range: lsp::Range {
-            start: lsp::Position {
-              line: 1,
-              character: 19
-            },
-            end: lsp::Position {
-              line: 1,
-              character: 27
-            }
-          },
-          severity: Some(lsp::DiagnosticSeverity::INFORMATION),
-          code: Some(lsp::NumberOrString::String("redirect".to_string())),
-          source: Some("deno".to_string()),
-          message: format!(
-            "The import of \"{}\" was redirected to \"{}\".",
-            temp_dir.join("b.js").uri_file(),
-            temp_dir.join("b.ts").uri_file()
-          ),
-          data: Some(json!({
-            "specifier": temp_dir.join("b.js").uri_file(),
-            "redirect": temp_dir.join("b.ts").uri_file()
-          })),
-          ..Default::default()
-        }
-      ],
-      version: Some(1),
-    }
+
+  assert_eq!(json!(diagnostics.all()), json!([]));
+
+  client.shutdown();
+}
+
+#[test]
+fn lsp_sloppy_imports_prefers_dts() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+  let temp_dir = temp_dir.path();
+
+  temp_dir
+    .join("deno.json")
+    .write(r#"{ "unstable": ["sloppy-imports"] }"#);
+
+  let mut client: LspClient = context
+    .new_lsp_command()
+    .set_root_dir(temp_dir.clone())
+    .build();
+  client.initialize_default();
+
+  temp_dir.join("a.js").write("export const foo: number;");
+
+  let a_dts = source_file(temp_dir.join("a.d.ts"), "export const foo = 3;");
+  let file = source_file(
+    temp_dir.join("file.ts"),
+    "import { foo } from './a.js';\nconsole.log(foo);",
   );
+  let diagnostics = client.did_open_file(&file);
+  // no warnings because "a.js" exists
+  assert_eq!(diagnostics.all().len(), 0);
 
-  let res = client.write_request(
-    "textDocument/codeAction",
+  let diagnostics = client.did_open_file(&a_dts);
+  assert_eq!(diagnostics.all().len(), 0, "Got {:#?}", diagnostics.all());
+
+  let response = client.write_request(
+    "textDocument/references",
     json!({
-      "textDocument": {
-        "uri": temp_dir.join("file.ts").uri_file()
-      },
-      "range": {
-        "start": { "line": 0, "character": 19 },
-        "end": { "line": 0, "character": 24 }
-      },
+      "textDocument": a_dts.identifier(),
+      "position": a_dts.range_of("foo").start,
       "context": {
-        "diagnostics": [{
-          "range": {
-            "start": { "line": 0, "character": 19 },
-            "end": { "line": 0, "character": 24 }
-          },
-          "severity": 3,
-          "code": "redirect",
-          "source": "deno",
-          "message": format!(
-            "The import of \"{}\" was redirected to \"{}\".",
-            temp_dir.join("a").uri_file(),
-            temp_dir.join("a.ts").uri_file()
-          ),
-          "data": {
-            "specifier": temp_dir.join("a").uri_file(),
-            "redirect": temp_dir.join("a.ts").uri_file(),
-          },
-        }],
-        "only": ["quickfix"]
+        "includeDeclaration": false
       }
     }),
   );
-  assert_eq!(
-    res,
-    json!([{
-      "title": "Update specifier to its redirected specifier.",
-      "kind": "quickfix",
-      "diagnostics": [{
-        "range": {
-          "start": { "line": 0, "character": 19 },
-          "end": { "line": 0, "character": 24 }
-        },
-        "severity": 3,
-        "code": "redirect",
-        "source": "deno",
-        "message": format!(
-          "The import of \"{}\" was redirected to \"{}\".",
-          temp_dir.join("a").uri_file(),
-          temp_dir.join("a.ts").uri_file()
-        ),
-        "data": {
-          "specifier": temp_dir.join("a").uri_file(),
-          "redirect": temp_dir.join("a.ts").uri_file()
-        },
-      }],
-      "edit": {
-        "changes": {
-          temp_dir.join("file.ts").uri_file(): [{
-            "range": {
-              "start": { "line": 0, "character": 19 },
-              "end": { "line": 0, "character": 24 }
-            },
-            "newText": "\"./a.ts\""
-          }]
-        }
+  assert_json_subset(
+    response,
+    json!([
+      {
+        "uri": file.uri(),
+        // the import
+        "range": file.range_of("foo"),
+      },
+      {
+        "uri": file.uri(),
+        // the usage
+        "range": file.range_of_nth(1, "foo"),
       }
-    }])
+    ]),
   );
-
-  client.shutdown();
 }
 
 #[test]

filename: tests/util/server/src/lsp.rs
patch: @@ -778,6 +778,12 @@ impl LspClient {
     }
   }
 
+  pub fn did_open_file(&mut self, file: &SourceFile) -> CollectedDiagnostics {
+    self.did_open(json!({
+        "textDocument": file.text_document(),
+    }))
+  }
+
   pub fn did_open(&mut self, params: Value) -> CollectedDiagnostics {
     self.did_open_raw(params);
     self.read_diagnostics()
@@ -1138,6 +1144,130 @@ impl CollectedDiagnostics {
   }
 }
 
+#[derive(Debug, Clone)]
+pub struct SourceFile {
+  path: PathRef,
+  src: String,
+  lang: &'static str,
+  version: i32,
+}
+
+impl SourceFile {
+  pub fn new(path: PathRef, src: String) -> Self {
+    path.write(&src);
+    Self::new_in_mem(path, src)
+  }
+
+  pub fn new_in_mem(path: PathRef, src: String) -> Self {
+    let lang = match path.as_path().extension().unwrap().to_str().unwrap() {
+      "js" => "javascript",
+      "ts" | "d.ts" => "typescript",
+      "json" => "json",
+      other => panic!("unsupported file extension: {other}"),
+    };
+    Self {
+      path,
+      src,
+      lang,
+      version: 1,
+    }
+  }
+
+  pub fn range_of(&self, text: &str) -> lsp::Range {
+    range_of(text, &self.src)
+  }
+
+  pub fn range_of_nth(&self, n: usize, text: &str) -> lsp::Range {
+    range_of_nth(n, text, &self.src)
+  }
+
+  pub fn uri(&self) -> lsp::Url {
+    self.path.uri_file()
+  }
+
+  pub fn text_document(&self) -> lsp::TextDocumentItem {
+    lsp::TextDocumentItem {
+      uri: self.uri(),
+      language_id: self.lang.to_string(),
+      version: self.version,
+      text: self.src.clone(),
+    }
+  }
+
+  pub fn identifier(&self) -> lsp::TextDocumentIdentifier {
+    lsp::TextDocumentIdentifier { uri: self.uri() }
+  }
+}
+
+/// Helper to create a `SourceFile` and write its contents to disk.
+pub fn source_file(path: PathRef, src: impl AsRef<str>) -> SourceFile {
+  SourceFile::new(path, src.as_ref().to_string())
+}
+
+/// Helper to create a `SourceFile` in memory without writing to disk.
+pub fn source_file_in_mem(path: PathRef, src: impl AsRef<str>) -> SourceFile {
+  SourceFile::new_in_mem(path, src.as_ref().to_string())
+}
+
+/// Helper to get the `lsp::Range` of the `n`th occurrence of
+/// `text` in `src`. `n` is zero-based, like most indexes.
+pub fn range_of_nth(
+  n: usize,
+  text: impl AsRef<str>,
+  src: impl AsRef<str>,
+) -> lsp::Range {
+  let text = text.as_ref();
+
+  let src = src.as_ref();
+
+  let start = src
+    .match_indices(text)
+    .nth(n)
+    .map(|(i, _)| i)
+    .unwrap_or_else(|| panic!("couldn't find text {text} in source {src}"));
+  let end = start + text.len();
+  let mut line = 0;
+  let mut col = 0;
+  let mut byte_idx = 0;
+
+  let pos = |line, col| lsp::Position {
+    line,
+    character: col,
+  };
+
+  let mut start_pos = None;
+  let mut end_pos = None;
+  for c in src.chars() {
+    if byte_idx == start {
+      start_pos = Some(pos(line, col));
+    }
+    if byte_idx == end {
+      end_pos = Some(pos(line, col));
+      break;
+    }
+    if c == '\n' {
+      line += 1;
+      col = 0;
+    } else {
+      col += c.len_utf16() as u32;
+    }
+    byte_idx += c.len_utf8();
+  }
+  if start_pos.is_some() && end_pos.is_none() {
+    // range extends to end of string
+    end_pos = Some(pos(line, col));
+  }
+
+  let (start, end) = (start_pos.unwrap(), end_pos.unwrap());
+  lsp::Range { start, end }
+}
+
+/// Helper to get the `lsp::Range` of the first occurrence of
+/// `text` in `src`. Equivalent to `range_of_nth(0, text, src)`.
+pub fn range_of(text: impl AsRef<str>, src: impl AsRef<str>) -> lsp::Range {
+  range_of_nth(0, text, src)
+}
+
 #[cfg(test)]
 mod tests {
   use super::*;