feat(publish): error on invalid external imports (#22088)

cli/diagnostics.rs

@@ -49,6 +49,12 @@ pub struct DiagnosticSourceRange {
 pub enum DiagnosticSourcePos {
   SourcePos(SourcePos),
   ByteIndex(usize),
+  LineAndCol {
+    // 0-indexed line number
+    line: usize,
+    // 0-indexed column number
+    column: usize,
+  },
 }
 
 impl DiagnosticSourcePos {
@@ -56,6 +62,9 @@ impl DiagnosticSourcePos {
     match self {
       DiagnosticSourcePos::SourcePos(pos) => *pos,
       DiagnosticSourcePos::ByteIndex(index) => source.range().start() + *index,
+      DiagnosticSourcePos::LineAndCol { line, column } => {
+        source.line_start(*line) + *column
+      }
     }
   }
 }

cli/tests/integration/publish_tests.rs

@@ -3,6 +3,7 @@
 use deno_core::serde_json::json;
 use test_util::assert_contains;
 use test_util::assert_not_contains;
+use test_util::env_vars_for_npm_tests;
 use test_util::TestContextBuilder;
 
 static TEST_REGISTRY_URL: &str = "http://127.0.0.1:4250";
@@ -49,6 +50,15 @@ itest!(symlink {
   exit_code: 0,
 });
 
+itest!(invalid_import {
+  args: "publish --token 'sadfasdf' --dry-run",
+  output: "publish/invalid_import.out",
+  cwd: Some("publish/invalid_import"),
+  envs: env_vars_for_npm_tests(),
+  exit_code: 1,
+  http_server: true,
+});
+
 itest!(javascript_missing_decl_file {
   args: "publish --token 'sadfasdf'",
   output: "publish/javascript_missing_decl_file.out",

cli/tests/testdata/publish/invalid_import.out

@@ -0,0 +1,32 @@
+Download http://localhost:4545/welcome.ts
+Download http://localhost:4545/echo.ts
+Download http://localhost:4545/npm/registry/chalk
+Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz
+Checking fast check type graph for errors...
+Ensuring type checks...
+Check file://[WILDCARD]mod.ts
+error[invalid-external-import]: invalid import to a non-JSR 'http' specifier
+ --> [WILDCARD]mod.ts:1:8
+  | 
+1 | import "http://localhost:4545/welcome.ts";
+  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the specifier
+  = hint: replace this import with one from jsr or npm, or vendor the dependency into your package
+
+  info: the import was resolved to 'http://localhost:4545/welcome.ts'
+  info: this specifier is not allowed to be imported on jsr
+  info: jsr only supports importing `jsr:`, `npm:`, and `data:` specifiers
+  docs: https://jsr.io/go/invalid-external-import
+
+error[invalid-external-import]: invalid import to a non-JSR 'http' specifier
+ --> [WILDCARD]mod.ts:2:8
+  | 
+2 | import "$echo";
+  |        ^^^^^^^ the specifier
+  = hint: replace this import with one from jsr or npm, or vendor the dependency into your package
+
+  info: the import was resolved to 'http://localhost:4545/echo.ts'
+  info: this specifier is not allowed to be imported on jsr
+  info: jsr only supports importing `jsr:`, `npm:`, and `data:` specifiers
+  docs: https://jsr.io/go/invalid-external-import
+
+error: Found 2 problems

cli/tests/testdata/publish/invalid_import/deno.json

@@ -0,0 +1,10 @@
+{
+  "name": "@foo/bar",
+  "version": "1.0.0",
+  "imports": {
+    "$echo": "http://localhost:4545/echo.ts"
+  },
+  "exports": {
+    ".": "./mod.ts"
+  }
+}

cli/tests/testdata/publish/invalid_import/mod.ts

@@ -0,0 +1,9 @@
+import "http://localhost:4545/welcome.ts";
+import "$echo";
+
+import "data:application/javascript,console.log(1)";
+import "npm:chalk@5";
+
+export function foobar(): string {
+  return "string";
+}

cli/tests/testdata/publish/symlink.out

@@ -7,5 +7,6 @@ warning[unsupported-file-type]: unsupported file type 'symlink'
 
   info: only files and directories are supported
   info: the file was ignored and will not be published
+  docs: https://jsr.io/go/unsupported-file-type
 
 Warning Aborting due to --dry-run

cli/tools/registry/diagnostics.rs

@@ -63,60 +63,72 @@ impl PublishDiagnosticsCollector {
 pub enum PublishDiagnostic {
   FastCheck(FastCheckDiagnostic),
   ImportMapUnfurl(ImportMapUnfurlDiagnostic),
-  InvalidPath { path: PathBuf, message: String },
+  InvalidPath {
-  DuplicatePath { path: PathBuf },
+    path: PathBuf,
-  UnsupportedFileType { specifier: Url, kind: String },
+    message: String,
+  },
+  DuplicatePath {
+    path: PathBuf,
+  },
+  UnsupportedFileType {
+    specifier: Url,
+    kind: String,
+  },
+  InvalidExternalImport {
+    kind: String,
+    imported: Url,
+    referrer: deno_graph::Range,
+  },
 }
 
 impl Diagnostic for PublishDiagnostic {
   fn level(&self) -> DiagnosticLevel {
+    use PublishDiagnostic::*;
     match self {
-      PublishDiagnostic::FastCheck(
+      FastCheck(FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint {
-        FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. },
+        ..
-      ) => DiagnosticLevel::Warning,
+      }) => DiagnosticLevel::Warning,
-      PublishDiagnostic::FastCheck(_) => DiagnosticLevel::Error,
+      FastCheck(_) => DiagnosticLevel::Error,
-      PublishDiagnostic::ImportMapUnfurl(_) => DiagnosticLevel::Warning,
+      ImportMapUnfurl(_) => DiagnosticLevel::Warning,
-      PublishDiagnostic::InvalidPath { .. } => DiagnosticLevel::Error,
+      InvalidPath { .. } => DiagnosticLevel::Error,
-      PublishDiagnostic::DuplicatePath { .. } => DiagnosticLevel::Error,
+      DuplicatePath { .. } => DiagnosticLevel::Error,
-      PublishDiagnostic::UnsupportedFileType { .. } => DiagnosticLevel::Warning,
+      UnsupportedFileType { .. } => DiagnosticLevel::Warning,
+      InvalidExternalImport { .. } => DiagnosticLevel::Error,
     }
   }
 
   fn code(&self) -> impl Display + '_ {
+    use PublishDiagnostic::*;
     match &self {
-      PublishDiagnostic::FastCheck(diagnostic) => diagnostic.code(),
+      FastCheck(diagnostic) => diagnostic.code(),
-      PublishDiagnostic::ImportMapUnfurl(diagnostic) => diagnostic.code(),
+      ImportMapUnfurl(diagnostic) => diagnostic.code(),
-      PublishDiagnostic::InvalidPath { .. } => "invalid-path",
+      InvalidPath { .. } => "invalid-path",
-      PublishDiagnostic::DuplicatePath { .. } => {
+      DuplicatePath { .. } => "case-insensitive-duplicate-path",
-        "case-insensitive-duplicate-path"
+      UnsupportedFileType { .. } => "unsupported-file-type",
-      }
+      InvalidExternalImport { .. } => "invalid-external-import",
-      PublishDiagnostic::UnsupportedFileType { .. } => "unsupported-file-type",
     }
   }
 
   fn message(&self) -> impl Display + '_ {
+    use PublishDiagnostic::*;
     match &self {
-      PublishDiagnostic::FastCheck(diagnostic) => {
+      FastCheck(diagnostic) => Cow::Owned(diagnostic.to_string()) ,
-        Cow::Owned(diagnostic.to_string())
+      ImportMapUnfurl(diagnostic) => Cow::Borrowed(diagnostic.message()),
-      }
+      InvalidPath { message, .. } => Cow::Borrowed(message.as_str()),
-      PublishDiagnostic::ImportMapUnfurl(diagnostic) => {
+      DuplicatePath { .. } => {
-        Cow::Borrowed(diagnostic.message())
-      }
-      PublishDiagnostic::InvalidPath { message, .. } => {
-        Cow::Borrowed(message.as_str())
-      }
-      PublishDiagnostic::DuplicatePath { .. } => {
         Cow::Borrowed("package path is a case insensitive duplicate of another path in the package")
       }
-      PublishDiagnostic::UnsupportedFileType { kind, .. } => {
+      UnsupportedFileType { kind, .. } => {
-        Cow::Owned(format!("unsupported file type '{kind}'",))
+        Cow::Owned(format!("unsupported file type '{kind}'"))
       }
+      InvalidExternalImport { kind, .. } => Cow::Owned(format!("invalid import to a {kind} specifier")),
     }
   }
 
   fn location(&self) -> DiagnosticLocation {
+    use PublishDiagnostic::*;
     match &self {
-      PublishDiagnostic::FastCheck(diagnostic) => match diagnostic.range() {
+      FastCheck(diagnostic) => match diagnostic.range() {
         Some(range) => DiagnosticLocation::ModulePosition {
           specifier: Cow::Borrowed(diagnostic.specifier()),
           source_pos: DiagnosticSourcePos::SourcePos(range.range.start),
@@ -125,7 +137,7 @@ impl Diagnostic for PublishDiagnostic {
           specifier: Cow::Borrowed(diagnostic.specifier()),
         },
       },
-      PublishDiagnostic::ImportMapUnfurl(diagnostic) => match diagnostic {
+      ImportMapUnfurl(diagnostic) => match diagnostic {
         ImportMapUnfurlDiagnostic::UnanalyzableDynamicImport {
           specifier,
           range,
@@ -134,15 +146,22 @@ impl Diagnostic for PublishDiagnostic {
           source_pos: DiagnosticSourcePos::SourcePos(range.start),
         },
       },
-      PublishDiagnostic::InvalidPath { path, .. } => {
+      InvalidPath { path, .. } => {
         DiagnosticLocation::Path { path: path.clone() }
       }
-      PublishDiagnostic::DuplicatePath { path, .. } => {
+      DuplicatePath { path, .. } => {
         DiagnosticLocation::Path { path: path.clone() }
       }
-      PublishDiagnostic::UnsupportedFileType { specifier, .. } => {
+      UnsupportedFileType { specifier, .. } => DiagnosticLocation::Module {
-        DiagnosticLocation::Module {
+        specifier: Cow::Borrowed(specifier),
-          specifier: Cow::Borrowed(specifier),
+      },
+      InvalidExternalImport { referrer, .. } => {
+        DiagnosticLocation::ModulePosition {
+          specifier: Cow::Borrowed(&referrer.specifier),
+          source_pos: DiagnosticSourcePos::LineAndCol {
+            line: referrer.start.line,
+            column: referrer.start.character,
+          },
         }
       }
     }
@@ -184,6 +203,27 @@ impl Diagnostic for PublishDiagnostic {
       PublishDiagnostic::InvalidPath { .. } => None,
       PublishDiagnostic::DuplicatePath { .. } => None,
       PublishDiagnostic::UnsupportedFileType { .. } => None,
+      PublishDiagnostic::InvalidExternalImport { referrer, .. } => {
+        Some(DiagnosticSnippet {
+          source: DiagnosticSnippetSource::Specifier(Cow::Borrowed(
+            &referrer.specifier,
+          )),
+          highlight: DiagnosticSnippetHighlight {
+            style: DiagnosticSnippetHighlightStyle::Error,
+            range: DiagnosticSourceRange {
+              start: DiagnosticSourcePos::LineAndCol {
+                line: referrer.start.line,
+                column: referrer.start.character,
+              },
+              end: DiagnosticSourcePos::LineAndCol {
+                line: referrer.end.line,
+                column: referrer.end.character,
+              },
+            },
+            description: Some("the specifier".into()),
+          },
+        })
+      }
     }
   }
 
@@ -200,6 +240,7 @@ impl Diagnostic for PublishDiagnostic {
       PublishDiagnostic::UnsupportedFileType { .. } => Some(
         "remove the file, or add it to 'publish.exclude' in the config file",
       ),
+      PublishDiagnostic::InvalidExternalImport { .. } => Some("replace this import with one from jsr or npm, or vendor the dependency into your package")
     }
   }
 
@@ -234,6 +275,11 @@ impl Diagnostic for PublishDiagnostic {
         Cow::Borrowed("only files and directories are supported"),
         Cow::Borrowed("the file was ignored and will not be published")
       ]),
+      PublishDiagnostic::InvalidExternalImport { imported, .. } => Cow::Owned(vec![
+        Cow::Owned(format!("the import was resolved to '{}'", imported)),
+        Cow::Borrowed("this specifier is not allowed to be imported on jsr"),
+        Cow::Borrowed("jsr only supports importing `jsr:`, `npm:`, and `data:` specifiers"),
+      ]),
     }
   }
 
@@ -251,7 +297,12 @@ impl Diagnostic for PublishDiagnostic {
       PublishDiagnostic::DuplicatePath { .. } => {
         Some("https://jsr.io/go/case-insensitive-duplicate-path".to_owned())
       }
-      PublishDiagnostic::UnsupportedFileType { .. } => None,
+      PublishDiagnostic::UnsupportedFileType { .. } => {
+        Some("https://jsr.io/go/unsupported-file-type".to_owned())
+      }
+      PublishDiagnostic::InvalidExternalImport { .. } => {
+        Some("https://jsr.io/go/invalid-external-import".to_owned())
+      }
     }
   }
 }

cli/tools/registry/graph.rs

@@ -10,7 +10,11 @@ use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_graph::FastCheckDiagnostic;
+use deno_graph::ModuleEntryRef;
 use deno_graph::ModuleGraph;
+use deno_graph::ResolutionResolved;
+use deno_graph::WalkOptions;
+use lsp_types::Url;
 
 use super::diagnostics::PublishDiagnostic;
 use super::diagnostics::PublishDiagnosticsCollector;
@@ -64,6 +68,75 @@ pub fn resolve_config_file_roots_from_exports(
   Ok(exports)
 }
 
+pub fn collect_invalid_external_imports(
+  graph: &ModuleGraph,
+  diagnostics_collector: &PublishDiagnosticsCollector,
+) {
+  let mut visited = HashSet::new();
+  let mut skip_specifiers: HashSet<Url> = HashSet::new();
+
+  let mut collect_if_invalid =
+    |skip_specifiers: &mut HashSet<Url>, resolution: &ResolutionResolved| {
+      if visited.insert(resolution.specifier.clone()) {
+        match resolution.specifier.scheme() {
+          "file" | "data" => {}
+          "jsr" | "npm" => {
+            skip_specifiers.insert(resolution.specifier.clone());
+          }
+          "http" | "https" => {
+            skip_specifiers.insert(resolution.specifier.clone());
+            diagnostics_collector.push(
+              PublishDiagnostic::InvalidExternalImport {
+                kind: format!("non-JSR '{}'", resolution.specifier.scheme()),
+                imported: resolution.specifier.clone(),
+                referrer: resolution.range.clone(),
+              },
+            );
+          }
+          _ => {
+            skip_specifiers.insert(resolution.specifier.clone());
+            diagnostics_collector.push(
+              PublishDiagnostic::InvalidExternalImport {
+                kind: format!("'{}'", resolution.specifier.scheme()),
+                imported: resolution.specifier.clone(),
+                referrer: resolution.range.clone(),
+              },
+            );
+          }
+        }
+      }
+    };
+
+  let options = WalkOptions {
+    check_js: true,
+    follow_dynamic: true,
+    follow_type_only: true,
+  };
+  let mut iter = graph.walk(&graph.roots, options);
+  while let Some((specifier, entry)) = iter.next() {
+    if skip_specifiers.contains(specifier) {
+      iter.skip_previous_dependencies();
+      continue;
+    }
+
+    let ModuleEntryRef::Module(module) = entry else {
+      continue;
+    };
+    let Some(module) = module.esm() else {
+      continue;
+    };
+
+    for (_, dep) in &module.dependencies {
+      if let Some(resolved) = dep.maybe_code.ok() {
+        collect_if_invalid(&mut skip_specifiers, resolved);
+      }
+      if let Some(resolved) = dep.maybe_type.ok() {
+        collect_if_invalid(&mut skip_specifiers, resolved);
+      }
+    }
+  }
+}
+
 /// Collects diagnostics from the module graph for the given packages.
 /// Returns true if any diagnostics were collected.
 pub fn collect_fast_check_type_graph_diagnostics(

cli/tools/registry/mod.rs

@@ -34,6 +34,7 @@ use crate::http_util::HttpClient;
 use crate::tools::check::CheckOptions;
 use crate::tools::registry::diagnostics::PublishDiagnosticsCollector;
 use crate::tools::registry::graph::collect_fast_check_type_graph_diagnostics;
+use crate::tools::registry::graph::collect_invalid_external_imports;
 use crate::tools::registry::graph::get_workspace_member_roots;
 use crate::tools::registry::graph::resolve_config_file_roots_from_exports;
 use crate::tools::registry::graph::MemberRoots;
@@ -752,6 +753,9 @@ async fn build_and_check_graph_for_publish(
       .await?,
   );
   graph.valid()?;
+
+  collect_invalid_external_imports(&graph, diagnostics_collector);
+
   log::info!("Checking fast check type graph for errors...");
   let has_fast_check_diagnostics = collect_fast_check_type_graph_diagnostics(
     &graph,

ext/node/ops/crypto/x509.rs

@@ -106,7 +106,6 @@ pub fn op_node_x509_check_email(
 
   if let Some(subject_alt) = subject_alt {
     for name in &subject_alt.general_names {
-      dbg!(name);
       if let extensions::GeneralName::RFC822Name(n) = name {
         if *n == email {
           return Ok(true);