feat: add suggestions to module not found error messages for file urls (

denoland#21498)

cli/factory.rs

@@ -39,7 +39,7 @@ use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
 use crate::npm::CliNpmResolverManagedSnapshotOption;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
-use crate::resolver::UnstableSloppyImportsResolver;
+use crate::resolver::SloppyImportsResolver;
 use crate::standalone::DenoCompileBinaryWriter;
 use crate::tools::check::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
@@ -383,7 +383,7 @@ impl CliFactory {
           fs: self.fs().clone(),
           cjs_resolutions: Some(self.cjs_resolutions().clone()),
           sloppy_imports_resolver: if self.options.unstable_sloppy_imports() {
-            Some(UnstableSloppyImportsResolver::new(self.fs().clone()))
+            Some(SloppyImportsResolver::new(self.fs().clone()))
           } else {
             None
           },

cli/graph_util.rs

@@ -12,12 +12,15 @@ 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;
@@ -58,11 +61,13 @@ 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>,
   roots: &[ModuleSpecifier],
   options: &CliOptions,
 ) -> Result<(), AnyError> {
   graph_valid(
     graph,
+    fs,
     roots,
     GraphValidOptions {
       is_vendoring: false,
@@ -81,6 +86,7 @@ pub fn graph_valid_with_cli_options(
 /// for the CLI.
 pub fn graph_valid(
   graph: &ModuleGraph,
+  fs: &Arc<dyn FileSystem>,
   roots: &[ModuleSpecifier],
   options: GraphValidOptions,
 ) -> Result<(), AnyError> {
@@ -109,10 +115,12 @@ pub fn graph_valid(
         ModuleGraphError::TypesResolutionError(resolution_error) => {
           format!(
             "Failed resolving types. {}",
-            enhanced_resolution_error_message(resolution_error,)
+            enhanced_resolution_error_message(resolution_error)
           )
         }
-        ModuleGraphError::ModuleError(_) => format!("{error}"),
+        ModuleGraphError::ModuleError(e) => {
+          enhanced_module_error_message(fs, e)
+        }
       };
 
       if let Some(range) = error.maybe_range() {
@@ -356,7 +364,12 @@ impl ModuleGraphBuilder {
       .await?;
 
     let graph = Arc::new(graph);
-    graph_valid_with_cli_options(&graph, &graph.roots, &self.options)?;
+    graph_valid_with_cli_options(
+      &graph,
+      &self.fs,
+      &graph.roots,
+      &self.options,
+    )?;
     if let Some(lockfile) = &self.lockfile {
       graph_lock_or_exit(&graph, &mut lockfile.lock());
     }
@@ -524,6 +537,68 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
   message
 }
 
+pub fn enhanced_module_error_message(
+  fs: &Arc<dyn FileSystem>,
+  error: &ModuleError,
+) -> String {
+  let additional_message = match error {
+    ModuleError::Missing(specifier, _) => {
+      maybe_sloppy_imports_suggestion_message(fs, specifier)
+    }
+    _ => None,
+  };
+  if let Some(message) = additional_message {
+    format!(
+      "{} {} or run with --unstable-sloppy-imports",
+      error, message
+    )
+  } else {
+    format!("{}", error)
+  }
+}
+
+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> {
@@ -897,4 +972,46 @@ 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:https:///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:https:///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:https:///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:https:///dir/index.mts").unwrap()
+        )
+      )
+      .unwrap(),
+      "Maybe change the extension to '.mts'"
+    );
+  }
 }

cli/lsp/diagnostics.rs

@@ -37,6 +37,7 @@ use deno_graph::Resolution;
 use deno_graph::ResolutionError;
 use deno_graph::SpecifierError;
 use deno_lint::rules::LintRule;
+use deno_runtime::deno_fs;
 use deno_runtime::deno_node;
 use deno_runtime::tokio_util::create_basic_runtime;
 use deno_semver::npm::NpmPackageReqReference;
@@ -1166,14 +1167,29 @@ 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);
+      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)
+      {
+        message.push_str(&additional_message);
+        message.push('.');
+      } else {
+        message.push_str("Please check the file path.");
+      }
+      message
+    }
+
     let (severity, message, data) = match self {
       Self::DenoWarn(message) => (lsp::DiagnosticSeverity::WARNING, message.to_string(), None),
       Self::ImportMapRemap { from, to } => (lsp::DiagnosticSeverity::HINT, format!("The import specifier can be remapped to \"{to}\" which will resolve it via the active import map."), Some(json!({ "from": from, "to": to }))),
       Self::InvalidAttributeType(assert_type) => (lsp::DiagnosticSeverity::ERROR, format!("The module is a JSON module and expected an attribute type of \"json\". Instead got \"{assert_type}\"."), None),
       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, format!("Unable to load a local module: {specifier}\n  Please check the file path."), None),
+      Self::NoLocal(specifier) => (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier), None),
       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,

cli/lsp/documents.rs

@@ -20,9 +20,9 @@ use crate::lsp::logging::lsp_warn;
 use crate::npm::CliNpmResolver;
 use crate::resolver::CliGraphResolver;
 use crate::resolver::CliGraphResolverOptions;
-use crate::resolver::UnstableSloppyImportsFsEntry;
-use crate::resolver::UnstableSloppyImportsResolution;
-use crate::resolver::UnstableSloppyImportsResolver;
+use crate::resolver::SloppyImportsFsEntry;
+use crate::resolver::SloppyImportsResolution;
+use crate::resolver::SloppyImportsResolver;
 use crate::util::glob;
 use crate::util::path::specifier_to_file_path;
 use crate::util::text_encoding;
@@ -1065,20 +1065,20 @@ impl Documents {
   fn resolve_unstable_sloppy_import<'a>(
     &self,
     specifier: &'a ModuleSpecifier,
-  ) -> UnstableSloppyImportsResolution<'a> {
-    UnstableSloppyImportsResolver::resolve_with_stat_sync(specifier, |path| {
+  ) -> SloppyImportsResolution<'a> {
+    SloppyImportsResolver::resolve_with_stat_sync(specifier, |path| {
       if let Ok(specifier) = ModuleSpecifier::from_file_path(path) {
         if self.open_docs.contains_key(&specifier)
           || self.cache.contains(&specifier)
         {
-          return Some(UnstableSloppyImportsFsEntry::File);
+          return Some(SloppyImportsFsEntry::File);
         }
       }
       path.metadata().ok().and_then(|m| {
         if m.is_file() {
-          Some(UnstableSloppyImportsFsEntry::File)
+          Some(SloppyImportsFsEntry::File)
         } else if m.is_dir() {
-          Some(UnstableSloppyImportsFsEntry::Dir)
+          Some(SloppyImportsFsEntry::Dir)
         } else {
           None
         }
@@ -1732,18 +1732,18 @@ impl<'a> OpenDocumentsGraphLoader<'a> {
   fn resolve_unstable_sloppy_import<'b>(
     &self,
     specifier: &'b ModuleSpecifier,
-  ) -> UnstableSloppyImportsResolution<'b> {
-    UnstableSloppyImportsResolver::resolve_with_stat_sync(specifier, |path| {
+  ) -> SloppyImportsResolution<'b> {
+    SloppyImportsResolver::resolve_with_stat_sync(specifier, |path| {
       if let Ok(specifier) = ModuleSpecifier::from_file_path(path) {
         if self.open_docs.contains_key(&specifier) {
-          return Some(UnstableSloppyImportsFsEntry::File);
+          return Some(SloppyImportsFsEntry::File);
         }
       }
       path.metadata().ok().and_then(|m| {
         if m.is_file() {
-          Some(UnstableSloppyImportsFsEntry::File)
+          Some(SloppyImportsFsEntry::File)
         } else if m.is_dir() {
-          Some(UnstableSloppyImportsFsEntry::Dir)
+          Some(SloppyImportsFsEntry::Dir)
         } else {
           None
         }

cli/lsp/language_server.rs

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

cli/module_loader.rs

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