feat(lsp): provide quick fixes for specifiers that could be resolved sloppily

cli/graph_util.rs

@@ -12,15 +12,13 @@ use crate::errors::get_error_class_name;
 use crate::file_fetcher::FileFetcher;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
-use crate::resolver::SloppyImportsResolution;
 use crate::resolver::SloppyImportsResolver;
 use crate::tools::check;
 use crate::tools::check::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
 use crate::util::sync::TaskQueue;
 use crate::util::sync::TaskQueuePermit;
 
-use deno_ast::MediaType;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::custom_error;
@@ -61,7 +59,7 @@ pub struct GraphValidOptions {
 /// error statically reachable from `roots` and not a dynamic import.
 pub fn graph_valid_with_cli_options(
   graph: &ModuleGraph,
-  fs: &Arc<dyn FileSystem>,
+  fs: &dyn FileSystem,
   roots: &[ModuleSpecifier],
   options: &CliOptions,
 ) -> Result<(), AnyError> {
@@ -86,7 +84,7 @@ pub fn graph_valid_with_cli_options(
 /// for the CLI.
 pub fn graph_valid(
   graph: &ModuleGraph,
-  fs: &Arc<dyn FileSystem>,
+  fs: &dyn FileSystem,
   roots: &[ModuleSpecifier],
   options: GraphValidOptions,
 ) -> Result<(), AnyError> {
@@ -366,7 +364,7 @@ impl ModuleGraphBuilder {
     let graph = Arc::new(graph);
     graph_valid_with_cli_options(
       &graph,
-      &self.fs,
+      self.fs.as_ref(),
       &graph.roots,
       &self.options,
     )?;
@@ -538,12 +536,13 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
 }
 
 pub fn enhanced_module_error_message(
-  fs: &Arc<dyn FileSystem>,
+  fs: &dyn FileSystem,
   error: &ModuleError,
 ) -> String {
   let additional_message = match error {
     ModuleError::Missing(specifier, _) => {
-      maybe_sloppy_imports_suggestion_message(fs, specifier)
+      SloppyImportsResolver::resolve_with_fs(fs, specifier)
+        .as_suggestion_message()
     }
     _ => None,
   };
@@ -557,48 +556,6 @@ pub fn enhanced_module_error_message(
   }
 }
 
-pub fn maybe_sloppy_imports_suggestion_message(
-  fs: &Arc<dyn FileSystem>,
-  original_specifier: &ModuleSpecifier,
-) -> Option<String> {
-  let sloppy_imports_resolver = SloppyImportsResolver::new(fs.clone());
-  let resolution = sloppy_imports_resolver.resolve(original_specifier);
-  sloppy_import_resolution_to_suggestion_message(&resolution)
-}
-
-fn sloppy_import_resolution_to_suggestion_message(
-  resolution: &SloppyImportsResolution,
-) -> Option<String> {
-  match resolution {
-    SloppyImportsResolution::None(_) => None,
-    SloppyImportsResolution::JsToTs(specifier) => {
-      let media_type = MediaType::from_specifier(specifier);
-      Some(format!(
-        "Maybe change the extension to '{}'",
-        media_type.as_ts_extension()
-      ))
-    }
-    SloppyImportsResolution::NoExtension(specifier) => {
-      let media_type = MediaType::from_specifier(specifier);
-      Some(format!(
-        "Maybe add a '{}' extension",
-        media_type.as_ts_extension()
-      ))
-    }
-    SloppyImportsResolution::Directory(specifier) => {
-      let file_name = specifier
-        .path()
-        .rsplit_once('/')
-        .map(|(_, file_name)| file_name)
-        .unwrap_or(specifier.path());
-      Some(format!(
-        "Maybe specify path to '{}' file in directory instead",
-        file_name
-      ))
-    }
-  }
-}
-
 pub fn get_resolution_error_bare_node_specifier(
   error: &ResolutionError,
 ) -> Option<&str> {
@@ -972,46 +929,4 @@ mod test {
       assert_eq!(get_resolution_error_bare_node_specifier(&err), output,);
     }
   }
-
-  #[test]
-  fn test_sloppy_import_resolution_to_message() {
-    // none
-    let url = ModuleSpecifier::parse("file:///dir/index.js").unwrap();
-    assert_eq!(
-      sloppy_import_resolution_to_suggestion_message(
-        &SloppyImportsResolution::None(&url)
-      ),
-      None,
-    );
-    // directory
-    assert_eq!(
-      sloppy_import_resolution_to_suggestion_message(
-        &SloppyImportsResolution::Directory(
-          ModuleSpecifier::parse("file:///dir/index.js").unwrap()
-        )
-      )
-      .unwrap(),
-      "Maybe specify path to 'index.js' file in directory instead"
-    );
-    // no ext
-    assert_eq!(
-      sloppy_import_resolution_to_suggestion_message(
-        &SloppyImportsResolution::NoExtension(
-          ModuleSpecifier::parse("file:///dir/index.mjs").unwrap()
-        )
-      )
-      .unwrap(),
-      "Maybe add a '.mjs' extension"
-    );
-    // js to ts
-    assert_eq!(
-      sloppy_import_resolution_to_suggestion_message(
-        &SloppyImportsResolution::JsToTs(
-          ModuleSpecifier::parse("file:///dir/index.mts").unwrap()
-        )
-      )
-      .unwrap(),
-      "Maybe change the extension to '.mts'"
-    );
-  }
 }

cli/lsp/diagnostics.rs

@@ -19,6 +19,8 @@ use crate::args::LintOptions;
 use crate::graph_util;
 use crate::graph_util::enhanced_resolution_error_message;
 use crate::lsp::lsp_custom::DiagnosticBatchNotificationParams;
+use crate::resolver::SloppyImportsResolution;
+use crate::resolver::SloppyImportsResolver;
 use crate::tools::lint::get_configured_rules;
 
 use deno_ast::MediaType;
@@ -938,6 +940,13 @@ struct DiagnosticDataRedirect {
   pub redirect: ModuleSpecifier,
 }
 
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct DiagnosticDataNoLocal {
+  pub to: ModuleSpecifier,
+  pub message: String,
+}
+
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct DiagnosticDataImportMapRemap {
@@ -1084,6 +1093,32 @@ impl DenoDiagnostic {
             ..Default::default()
           }
         }
+        "no-local" => {
+          let data = diagnostic
+            .data
+            .clone()
+            .ok_or_else(|| anyhow!("Diagnostic is missing data"))?;
+          let data: DiagnosticDataNoLocal = serde_json::from_value(data)?;
+          lsp::CodeAction {
+            title: data.message,
+            kind: Some(lsp::CodeActionKind::QUICKFIX),
+            diagnostics: Some(vec![diagnostic.clone()]),
+            edit: Some(lsp::WorkspaceEdit {
+              changes: Some(HashMap::from([(
+                specifier.clone(),
+                vec![lsp::TextEdit {
+                  new_text: format!(
+                    "\"{}\"",
+                    relative_specifier(&data.to, specifier)
+                  ),
+                  range: diagnostic.range,
+                }],
+              )])),
+              ..Default::default()
+            }),
+            ..Default::default()
+          }
+        }
         "redirect" => {
           let data = diagnostic
             .data
@@ -1150,15 +1185,16 @@ impl DenoDiagnostic {
   /// diagnostic is fixable or not
   pub fn is_fixable(diagnostic: &lsp_types::Diagnostic) -> bool {
     if let Some(lsp::NumberOrString::String(code)) = &diagnostic.code {
-      matches!(
-        code.as_str(),
+      match code.as_str() {
         "import-map-remap"
-          | "no-cache"
-          | "no-cache-npm"
-          | "no-attribute-type"
-          | "redirect"
-          | "import-node-prefix-missing"
-      )
+        | "no-cache"
+        | "no-cache-npm"
+        | "no-attribute-type"
+        | "redirect"
+        | "import-node-prefix-missing" => true,
+        "no-local" => diagnostic.data.is_some(),
+        _ => false,
+      }
     } else {
       false
     }
@@ -1167,12 +1203,14 @@ impl DenoDiagnostic {
   /// Convert to an lsp Diagnostic when the range the diagnostic applies to is
   /// provided.
   pub fn to_lsp_diagnostic(&self, range: &lsp::Range) -> lsp::Diagnostic {
-    fn no_local_message(specifier: &ModuleSpecifier) -> String {
-      let fs: Arc<dyn deno_fs::FileSystem> = Arc::new(deno_fs::RealFs);
+    fn no_local_message(
+      specifier: &ModuleSpecifier,
+      sloppy_resolution: SloppyImportsResolution,
+    ) -> String {
       let mut message =
         format!("Unable to load a local module: {}\n", specifier);
       if let Some(additional_message) =
-        graph_util::maybe_sloppy_imports_suggestion_message(&fs, specifier)
+        sloppy_resolution.as_suggestion_message()
       {
         message.push_str(&additional_message);
         message.push('.');
@@ -1189,7 +1227,17 @@ impl DenoDiagnostic {
       Self::NoAttributeType => (lsp::DiagnosticSeverity::ERROR, "The module is a JSON module and not being imported with an import attribute. Consider adding `with { type: \"json\" }` to the import statement.".to_string(), None),
       Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: {specifier}"), Some(json!({ "specifier": specifier }))),
       Self::NoCacheNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: {}", pkg_req), Some(json!({ "specifier": specifier }))),
-      Self::NoLocal(specifier) => (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier), None),
+      Self::NoLocal(specifier) => {
+        let sloppy_resolution = SloppyImportsResolver::resolve_with_fs(&deno_fs::RealFs, specifier);
+        let data = sloppy_resolution.as_lsp_quick_fix_message().map(|message| {
+          json!({
+            "specifier": specifier,
+            "to": sloppy_resolution.as_specifier(),
+            "message": message,
+          })
+        });
+        (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, sloppy_resolution), data)
+      },
       Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{from}\" was redirected to \"{to}\"."), Some(json!({ "specifier": from, "redirect": to }))),
       Self::ResolutionError(err) => (
         lsp::DiagnosticSeverity::ERROR,
@@ -1218,21 +1266,25 @@ fn specifier_text_for_redirected(
 ) -> String {
   if redirect.scheme() == "file" && referrer.scheme() == "file" {
     // use a relative specifier when it's going to a file url
-    match referrer.make_relative(redirect) {
-      Some(relative) => {
-        if relative.starts_with('.') {
-          relative
-        } else {
-          format!("./{}", relative)
-        }
-      }
-      None => redirect.to_string(),
-    }
+    relative_specifier(redirect, referrer)
   } else {
     redirect.to_string()
   }
 }
 
+fn relative_specifier(specifier: &lsp::Url, referrer: &lsp::Url) -> String {
+  match referrer.make_relative(specifier) {
+    Some(relative) => {
+      if relative.starts_with('.') {
+        relative
+      } else {
+        format!("./{}", relative)
+      }
+    }
+    None => specifier.to_string(),
+  }
+}
+
 fn diagnose_resolution(
   snapshot: &language_server::StateSnapshot,
   dependency_key: &str,

cli/lsp/documents.rs

@@ -1055,7 +1055,8 @@ impl Documents {
       Some(
         self
           .resolve_unstable_sloppy_import(specifier)
-          .into_owned_specifier(),
+          .into_specifier()
+          .into_owned(),
       )
     } else {
       self.redirect_resolver.resolve(specifier)

cli/lsp/language_server.rs

@@ -263,7 +263,7 @@ impl LanguageServer {
         .await?;
       graph_util::graph_valid(
         &graph,
-        factory.fs(),
+        factory.fs().as_ref(),
         &roots,
         graph_util::GraphValidOptions {
           is_vendoring: false,

cli/module_loader.rs

@@ -169,7 +169,12 @@ impl ModuleLoadPreparer {
       )
       .await?;
 
-    graph_valid_with_cli_options(graph, &self.fs, &roots, &self.options)?;
+    graph_valid_with_cli_options(
+      graph,
+      self.fs.as_ref(),
+      &roots,
+      &self.options,
+    )?;
 
     // If there is a lockfile...
     if let Some(lockfile) = &self.lockfile {