fix(publish): error for missing version constraints on dry-publish instead of just publish (#23798)

Closes https://github.com/denoland/deno/issues/22835

2 files changed, 148 insertions, 33 deletions

diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs
index 31f815767..49f8de045 100644
--- a/cli/tools/registry/diagnostics.rs
+++ b/cli/tools/registry/diagnostics.rs
@@ -20,6 +20,7 @@ use deno_ast::SourceTextInfo;
 use deno_core::anyhow::anyhow;
 use deno_core::error::AnyError;
 use deno_graph::FastCheckDiagnostic;
+use deno_semver::Version;
 use lsp_types::Url;
 
 use super::unfurl::SpecifierUnfurlerDiagnostic;
@@ -107,6 +108,13 @@ pub enum PublishDiagnostic {
   ExcludedModule {
     specifier: Url,
   },
+  MissingConstraint {
+    specifier: Url,
+    specifier_text: String,
+    resolved_version: Option<Version>,
+    text_info: SourceTextInfo,
+    referrer: deno_graph::Range,
+  },
 }
 
 impl PublishDiagnostic {
@@ -153,6 +161,7 @@ impl Diagnostic for PublishDiagnostic {
       InvalidExternalImport { .. } => DiagnosticLevel::Error,
       UnsupportedJsxTsx { .. } => DiagnosticLevel::Warning,
       ExcludedModule { .. } => DiagnosticLevel::Error,
+      MissingConstraint { .. } => DiagnosticLevel::Error,
     }
   }
 
@@ -167,6 +176,7 @@ impl Diagnostic for PublishDiagnostic {
       InvalidExternalImport { .. } => Cow::Borrowed("invalid-external-import"),
       UnsupportedJsxTsx { .. } => Cow::Borrowed("unsupported-jsx-tsx"),
       ExcludedModule { .. } => Cow::Borrowed("excluded-module"),
+      MissingConstraint { .. } => Cow::Borrowed("missing-constraint"),
     }
   }
 
@@ -185,10 +195,25 @@ impl Diagnostic for PublishDiagnostic {
       InvalidExternalImport { kind, .. } => Cow::Owned(format!("invalid import to a {kind} specifier")),
       UnsupportedJsxTsx { .. } => Cow::Borrowed("JSX and TSX files are currently not supported"),
       ExcludedModule { .. } => Cow::Borrowed("module in package's module graph was excluded from publishing"),
+      MissingConstraint { specifier, .. } => Cow::Owned(format!("specifier '{}' is missing a version constraint", specifier)),
     }
   }
 
   fn location(&self) -> DiagnosticLocation {
+    fn from_referrer_range<'a>(
+      referrer: &'a deno_graph::Range,
+      text_info: &'a SourceTextInfo,
+    ) -> DiagnosticLocation<'a> {
+      DiagnosticLocation::ModulePosition {
+        specifier: Cow::Borrowed(&referrer.specifier),
+        text_info: Cow::Borrowed(text_info),
+        source_pos: DiagnosticSourcePos::LineAndCol {
+          line: referrer.start.line,
+          column: referrer.start.character,
+        },
+      }
+    }
+
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.location(),
@@ -216,24 +241,49 @@ impl Diagnostic for PublishDiagnostic {
         referrer,
         text_info,
         ..
-      } => DiagnosticLocation::ModulePosition {
-        specifier: Cow::Borrowed(&referrer.specifier),
-        text_info: Cow::Borrowed(text_info),
-        source_pos: DiagnosticSourcePos::LineAndCol {
-          line: referrer.start.line,
-          column: referrer.start.character,
-        },
-      },
+      } => from_referrer_range(referrer, text_info),
       UnsupportedJsxTsx { specifier } => DiagnosticLocation::Module {
         specifier: Cow::Borrowed(specifier),
       },
       ExcludedModule { specifier } => DiagnosticLocation::Module {
         specifier: Cow::Borrowed(specifier),
       },
+      MissingConstraint {
+        referrer,
+        text_info,
+        ..
+      } => from_referrer_range(referrer, text_info),
     }
   }
 
   fn snippet(&self) -> Option<DiagnosticSnippet<'_>> {
+    fn from_range<'a>(
+      text_info: &'a SourceTextInfo,
+      referrer: &'a deno_graph::Range,
+    ) -> Option<DiagnosticSnippet<'a>> {
+      if referrer.start.line == 0 && referrer.start.character == 0 {
+        return None; // no range, probably a jsxImportSource import
+      }
+
+      Some(DiagnosticSnippet {
+        source: Cow::Borrowed(text_info),
+        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()),
+        },
+      })
+    }
+
     use PublishDiagnostic::*;
     match &self {
       FastCheck(diagnostic) => diagnostic.snippet(),
@@ -261,25 +311,14 @@ impl Diagnostic for PublishDiagnostic {
         referrer,
         text_info,
         ..
-      } => Some(DiagnosticSnippet {
-        source: Cow::Borrowed(text_info),
-        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()),
-        },
-      }),
+      } => from_range(text_info, referrer),
       UnsupportedJsxTsx { .. } => None,
       ExcludedModule { .. } => None,
+      MissingConstraint {
+        referrer,
+        text_info,
+        ..
+      } => from_range(text_info, referrer),
     }
   }
 
@@ -302,6 +341,13 @@ impl Diagnostic for PublishDiagnostic {
       ExcludedModule { .. } => Some(
         Cow::Borrowed("remove the module from 'exclude' and/or 'publish.exclude' in the config file or use 'publish.exclude' with a negative glob to unexclude from gitignore"),
       ),
+      MissingConstraint { specifier_text, .. } => {
+        Some(Cow::Borrowed(if specifier_text.starts_with("jsr:") || specifier_text.starts_with("npm:") {
+          "specify a version constraint for the specifier"
+        } else {
+          "specify a version constraint for the specifier in the import map"
+        }))
+      },
     }
   }
 
@@ -366,7 +412,14 @@ impl Diagnostic for PublishDiagnostic {
       ]),
       ExcludedModule { .. } => Cow::Owned(vec![
         Cow::Borrowed("excluded modules referenced via a package export will error at runtime due to not existing in the package"),
-      ])
+      ]),
+      MissingConstraint { resolved_version, .. } => Cow::Owned(vec![
+        Cow::Owned(format!(
+          "the specifier resolved to version {} today, but will resolve to a different",
+          resolved_version.as_ref().map(|v| v.to_string()).unwrap_or_else(|| "<unresolved>".to_string())),
+        ),
+        Cow::Borrowed("major version if one is published in the future and potentially break"),
+      ]),
     }
   }
 
@@ -393,6 +446,9 @@ impl Diagnostic for PublishDiagnostic {
       ExcludedModule { .. } => {
         Some(Cow::Borrowed("https://jsr.io/go/excluded-module"))
       }
+      MissingConstraint { .. } => {
+        Some(Cow::Borrowed("https://jsr.io/go/missing-constraint"))
+      }
     }
   }
 }
diff --git a/cli/tools/registry/graph.rs b/cli/tools/registry/graph.rs
index 001f85e76..7e3239ced 100644
--- a/cli/tools/registry/graph.rs
+++ b/cli/tools/registry/graph.rs
@@ -8,6 +8,8 @@ use deno_graph::ModuleEntryRef;
 use deno_graph::ModuleGraph;
 use deno_graph::ResolutionResolved;
 use deno_graph::WalkOptions;
+use deno_semver::jsr::JsrPackageReqReference;
+use deno_semver::npm::NpmPackageReqReference;
 use lsp_types::Url;
 
 use super::diagnostics::PublishDiagnostic;
@@ -22,20 +24,67 @@ pub fn collect_invalid_external_imports(
 
   let mut collect_if_invalid =
     |skip_specifiers: &mut HashSet<Url>,
-     text: &Arc<str>,
+     source_text: &Arc<str>,
+     specifier_text: &str,
      resolution: &ResolutionResolved| {
       if visited.insert(resolution.specifier.clone()) {
         match resolution.specifier.scheme() {
           "file" | "data" | "node" => {}
-          "jsr" | "npm" => {
+          "jsr" => {
             skip_specifiers.insert(resolution.specifier.clone());
+
+            // check for a missing version constraint
+            if let Ok(jsr_req_ref) =
+              JsrPackageReqReference::from_specifier(&resolution.specifier)
+            {
+              if jsr_req_ref.req().version_req.version_text() == "*" {
+                let maybe_version = graph
+                  .packages
+                  .mappings()
+                  .find(|(req, _)| *req == jsr_req_ref.req())
+                  .map(|(_, nv)| nv.version.clone());
+                diagnostics_collector.push(
+                  PublishDiagnostic::MissingConstraint {
+                    specifier: resolution.specifier.clone(),
+                    specifier_text: specifier_text.to_string(),
+                    resolved_version: maybe_version,
+                    text_info: SourceTextInfo::new(source_text.clone()),
+                    referrer: resolution.range.clone(),
+                  },
+                );
+              }
+            }
+          }
+          "npm" => {
+            skip_specifiers.insert(resolution.specifier.clone());
+
+            // check for a missing version constraint
+            if let Ok(jsr_req_ref) =
+              NpmPackageReqReference::from_specifier(&resolution.specifier)
+            {
+              if jsr_req_ref.req().version_req.version_text() == "*" {
+                let maybe_version = graph
+                  .get(&resolution.specifier)
+                  .and_then(|m| m.npm())
+                  .map(|n| n.nv_reference.nv().version.clone());
+                diagnostics_collector.push(
+                  PublishDiagnostic::MissingConstraint {
+                    specifier: resolution.specifier.clone(),
+                    specifier_text: specifier_text.to_string(),
+                    resolved_version: maybe_version,
+                    text_info: SourceTextInfo::new(source_text.clone()),
+                    referrer: resolution.range.clone(),
+                  },
+                );
+              }
+            }
           }
           "http" | "https" => {
             skip_specifiers.insert(resolution.specifier.clone());
             diagnostics_collector.push(
               PublishDiagnostic::InvalidExternalImport {
                 kind: format!("non-JSR '{}'", resolution.specifier.scheme()),
-                text_info: SourceTextInfo::new(text.clone()),
+                text_info: SourceTextInfo::new(source_text.clone()),
                 imported: resolution.specifier.clone(),
                 referrer: resolution.range.clone(),
               },
@@ -46,7 +95,7 @@ pub fn collect_invalid_external_imports(
             diagnostics_collector.push(
               PublishDiagnostic::InvalidExternalImport {
                 kind: format!("'{}'", resolution.specifier.scheme()),
-                text_info: SourceTextInfo::new(text.clone()),
+                text_info: SourceTextInfo::new(source_text.clone()),
                 imported: resolution.specifier.clone(),
                 referrer: resolution.range.clone(),
               },
@@ -77,12 +126,22 @@ pub fn collect_invalid_external_imports(
       continue;
     };
 
-    for (_, dep) in &module.dependencies {
+    for (specifier_text, dep) in &module.dependencies {
       if let Some(resolved) = dep.maybe_code.ok() {
-        collect_if_invalid(&mut skip_specifiers, &module.source, resolved);
+        collect_if_invalid(
+          &mut skip_specifiers,
+          &module.source,
+          specifier_text,
+          resolved,
+        );
       }
       if let Some(resolved) = dep.maybe_type.ok() {
-        collect_if_invalid(&mut skip_specifiers, &module.source, resolved);
+        collect_if_invalid(
+          &mut skip_specifiers,
+          &module.source,
+          specifier_text,
+          resolved,
+        );
       }
     }
   }