fix: ensure "fs" -> "node:fs" error/quick fix works when user has import map (#17566)

Closes #17563

cli/graph_util.rs

@@ -31,6 +31,7 @@ use deno_graph::ResolutionError;
 use deno_graph::Resolved;
 use deno_graph::SpecifierError;
 use deno_runtime::permissions::PermissionsContainer;
+use import_map::ImportMapError;
 use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -646,17 +647,93 @@ fn handle_check_error(
 pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
   let mut message = format!("{error}");
 
+  if let Some(specifier) = get_resolution_error_bare_node_specifier(error) {
+    message.push_str(&format!(
+        "\nIf you want to use a built-in Node module, add a \"node:\" prefix (ex. \"node:{specifier}\")."
+      ));
+  }
+
+  message
+}
+
+pub fn get_resolution_error_bare_node_specifier(
+  error: &ResolutionError,
+) -> Option<&str> {
+  get_resolution_error_bare_specifier(error).filter(|specifier| {
+    crate::node::resolve_builtin_node_module(specifier).is_ok()
+  })
+}
+
+fn get_resolution_error_bare_specifier(
+  error: &ResolutionError,
+) -> Option<&str> {
   if let ResolutionError::InvalidSpecifier {
     error: SpecifierError::ImportPrefixMissing(specifier, _),
     ..
   } = error
   {
-    if crate::node::resolve_builtin_node_module(specifier).is_ok() {
+    Some(specifier.as_str())
-      message.push_str(&format!(
+  } else if let ResolutionError::ResolverError { error, .. } = error {
-        "\nIf you want to use a built-in Node module, add a \"node:\" prefix (ex. \"node:{specifier}\")."
+    if let Some(ImportMapError::UnmappedBareSpecifier(specifier, _)) =
-      ));
+      error.downcast_ref::<ImportMapError>()
+    {
+      Some(specifier.as_str())
+    } else {
+      None
+    }
+  } else {
+    None
   }
 }
 
-  message
+#[cfg(test)]
+mod test {
+  use std::sync::Arc;
+
+  use deno_ast::ModuleSpecifier;
+  use deno_graph::Position;
+  use deno_graph::Range;
+  use deno_graph::ResolutionError;
+  use deno_graph::SpecifierError;
+
+  use crate::graph_util::get_resolution_error_bare_node_specifier;
+
+  #[test]
+  fn import_map_node_resolution_error() {
+    let cases = vec![("fs", Some("fs")), ("other", None)];
+    for (input, output) in cases {
+      let import_map = import_map::ImportMap::new(
+        ModuleSpecifier::parse("file:///deno.json").unwrap(),
+      );
+      let specifier = ModuleSpecifier::parse("file:///file.ts").unwrap();
+      let err = import_map.resolve(input, &specifier).err().unwrap();
+      let err = ResolutionError::ResolverError {
+        error: Arc::new(err.into()),
+        specifier: input.to_string(),
+        range: Range {
+          specifier,
+          start: Position::zeroed(),
+          end: Position::zeroed(),
+        },
+      };
+      assert_eq!(get_resolution_error_bare_node_specifier(&err), output);
+    }
+  }
+
+  #[test]
+  fn bare_specifier_node_resolution_error() {
+    let cases = vec![("process", Some("process")), ("other", None)];
+    for (input, output) in cases {
+      let specifier = ModuleSpecifier::parse("file:///file.ts").unwrap();
+      let err = ResolutionError::InvalidSpecifier {
+        range: Range {
+          specifier,
+          start: Position::zeroed(),
+          end: Position::zeroed(),
+        },
+        error: SpecifierError::ImportPrefixMissing(input.to_string(), None),
+      };
+      assert_eq!(get_resolution_error_bare_node_specifier(&err), output,);
+    }
+  }
 }

cli/lsp/diagnostics.rs

@@ -13,6 +13,7 @@ use super::tsc;
 use super::tsc::TsServer;
 
 use crate::args::LintOptions;
+use crate::graph_util;
 use crate::graph_util::enhanced_resolution_error_message;
 use crate::node;
 use crate::npm::NpmPackageReference;
@@ -641,15 +642,25 @@ impl DenoDiagnostic {
       Self::NoCacheNpm(_, _) => "no-cache-npm",
       Self::NoLocal(_) => "no-local",
       Self::Redirect { .. } => "redirect",
-      Self::ResolutionError(err) => match err {
+      Self::ResolutionError(err) => {
+        if graph_util::get_resolution_error_bare_node_specifier(err).is_some() {
+          "import-node-prefix-missing"
+        } else {
+          match err {
             ResolutionError::InvalidDowngrade { .. } => "invalid-downgrade",
-        ResolutionError::InvalidLocalImport { .. } => "invalid-local-import",
+            ResolutionError::InvalidLocalImport { .. } => {
+              "invalid-local-import"
+            }
             ResolutionError::InvalidSpecifier { error, .. } => match error {
-          SpecifierError::ImportPrefixMissing(_, _) => "import-prefix-missing",
+              SpecifierError::ImportPrefixMissing(_, _) => {
+                "import-prefix-missing"
+              }
               SpecifierError::InvalidUrl(_) => "invalid-url",
             },
             ResolutionError::ResolverError { .. } => "resolver-error",
-      },
+          }
+        }
+      }
       Self::InvalidNodeSpecifier(_) => "resolver-error",
     }
   }
@@ -752,10 +763,7 @@ impl DenoDiagnostic {
             ..Default::default()
           }
         }
-        "import-prefix-missing" => {
+        "import-node-prefix-missing" => {
-          // if an import-prefix-missing diagnostic ends up here, then that currently
-          // will only ever occur for a possible "node:" specifier, so don't bother
-          // checking if it's actually a "node:"" specifier
           let data = diagnostic
             .data
             .clone()
@@ -795,9 +803,6 @@ 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 {
-      if code == "import-prefix-missing" {
-        diagnostic.data.is_some()
-      } else {
       matches!(
         code.as_str(),
         "import-map-remap"
@@ -806,8 +811,8 @@ impl DenoDiagnostic {
           | "no-cache-data"
           | "no-assert-type"
           | "redirect"
+          | "import-node-prefix-missing"
       )
-      }
     } else {
       false
     }
@@ -830,18 +835,8 @@ impl DenoDiagnostic {
       Self::ResolutionError(err) => (
         lsp::DiagnosticSeverity::ERROR,
         enhanced_resolution_error_message(err),
-        if let ResolutionError::InvalidSpecifier {
+        graph_util::get_resolution_error_bare_node_specifier(err)
-          error: SpecifierError::ImportPrefixMissing(specifier, _),
+          .map(|specifier| json!({ "specifier": specifier }))
-          ..
-        } = err {
-          if crate::node::resolve_builtin_node_module(specifier).is_ok() {
-            Some(json!({ "specifier": specifier }))
-          } else {
-            None
-          }
-        } else {
-          None
-        },
       ),
       Self::InvalidNodeSpecifier(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Unknown Node built-in module: {}", specifier.path()), None),
     };

cli/tests/integration/lsp_tests.rs

@@ -4563,7 +4563,7 @@ fn lsp_completions_node_specifier() {
     .filter(|d| {
       d.code
         == Some(lsp::NumberOrString::String(
-          "import-prefix-missing".to_string(),
+          "import-node-prefix-missing".to_string(),
         ))
     })
     .collect::<Vec<_>>();
@@ -4602,7 +4602,7 @@ fn lsp_completions_node_specifier() {
             "end": { "line": 0, "character": 19 }
           },
           "severity": 1,
-          "code": "import-prefix-missing",
+          "code": "import-node-prefix-missing",
           "source": "deno",
           "message": "Relative import path \"fs\" not prefixed with / or ./ or ../\nIf you want to use a built-in Node module, add a \"node:\" prefix (ex. \"node:fs\").",
           "data": {