diff options
16 files changed, 254 insertions, 38 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, + ); } } } diff --git a/tests/specs/jsr/no_unused_params/main.ts b/tests/specs/jsr/no_unused_params/main.ts index e8b12ca89..54665c107 100644 --- a/tests/specs/jsr/no_unused_params/main.ts +++ b/tests/specs/jsr/no_unused_params/main.ts @@ -1,4 +1,4 @@ -import * as inner from "jsr:@denotest/add"; +import * as inner from "jsr:@denotest/add@1"; export function add(a: number, b: number): number { return inner.add(a, b); diff --git a/tests/specs/publish/missing_constraint/__test__.jsonc b/tests/specs/publish/missing_constraint/__test__.jsonc new file mode 100644 index 000000000..06a91f5b6 --- /dev/null +++ b/tests/specs/publish/missing_constraint/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "publish --dry-run", + "output": "publish.out", + "exitCode": 1 +} diff --git a/tests/specs/publish/missing_constraint/deno.json b/tests/specs/publish/missing_constraint/deno.json new file mode 100644 index 000000000..89f6db90c --- /dev/null +++ b/tests/specs/publish/missing_constraint/deno.json @@ -0,0 +1,9 @@ +{ + "name": "@scope/pkg", + "version": "1.0.0", + "exports": "./mod.ts", + "imports": { + "basic": "npm:@denotest/esm-basic", + "add": "jsr:@denotest/add" + } +} diff --git a/tests/specs/publish/missing_constraint/mod.ts b/tests/specs/publish/missing_constraint/mod.ts new file mode 100644 index 000000000..59e40d241 --- /dev/null +++ b/tests/specs/publish/missing_constraint/mod.ts @@ -0,0 +1,7 @@ +import { add } from "add"; +import * as basic from "basic"; +import * as deps from "jsr:@denotest/deps"; + +console.log(add(1, 2)); +console.log(deps); +console.log(basic); diff --git a/tests/specs/publish/missing_constraint/publish.out b/tests/specs/publish/missing_constraint/publish.out new file mode 100644 index 000000000..846612979 --- /dev/null +++ b/tests/specs/publish/missing_constraint/publish.out @@ -0,0 +1,37 @@ +[WILDCARD] +Checking for slow types in the public API... +Check file:///[WILDLINE]/mod.ts +error[missing-constraint]: specifier 'jsr:@denotest/add' is missing a version constraint + --> [WILDLINE]mod.ts:[WILDLINE] + | +1 | import { add } from "add"; + | ^^^^^ the specifier + = hint: specify a version constraint for the specifier in the import map + + info: the specifier resolved to version 1.0.0 today, but will resolve to a different + info: major version if one is published in the future and potentially break + docs: https://jsr.io/go/missing-constraint + +error[missing-constraint]: specifier 'npm:@denotest/esm-basic' is missing a version constraint + --> [WILDLINE]mod.ts:[WILDLINE] + | +2 | import * as basic from "basic"; + | ^^^^^^^ the specifier + = hint: specify a version constraint for the specifier in the import map + + info: the specifier resolved to version 1.0.0 today, but will resolve to a different + info: major version if one is published in the future and potentially break + docs: https://jsr.io/go/missing-constraint + +error[missing-constraint]: specifier 'jsr:@denotest/deps' is missing a version constraint + --> [WILDLINE]mod.ts:[WILDLINE] + | +3 | import * as deps from "jsr:@denotest/deps"; + | ^^^^^^^^^^^^^^^^^^^^ the specifier + = hint: specify a version constraint for the specifier + + info: the specifier resolved to version 1.0.0 today, but will resolve to a different + info: major version if one is published in the future and potentially break + docs: https://jsr.io/go/missing-constraint + +error: Found 3 problems diff --git a/tests/specs/publish/missing_constraint_jsx_import_source/__test__.jsonc b/tests/specs/publish/missing_constraint_jsx_import_source/__test__.jsonc new file mode 100644 index 000000000..1b96278a0 --- /dev/null +++ b/tests/specs/publish/missing_constraint_jsx_import_source/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "args": "publish --token 'sadfasdf'", + "output": "mod.out", + "exitCode": 1 +} diff --git a/tests/specs/publish/missing_constraint_jsx_import_source/foo.tsx b/tests/specs/publish/missing_constraint_jsx_import_source/foo.tsx new file mode 100644 index 000000000..120e8b334 --- /dev/null +++ b/tests/specs/publish/missing_constraint_jsx_import_source/foo.tsx @@ -0,0 +1,5 @@ +import { renderToString } from "npm:preact-render-to-string@6"; + +export default function render() { + return renderToString(<div>foo.tsx</div>); +} diff --git a/tests/specs/publish/missing_constraint_jsx_import_source/jsr.jsonc b/tests/specs/publish/missing_constraint_jsx_import_source/jsr.jsonc new file mode 100644 index 000000000..7aea08842 --- /dev/null +++ b/tests/specs/publish/missing_constraint_jsx_import_source/jsr.jsonc @@ -0,0 +1,11 @@ +{ + "name": "@foo/bar", + "version": "1.0.0", + "exports": { + ".": "./mod.ts" + }, + "compilerOptions": { + "jsx": "react-jsx", + "jsxImportSource": "npm:preact" + } +} diff --git a/tests/specs/publish/missing_constraint_jsx_import_source/mod.out b/tests/specs/publish/missing_constraint_jsx_import_source/mod.out new file mode 100644 index 000000000..d1da06be8 --- /dev/null +++ b/tests/specs/publish/missing_constraint_jsx_import_source/mod.out @@ -0,0 +1,17 @@ +[WILDCARD] +Checking for slow types in the public API... +Check file:///[WILDCARD]/mod.ts +error[missing-constraint]: specifier 'npm:preact/jsx-runtime' is missing a version constraint + --> [WILDLINE] + = hint: specify a version constraint for the specifier + + info: the specifier resolved to version 10.19.6 today, but will resolve to a different + info: major version if one is published in the future and potentially break + docs: https://jsr.io/go/missing-constraint + +warning[unsupported-jsx-tsx]: JSX and TSX files are currently not supported + --> [WILDLINE]foo.tsx + + info: follow https://github.com/jsr-io/jsr/issues/24 for updates + +error: Found 1 problem diff --git a/tests/specs/publish/missing_constraint_jsx_import_source/mod.ts b/tests/specs/publish/missing_constraint_jsx_import_source/mod.ts new file mode 100644 index 000000000..9935bf5ee --- /dev/null +++ b/tests/specs/publish/missing_constraint_jsx_import_source/mod.ts @@ -0,0 +1,5 @@ +import fooTsx from "./foo.tsx"; + +export function renderTsx() { + console.log(fooTsx()); +} diff --git a/tests/specs/publish/package_json/package.json b/tests/specs/publish/package_json/package.json index c1b171f4c..4239110bd 100644 --- a/tests/specs/publish/package_json/package.json +++ b/tests/specs/publish/package_json/package.json @@ -2,6 +2,6 @@ "name": "@deno/foo", "version": "0.0.1", "dependencies": { - "picocolors": "*" + "picocolors": "1" } } diff --git a/tests/specs/publish/unsupported_jsx_tsx/foo.jsx b/tests/specs/publish/unsupported_jsx_tsx/foo.jsx index 021c2d49e..120e8b334 100644 --- a/tests/specs/publish/unsupported_jsx_tsx/foo.jsx +++ b/tests/specs/publish/unsupported_jsx_tsx/foo.jsx @@ -1,4 +1,4 @@ -import { renderToString } from "npm:preact-render-to-string"; +import { renderToString } from "npm:preact-render-to-string@6"; export default function render() { return renderToString(<div>foo.tsx</div>); diff --git a/tests/specs/publish/unsupported_jsx_tsx/foo.tsx b/tests/specs/publish/unsupported_jsx_tsx/foo.tsx index 021c2d49e..120e8b334 100644 --- a/tests/specs/publish/unsupported_jsx_tsx/foo.tsx +++ b/tests/specs/publish/unsupported_jsx_tsx/foo.tsx @@ -1,4 +1,4 @@ -import { renderToString } from "npm:preact-render-to-string"; +import { renderToString } from "npm:preact-render-to-string@6"; export default function render() { return renderToString(<div>foo.tsx</div>); diff --git a/tests/specs/publish/unsupported_jsx_tsx/jsr.jsonc b/tests/specs/publish/unsupported_jsx_tsx/jsr.jsonc index 7aea08842..e411bf54b 100644 --- a/tests/specs/publish/unsupported_jsx_tsx/jsr.jsonc +++ b/tests/specs/publish/unsupported_jsx_tsx/jsr.jsonc @@ -6,6 +6,6 @@ }, "compilerOptions": { "jsx": "react-jsx", - "jsxImportSource": "npm:preact" + "jsxImportSource": "npm:preact@10" } } |