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

Closes https://github.com/denoland/deno/issues/22835
This commit is contained in:
David Sherret 2024-05-14 10:30:09 -04:00 committed by GitHub
parent c0a600786e
commit c6189e2070
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 254 additions and 38 deletions

View file

@ -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"))
}
}
}
}

View file

@ -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,
);
}
}
}