From 8793dce0c401831f78a9a259b1d8e6a64584c280 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Thu, 15 Jan 2026 20:26:46 +0100 Subject: [PATCH 01/10] feat(core): constraint validation infrastructure --- .../steadfast-princess-vainamoinen.md | 9 ++ AGENTS.md | 1 + crates/sampo-core/src/adapters.rs | 35 ++++- crates/sampo-core/src/adapters/cargo.rs | 11 ++ crates/sampo-core/src/adapters/hex.rs | 12 ++ crates/sampo-core/src/adapters/npm.rs | 12 ++ crates/sampo-core/src/adapters/pypi.rs | 12 ++ crates/sampo-core/src/errors.rs | 3 + crates/sampo-core/src/lib.rs | 5 +- crates/sampo-core/src/release.rs | 113 +++++++++++++- crates/sampo-core/src/types.rs | 139 ++++++++++++++++++ 11 files changed, 347 insertions(+), 5 deletions(-) create mode 100644 .sampo/changesets/steadfast-princess-vainamoinen.md diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md new file mode 100644 index 0000000..916d272 --- /dev/null +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -0,0 +1,9 @@ +--- +cargo/sampo: minor +cargo/sampo-core: minor +cargo/sampo-github-action: minor +--- + +**⚠️ breaking change:** Add infrastructure for validating dependency version constraints during release planning. This enables early detection of constraint violations before file modifications, with errors for fixed/linked packages and warnings for others. + +**⚠️ Warning:** Ecosystem-specific constraint parsing and validation logic is not yet implemented; current methods return "Skipped" stubs. diff --git a/AGENTS.md b/AGENTS.md index 0f0e272..cc3f002 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,7 @@ Principles for how automated agents and contributors generate code and docs here - Keep tests deterministic and independent of global state. ## Errors + - Common errors live in `sampo-core`'s `errors.rs`, while crate-specific errors live in their respective crates' `error.rs`. - Use typed error enums with `thiserror` for a stable, explicit API. - Keep error messages concise and in English; add context at the boundary (CLI/action) rather than deep in core. diff --git a/crates/sampo-core/src/adapters.rs b/crates/sampo-core/src/adapters.rs index 0ba1533..b835f53 100644 --- a/crates/sampo-core/src/adapters.rs +++ b/crates/sampo-core/src/adapters.rs @@ -8,7 +8,7 @@ pub mod pypi; pub use cargo::ManifestMetadata; use crate::errors::{Result, WorkspaceError}; -use crate::types::{PackageInfo, PackageKind}; +use crate::types::{ConstraintCheckResult, PackageInfo, PackageKind}; use std::collections::BTreeMap; use std::path::Path; @@ -221,4 +221,37 @@ impl PackageAdapter { PackageKind::Packagist => Self::Packagist, } } + + /// Check if a dependency version constraint is satisfied by a new version. + pub fn check_dependency_constraint( + &self, + manifest_path: &Path, + dep_name: &str, + current_constraint: &str, + new_version: &str, + ) -> Result { + match self { + Self::Cargo => { + cargo::check_dependency_constraint(dep_name, current_constraint, new_version) + } + Self::Npm => npm::check_dependency_constraint( + manifest_path, + dep_name, + current_constraint, + new_version, + ), + Self::Hex => hex::check_dependency_constraint( + manifest_path, + dep_name, + current_constraint, + new_version, + ), + Self::PyPI => pypi::check_dependency_constraint( + manifest_path, + dep_name, + current_constraint, + new_version, + ), + } + } } diff --git a/crates/sampo-core/src/adapters/cargo.rs b/crates/sampo-core/src/adapters/cargo.rs index 55ac7c7..99c4ed1 100644 --- a/crates/sampo-core/src/adapters/cargo.rs +++ b/crates/sampo-core/src/adapters/cargo.rs @@ -79,6 +79,17 @@ impl CargoAdapter { } } +/// Stub: returns `Skipped` until Cargo semver constraint parsing is implemented. +pub(super) fn check_dependency_constraint( + _dep_name: &str, + _current_constraint: &str, + _new_version: &str, +) -> Result { + Ok(crate::types::ConstraintCheckResult::Skipped { + reason: "Cargo constraint validation not yet implemented".to_string(), + }) +} + /// Detect the version of the `cargo` binary available on the PATH. pub fn detect_version() -> Result> { let output = match Command::new("cargo").arg("--version").output() { diff --git a/crates/sampo-core/src/adapters/hex.rs b/crates/sampo-core/src/adapters/hex.rs index 023339b..ee227d5 100644 --- a/crates/sampo-core/src/adapters/hex.rs +++ b/crates/sampo-core/src/adapters/hex.rs @@ -120,6 +120,18 @@ impl HexAdapter { } } +/// Stub: returns `Skipped` until Hex/Elixir constraint parsing is implemented. +pub(super) fn check_dependency_constraint( + _manifest_path: &Path, + _dep_name: &str, + _current_constraint: &str, + _new_version: &str, +) -> Result { + Ok(crate::types::ConstraintCheckResult::Skipped { + reason: "Hex constraint validation not yet implemented".to_string(), + }) +} + pub(super) fn publish_dry_run( packages: &[(&PackageInfo, &Path)], extra_args: &[String], diff --git a/crates/sampo-core/src/adapters/npm.rs b/crates/sampo-core/src/adapters/npm.rs index f32413f..0ee430f 100644 --- a/crates/sampo-core/src/adapters/npm.rs +++ b/crates/sampo-core/src/adapters/npm.rs @@ -178,6 +178,18 @@ impl NpmAdapter { } } +/// Stub: returns `Skipped` until npm semver constraint parsing is implemented. +pub(super) fn check_dependency_constraint( + _manifest_path: &Path, + _dep_name: &str, + _current_constraint: &str, + _new_version: &str, +) -> Result { + Ok(crate::types::ConstraintCheckResult::Skipped { + reason: "npm constraint validation not yet implemented".to_string(), + }) +} + pub(super) fn publish_dry_run( packages: &[(&PackageInfo, &Path)], extra_args: &[String], diff --git a/crates/sampo-core/src/adapters/pypi.rs b/crates/sampo-core/src/adapters/pypi.rs index 9a1044a..a0f9324 100644 --- a/crates/sampo-core/src/adapters/pypi.rs +++ b/crates/sampo-core/src/adapters/pypi.rs @@ -144,6 +144,18 @@ impl PyPIAdapter { } } +/// Stub: returns `Skipped` until PyPI/PEP 440 constraint parsing is implemented. +pub(super) fn check_dependency_constraint( + _manifest_path: &Path, + _dep_name: &str, + _current_constraint: &str, + _new_version: &str, +) -> Result { + Ok(crate::types::ConstraintCheckResult::Skipped { + reason: "PyPI constraint validation not yet implemented".to_string(), + }) +} + pub(super) fn publish_dry_run( packages: &[(&PackageInfo, &Path)], extra_args: &[String], diff --git a/crates/sampo-core/src/errors.rs b/crates/sampo-core/src/errors.rs index 4ac1966..3d5327c 100644 --- a/crates/sampo-core/src/errors.rs +++ b/crates/sampo-core/src/errors.rs @@ -34,6 +34,9 @@ pub enum SampoError { #[error("Pre-release error: {0}")] Prerelease(String), + #[error("Constraint violation: {0}")] + ConstraintViolation(String), + #[error("Invalid data: {0}")] InvalidData(String), diff --git a/crates/sampo-core/src/lib.rs b/crates/sampo-core/src/lib.rs index e4a1e4f..2d2a275 100644 --- a/crates/sampo-core/src/lib.rs +++ b/crates/sampo-core/src/lib.rs @@ -42,8 +42,9 @@ pub use release::{ infer_bump_from_versions, run_release, }; pub use types::{ - Bump, ChangelogCategory, DependencyUpdate, PackageInfo, PackageKind, ParsedChangeType, - PublishOutput, ReleaseOutput, ReleasedPackage, Workspace, + Bump, ChangelogCategory, ConstraintCheckResult, ConstraintViolation, DependencyUpdate, + PackageInfo, PackageKind, ParsedChangeType, PublishOutput, ReleaseOutput, ReleasedPackage, + Workspace, }; pub use workspace::{discover_packages_at, discover_workspace, find_sampo_root}; diff --git a/crates/sampo-core/src/release.rs b/crates/sampo-core/src/release.rs index 068ad08..2c71848 100644 --- a/crates/sampo-core/src/release.rs +++ b/crates/sampo-core/src/release.rs @@ -2,8 +2,9 @@ use crate::adapters::{ManifestMetadata, PackageAdapter}; use crate::errors::{Result, SampoError, io_error_with_path}; use crate::filters::should_ignore_package; use crate::types::{ - Bump, ChangelogCategory, DependencyUpdate, PackageInfo, PackageKind, PackageSpecifier, - ReleaseOutput, ReleasedPackage, SpecResolution, Workspace, format_ambiguity_options, + Bump, ChangelogCategory, ConstraintCheckResult, ConstraintViolation, DependencyUpdate, + PackageInfo, PackageKind, PackageSpecifier, ReleaseOutput, ReleasedPackage, SpecResolution, + Workspace, format_ambiguity_options, }; use crate::{ changeset::{ChangesetInfo, parse_changeset, render_changeset_markdown_with_tags}, @@ -674,6 +675,14 @@ fn compute_plan_state( return Ok(PlanOutcome::NoMatchingCrates); } + // Validate dependency constraints before proceeding with the release. + // This checks that internal dependency version constraints will be satisfied + // after the planned bumps. Returns error for fixed/linked packages, warnings otherwise. + let constraint_warnings = validate_dependency_constraints(&releases, workspace, config)?; + for warning in &constraint_warnings { + eprintln!("{}", warning); + } + let released_packages: Vec = releases .iter() .map(|(name, old_version, new_version)| { @@ -700,6 +709,106 @@ fn compute_plan_state( })) } +/// Validates dependency constraints before applying releases. +/// Returns error for fixed/linked packages with violations, warnings otherwise. +#[allow(dead_code)] // Infrastructure for future use +fn validate_dependency_constraints( + releases: &ReleasePlan, + workspace: &Workspace, + config: &Config, +) -> Result> { + let new_version_by_id: BTreeMap = releases + .iter() + .map(|(id, _, new_ver)| (id.clone(), new_ver.clone())) + .collect(); + + let by_id: BTreeMap = workspace + .members + .iter() + .map(|pkg| (pkg.canonical_identifier().to_string(), pkg)) + .collect(); + + let fixed_groups = + resolve_config_groups(workspace, &config.fixed_dependencies, "packages.fixed")?; + let linked_groups = + resolve_config_groups(workspace, &config.linked_dependencies, "packages.linked")?; + + let mut violations: Vec = Vec::new(); + let mut warnings: Vec = Vec::new(); + + for (pkg_id, _, _) in releases { + let Some(pkg_info) = by_id.get(pkg_id) else { + continue; + }; + + let adapter = PackageAdapter::from_kind(pkg_info.kind); + let manifest_path = adapter.manifest_path(&pkg_info.path); + + for dep_id in &pkg_info.internal_deps { + let Some(new_dep_version) = new_version_by_id.get(dep_id) else { + continue; + }; + + let Some(dep_info) = by_id.get(dep_id) else { + continue; + }; + + // Placeholder until per-adapter constraint extraction is implemented. + // Note: adapters receive the package name and must handle alias resolution + // themselves when parsing the manifest (e.g., `alias = { package = "name" }`). + let constraint_placeholder = "*"; + + let check_result = adapter.check_dependency_constraint( + &manifest_path, + &dep_info.name, + constraint_placeholder, + new_dep_version, + )?; + + match check_result { + ConstraintCheckResult::Satisfied => {} + ConstraintCheckResult::Skipped { .. } => {} + ConstraintCheckResult::NotSatisfied { + constraint, + new_version, + } => { + let violation = ConstraintViolation { + dependent_identifier: pkg_id.clone(), + dependency_identifier: dep_id.clone(), + dependency_name: dep_info.name.clone(), + constraint: constraint.clone(), + new_version: new_version.clone(), + }; + + let is_fixed_or_linked = is_in_group(pkg_id, &fixed_groups) + || is_in_group(pkg_id, &linked_groups); + + if is_fixed_or_linked { + violations.push(violation); + } else { + warnings.push(format!("Warning: {}", violation)); + } + } + } + } + } + + if !violations.is_empty() { + let messages: Vec = violations.iter().map(|v| v.to_string()).collect(); + return Err(SampoError::ConstraintViolation(format!( + "Version constraint violations in fixed/linked packages:\n - {}", + messages.join("\n - ") + ))); + } + + Ok(warnings) +} + +/// Check if a package identifier is in any of the provided groups. +fn is_in_group(pkg_id: &str, groups: &[Vec]) -> bool { + groups.iter().any(|group| group.contains(&pkg_id.to_string())) +} + fn releases_include_prerelease(releases: &ReleasePlan) -> bool { releases.iter().any(|(_, _, new_version)| { Version::parse(new_version) diff --git a/crates/sampo-core/src/types.rs b/crates/sampo-core/src/types.rs index be0cd10..ba7fb5f 100644 --- a/crates/sampo-core/src/types.rs +++ b/crates/sampo-core/src/types.rs @@ -446,6 +446,70 @@ impl std::fmt::Display for ChangelogCategory { } } +/// Result of checking a dependency version constraint against a new version. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ConstraintCheckResult { + Satisfied, + NotSatisfied { + constraint: String, + new_version: String, + }, + Skipped { + reason: String, + }, +} + +impl ConstraintCheckResult { + /// Returns true if the constraint check passed (satisfied or skipped). + pub fn is_ok(&self) -> bool { + matches!(self, Self::Satisfied | Self::Skipped { .. }) + } + + /// Returns true if the constraint is not satisfied. + pub fn is_not_satisfied(&self) -> bool { + matches!(self, Self::NotSatisfied { .. }) + } +} + +impl std::fmt::Display for ConstraintCheckResult { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Satisfied => write!(f, "satisfied"), + Self::NotSatisfied { + constraint, + new_version, + } => { + write!( + f, + "not satisfied: version {} does not match constraint '{}'", + new_version, constraint + ) + } + Self::Skipped { reason } => write!(f, "skipped: {}", reason), + } + } +} + +/// Information about a dependency constraint validation failure. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConstraintViolation { + pub dependent_identifier: String, + pub dependency_identifier: String, + pub dependency_name: String, + pub constraint: String, + pub new_version: String, +} + +impl std::fmt::Display for ConstraintViolation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{} depends on {} with constraint '{}', but new version {} does not satisfy it", + self.dependent_identifier, self.dependency_name, self.constraint, self.new_version + ) + } +} + /// Result of parsing a change type string that may include a custom tag. /// /// Parses formats like: @@ -673,4 +737,79 @@ mod tests { Bump::Patch ); } + + #[test] + fn constraint_check_result_is_ok() { + assert!(ConstraintCheckResult::Satisfied.is_ok()); + assert!( + ConstraintCheckResult::Skipped { + reason: "test".to_string() + } + .is_ok() + ); + assert!( + !ConstraintCheckResult::NotSatisfied { + constraint: "^1.0".to_string(), + new_version: "2.0.0".to_string(), + } + .is_ok() + ); + } + + #[test] + fn constraint_check_result_is_not_satisfied() { + assert!(!ConstraintCheckResult::Satisfied.is_not_satisfied()); + assert!( + !ConstraintCheckResult::Skipped { + reason: "test".to_string() + } + .is_not_satisfied() + ); + assert!( + ConstraintCheckResult::NotSatisfied { + constraint: "^1.0".to_string(), + new_version: "2.0.0".to_string(), + } + .is_not_satisfied() + ); + } + + #[test] + fn constraint_check_result_display() { + assert_eq!(format!("{}", ConstraintCheckResult::Satisfied), "satisfied"); + assert_eq!( + format!( + "{}", + ConstraintCheckResult::Skipped { + reason: "not implemented".to_string() + } + ), + "skipped: not implemented" + ); + assert_eq!( + format!( + "{}", + ConstraintCheckResult::NotSatisfied { + constraint: "^1.0.0".to_string(), + new_version: "2.0.0".to_string(), + } + ), + "not satisfied: version 2.0.0 does not match constraint '^1.0.0'" + ); + } + + #[test] + fn constraint_violation_display() { + let violation = ConstraintViolation { + dependent_identifier: "cargo/app".to_string(), + dependency_identifier: "cargo/lib".to_string(), + dependency_name: "lib".to_string(), + constraint: "^1.0.0".to_string(), + new_version: "2.0.0".to_string(), + }; + assert_eq!( + format!("{}", violation), + "cargo/app depends on lib with constraint '^1.0.0', but new version 2.0.0 does not satisfy it" + ); + } } From 28d39a7ae956100722b102b04a37a66e41646bc5 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Thu, 15 Jan 2026 20:27:00 +0100 Subject: [PATCH 02/10] format --- crates/sampo-core/src/release.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/sampo-core/src/release.rs b/crates/sampo-core/src/release.rs index 2c71848..c8075b3 100644 --- a/crates/sampo-core/src/release.rs +++ b/crates/sampo-core/src/release.rs @@ -780,8 +780,8 @@ fn validate_dependency_constraints( new_version: new_version.clone(), }; - let is_fixed_or_linked = is_in_group(pkg_id, &fixed_groups) - || is_in_group(pkg_id, &linked_groups); + let is_fixed_or_linked = + is_in_group(pkg_id, &fixed_groups) || is_in_group(pkg_id, &linked_groups); if is_fixed_or_linked { violations.push(violation); @@ -806,7 +806,9 @@ fn validate_dependency_constraints( /// Check if a package identifier is in any of the provided groups. fn is_in_group(pkg_id: &str, groups: &[Vec]) -> bool { - groups.iter().any(|group| group.contains(&pkg_id.to_string())) + groups + .iter() + .any(|group| group.contains(&pkg_id.to_string())) } fn releases_include_prerelease(releases: &ReleasePlan) -> bool { From b5e596c10fc9cef66eb3364f2fff56f89b92cca9 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Sat, 17 Jan 2026 16:55:56 +0100 Subject: [PATCH 03/10] cargo constraints --- .../steadfast-princess-vainamoinen.md | 4 +- crates/sampo-core/src/adapters.rs | 6 + crates/sampo-core/src/adapters/cargo.rs | 57 ++++++- .../src/adapters/cargo/cargo_tests.rs | 155 ++++++++++++++++++ crates/sampo-core/src/adapters/packagist.rs | 11 ++ crates/sampo-github-action/src/sampo.rs | 21 ++- crates/sampo/README.md | 3 + 7 files changed, 242 insertions(+), 15 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index 916d272..74f8fd0 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -4,6 +4,6 @@ cargo/sampo-core: minor cargo/sampo-github-action: minor --- -**⚠️ breaking change:** Add infrastructure for validating dependency version constraints during release planning. This enables early detection of constraint violations before file modifications, with errors for fixed/linked packages and warnings for others. +**⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump don't satisfies a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. -**⚠️ Warning:** Ecosystem-specific constraint parsing and validation logic is not yet implemented; current methods return "Skipped" stubs. +**Note:** Constraint validation is fully implemented, but not wired to release planning. Other ecosystems (npm, Hex, PyPI) will skip validation with an informative message until constraint extraction is implemented for them. diff --git a/crates/sampo-core/src/adapters.rs b/crates/sampo-core/src/adapters.rs index b835f53..ab33542 100644 --- a/crates/sampo-core/src/adapters.rs +++ b/crates/sampo-core/src/adapters.rs @@ -252,6 +252,12 @@ impl PackageAdapter { current_constraint, new_version, ), + Self::Packagist => packagist::check_dependency_constraint( + manifest_path, + dep_name, + current_constraint, + new_version, + ), } } } diff --git a/crates/sampo-core/src/adapters/cargo.rs b/crates/sampo-core/src/adapters/cargo.rs index 99c4ed1..06e7921 100644 --- a/crates/sampo-core/src/adapters/cargo.rs +++ b/crates/sampo-core/src/adapters/cargo.rs @@ -79,15 +79,43 @@ impl CargoAdapter { } } -/// Stub: returns `Skipped` until Cargo semver constraint parsing is implemented. +/// Check if a Cargo dependency version constraint is satisfied by a new version. pub(super) fn check_dependency_constraint( _dep_name: &str, - _current_constraint: &str, - _new_version: &str, + current_constraint: &str, + new_version: &str, ) -> Result { - Ok(crate::types::ConstraintCheckResult::Skipped { - reason: "Cargo constraint validation not yet implemented".to_string(), - }) + use crate::types::ConstraintCheckResult; + + let constraint = current_constraint.trim(); + let version_str = new_version.trim(); + + let req = match VersionReq::parse(constraint) { + Ok(r) => r, + Err(_) => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable constraint '{}'", constraint), + }); + } + }; + + let version = match Version::parse(version_str) { + Ok(v) => v, + Err(_) => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable version '{}'", version_str), + }); + } + }; + + if req.matches(&version) { + Ok(ConstraintCheckResult::Satisfied) + } else { + Ok(ConstraintCheckResult::NotSatisfied { + constraint: constraint.to_string(), + new_version: version_str.to_string(), + }) + } } /// Detect the version of the `cargo` binary available on the PATH. @@ -366,6 +394,7 @@ struct MetadataDependency { package_name: String, kind: DependencyKind, target: Option, + version_req: String, } impl ManifestMetadata { @@ -402,6 +431,7 @@ impl ManifestMetadata { package_name: dep.name.clone(), kind: dep.kind, target: dep.target.as_ref().map(|platform| platform.to_string()), + version_req: dep.req.to_string(), }) .collect(); @@ -427,6 +457,21 @@ impl ManifestMetadata { fn is_workspace_package(&self, name: &str) -> bool { self.by_name.contains_key(name) } + + /// Returns the version constraint for a dependency of a package, if found. + /// The `dep_name` is the package name (not the alias/manifest key). + pub fn get_dependency_constraint( + &self, + manifest_path: &Path, + dep_name: &str, + ) -> Option { + let package = self.package_for_manifest(manifest_path)?; + package + .dependencies + .iter() + .find(|dep| dep.package_name == dep_name) + .map(|dep| dep.version_req.clone()) + } } /// Update a Cargo manifest by setting the package version (if provided) and retargeting internal diff --git a/crates/sampo-core/src/adapters/cargo/cargo_tests.rs b/crates/sampo-core/src/adapters/cargo/cargo_tests.rs index 4d37f27..25cdf6d 100644 --- a/crates/sampo-core/src/adapters/cargo/cargo_tests.rs +++ b/crates/sampo-core/src/adapters/cargo/cargo_tests.rs @@ -314,3 +314,158 @@ fn cargo_discoverer_rejects_workspace_inheritance_without_workspace_version() { let err = result.unwrap_err().to_string(); assert!(err.contains("version.workspace = true requires workspace.package.version")); } + +mod constraint_validation { + use super::*; + use crate::types::ConstraintCheckResult; + + fn assert_satisfied(constraint: &str, version: &str) { + let result = check_dependency_constraint("test-dep", constraint, version).unwrap(); + assert_eq!( + result, + ConstraintCheckResult::Satisfied, + "Expected constraint '{}' to be satisfied by version '{}', got {:?}", + constraint, + version, + result + ); + } + + fn assert_not_satisfied(constraint: &str, version: &str) { + let result = check_dependency_constraint("test-dep", constraint, version).unwrap(); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "Expected constraint '{}' to NOT be satisfied by version '{}', got {:?}", + constraint, + version, + result + ); + } + + fn assert_skipped(constraint: &str, version: &str) { + let result = check_dependency_constraint("test-dep", constraint, version).unwrap(); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "Expected constraint '{}' with version '{}' to be skipped, got {:?}", + constraint, + version, + result + ); + } + + #[test] + fn caret_constraint_allows_compatible_minor_bump() { + assert_satisfied("1.2.3", "1.3.0"); + assert_satisfied("^1.2.3", "1.3.0"); + } + + #[test] + fn caret_constraint_rejects_major_bump() { + assert_not_satisfied("1.2.3", "2.0.0"); + assert_not_satisfied("^1.2.3", "2.0.0"); + } + + #[test] + fn caret_constraint_zero_major_is_stricter() { + assert_satisfied("0.2.3", "0.2.5"); + assert_not_satisfied("0.2.3", "0.3.0"); + } + + #[test] + fn tilde_constraint_allows_patch_only() { + assert_satisfied("~1.2.3", "1.2.9"); + } + + #[test] + fn tilde_constraint_rejects_minor_bump() { + assert_not_satisfied("~1.2.3", "1.3.0"); + } + + #[test] + fn wildcard_constraint_allows_any_minor() { + assert_satisfied("1.*", "1.99.0"); + } + + #[test] + fn wildcard_constraint_rejects_major_bump() { + assert_not_satisfied("1.*", "2.0.0"); + } + + #[test] + fn global_wildcard_allows_anything() { + assert_satisfied("*", "99.0.0"); + } + + #[test] + fn exact_constraint_allows_exact_match() { + assert_satisfied("=1.2.3", "1.2.3"); + } + + #[test] + fn exact_constraint_rejects_any_difference() { + assert_not_satisfied("=1.2.3", "1.2.4"); + } + + #[test] + fn range_constraint_allows_within_range() { + assert_satisfied(">=1.2, <2.0", "1.5.0"); + } + + #[test] + fn range_constraint_rejects_outside_range() { + assert_not_satisfied(">=1.2, <2.0", "2.0.0"); + } + + #[test] + fn prerelease_constraint_matches_prerelease_version() { + assert_satisfied("1.0.0-alpha", "1.0.0-beta"); + } + + #[test] + fn stable_constraint_rejects_prerelease_version() { + assert_not_satisfied("1.0", "1.0.0-alpha"); + } + + #[test] + fn invalid_constraint_is_skipped() { + assert_skipped("invalid", "1.0.0"); + } + + #[test] + fn invalid_version_is_skipped() { + assert_skipped("1.0", "invalid"); + } + + #[test] + fn empty_constraint_is_skipped() { + assert_skipped("", "1.0.0"); + } + + #[test] + fn empty_version_is_skipped() { + assert_skipped("1.0", ""); + } + + #[test] + fn whitespace_in_constraint_is_trimmed() { + assert_satisfied(" 1.2.3 ", "1.3.0"); + } + + #[test] + fn whitespace_in_version_is_trimmed() { + assert_satisfied("1.2.3", " 1.3.0 "); + } + + #[test] + fn partial_major_constraint_works() { + assert_satisfied("1", "1.5.0"); + assert_not_satisfied("1", "2.0.0"); + } + + #[test] + fn partial_minor_constraint_works() { + assert_satisfied("1.2", "1.2.5"); + assert_satisfied("1.2", "1.3.0"); + assert_not_satisfied("1.2", "2.0.0"); + } +} diff --git a/crates/sampo-core/src/adapters/packagist.rs b/crates/sampo-core/src/adapters/packagist.rs index 289a367..f5fccd4 100644 --- a/crates/sampo-core/src/adapters/packagist.rs +++ b/crates/sampo-core/src/adapters/packagist.rs @@ -327,6 +327,17 @@ pub(super) fn publish_dry_run( Ok(()) } +pub(super) fn check_dependency_constraint( + _manifest_path: &Path, + _dep_name: &str, + _current_constraint: &str, + _new_version: &str, +) -> Result { + Ok(crate::types::ConstraintCheckResult::Skipped { + reason: "packagist constraint validation not yet implemented".to_string(), + }) +} + fn enforce_packagist_rate_limit() { let lock = PACKAGIST_LAST_CALL.get_or_init(|| Mutex::new(None)); let mut guard = match lock.lock() { diff --git a/crates/sampo-github-action/src/sampo.rs b/crates/sampo-github-action/src/sampo.rs index 208b4c9..c3d98eb 100644 --- a/crates/sampo-github-action/src/sampo.rs +++ b/crates/sampo-github-action/src/sampo.rs @@ -408,14 +408,15 @@ mod tests { let a_dir = root.join("crates/a"); let b_dir = root.join("crates/b"); - fs::create_dir_all(&a_dir).unwrap(); - fs::create_dir_all(&b_dir).unwrap(); + fs::create_dir_all(a_dir.join("src")).unwrap(); + fs::create_dir_all(b_dir.join("src")).unwrap(); fs::write( b_dir.join("Cargo.toml"), "[package]\nname=\"b\"\nversion=\"0.1.0\"\n", ) .unwrap(); + fs::write(b_dir.join("src/lib.rs"), "").unwrap(); // a depends on b fs::write( @@ -423,6 +424,7 @@ mod tests { "[package]\nname=\"a\"\nversion=\"0.1.0\"\n\n[dependencies]\nb = { path=\"../b\", version=\"0.1.0\" }\n", ) .unwrap(); + fs::write(a_dir.join("src/lib.rs"), "").unwrap(); // Create a changeset that only affects b let csdir = root.join(".sampo/changesets"); @@ -462,14 +464,15 @@ mod tests { let a_dir = root.join("crates/a"); let b_dir = root.join("crates/b"); - fs::create_dir_all(&a_dir).unwrap(); - fs::create_dir_all(&b_dir).unwrap(); + fs::create_dir_all(a_dir.join("src")).unwrap(); + fs::create_dir_all(b_dir.join("src")).unwrap(); fs::write( b_dir.join("Cargo.toml"), "[package]\nname=\"b\"\nversion=\"1.0.0\"\n", ) .unwrap(); + fs::write(b_dir.join("src/lib.rs"), "").unwrap(); // a depends on b fs::write( @@ -477,6 +480,7 @@ mod tests { "[package]\nname=\"a\"\nversion=\"1.0.0\"\n\n[dependencies]\nb = { path=\"../b\", version=\"1.0.0\" }\n", ) .unwrap(); + fs::write(a_dir.join("src/lib.rs"), "").unwrap(); // Create sampo config with fixed dependencies let sampo_dir = root.join(".sampo"); @@ -526,14 +530,15 @@ mod tests { let a_dir = root.join("crates/a"); let b_dir = root.join("crates/b"); - fs::create_dir_all(&a_dir).unwrap(); - fs::create_dir_all(&b_dir).unwrap(); + fs::create_dir_all(a_dir.join("src")).unwrap(); + fs::create_dir_all(b_dir.join("src")).unwrap(); fs::write( b_dir.join("Cargo.toml"), "[package]\nname=\"b\"\nversion=\"1.0.0\"\n", ) .unwrap(); + fs::write(b_dir.join("src/lib.rs"), "").unwrap(); // a does NOT depend on b (important difference) fs::write( @@ -541,6 +546,7 @@ mod tests { "[package]\nname=\"a\"\nversion=\"1.0.0\"\n", ) .unwrap(); + fs::write(a_dir.join("src/lib.rs"), "").unwrap(); // Create sampo config with fixed dependencies let sampo_dir = root.join(".sampo"); @@ -594,12 +600,13 @@ mod tests { // Single crate 'example' v0.1.0 let ex_dir = root.join("crates/example"); - fs::create_dir_all(&ex_dir).unwrap(); + fs::create_dir_all(ex_dir.join("src")).unwrap(); fs::write( ex_dir.join("Cargo.toml"), "[package]\nname=\"example\"\nversion=\"0.1.0\"\n", ) .unwrap(); + fs::write(ex_dir.join("src/lib.rs"), "").unwrap(); // Changeset: example minor change let cs_dir = root.join(".sampo/changesets"); diff --git a/crates/sampo/README.md b/crates/sampo/README.md index 6e9d178..f796aa9 100644 --- a/crates/sampo/README.md +++ b/crates/sampo/README.md @@ -211,6 +211,9 @@ Sampo detects packages within the same repository that depend on each other and > [!WARNING] > Packages cannot appear in both `fixed` and `linked` configurations. +> [!NOTE] +> Sampo supports range constraints (e.g., `^1.0`, `~1.2`) for internal dependencies. In this case, when a package is released, Sampo validates that the updated version still satisfies the specified range constraints. If not, you'll get an error for packages in `fixed` or `linked` groups, or a warning otherwise. + ## Commands All commands should be run from the root of the repository: From 759cc65ad7e86f00c892b4ee1711340c80bcfc8f Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Sun, 15 Feb 2026 18:33:32 +0100 Subject: [PATCH 04/10] implement range constraint preservation --- .../steadfast-princess-vainamoinen.md | 4 +- crates/sampo-core/src/adapters/cargo.rs | 105 +++++++-- .../src/adapters/cargo/cargo_tests.rs | 213 ++++++++++++++++++ crates/sampo-core/src/release.rs | 35 ++- crates/sampo-core/src/release_tests.rs | 102 +++++++++ 5 files changed, 437 insertions(+), 22 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index 74f8fd0..ddc7c48 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -4,6 +4,6 @@ cargo/sampo-core: minor cargo/sampo-github-action: minor --- -**⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump don't satisfies a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. +**⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump doesn't satisfy a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. -**Note:** Constraint validation is fully implemented, but not wired to release planning. Other ecosystems (npm, Hex, PyPI) will skip validation with an informative message until constraint extraction is implemented for them. +**Note:** Constraint validation is currently implemented for Cargo packages only. Other ecosystems will skip validation with an informative message until constraint extraction is implemented for them. diff --git a/crates/sampo-core/src/adapters/cargo.rs b/crates/sampo-core/src/adapters/cargo.rs index 06e7921..516cf0f 100644 --- a/crates/sampo-core/src/adapters/cargo.rs +++ b/crates/sampo-core/src/adapters/cargo.rs @@ -472,6 +472,51 @@ impl ManifestMetadata { .find(|dep| dep.package_name == dep_name) .map(|dep| dep.version_req.clone()) } + + /// Check if a dependency's version in the raw TOML is a pinned exact version. + /// Pinned versions (bare semver like `"1.0.0"`) will be updated during manifest updates, + /// while range constraints (e.g., `"^1.0"`) are preserved and need constraint validation. + pub fn is_dependency_pinned(&self, manifest_path: &Path, dep_name: &str) -> bool { + let manifest_key = self + .package_for_manifest(manifest_path) + .and_then(|pkg| { + pkg.dependencies + .iter() + .find(|d| d.package_name == dep_name) + .map(|d| d.manifest_key.as_str()) + }) + .unwrap_or(dep_name); + + let content = match fs::read_to_string(manifest_path) { + Ok(c) => c, + Err(_) => return false, + }; + let doc: DocumentMut = match content.parse() { + Ok(d) => d, + Err(_) => return false, + }; + + let top = doc.as_table(); + for section in ["dependencies", "dev-dependencies", "build-dependencies"] { + if let Some(raw) = raw_dep_version(top, section, manifest_key) { + return Version::parse(raw.trim()).is_ok(); + } + } + + if let Some(targets) = top.get("target").and_then(Item::as_table) { + for (_, target_item) in targets.iter() { + if let Some(target_table) = target_item.as_table() { + for section in ["dependencies", "dev-dependencies", "build-dependencies"] { + if let Some(raw) = raw_dep_version(target_table, section, manifest_key) { + return Version::parse(raw.trim()).is_ok(); + } + } + } + } + } + + false + } } /// Update a Cargo manifest by setting the package version (if provided) and retargeting internal @@ -597,6 +642,21 @@ fn dependency_table_mut<'a>( } } +/// Extract the raw version string for a dependency from a TOML table section. +fn raw_dep_version<'a>(parent: &'a Table, section: &str, dep_name: &str) -> Option<&'a str> { + let dep_table = parent.get(section)?.as_table()?; + let item = dep_table.get(dep_name)?; + match item { + Item::Value(Value::String(s)) => Some(s.value()), + Item::Value(Value::InlineTable(t)) => t.get("version").and_then(Value::as_str), + Item::Table(t) => t + .get("version") + .and_then(Item::as_value) + .and_then(Value::as_str), + _ => None, + } +} + fn dependency_section_name(kind: DependencyKind) -> &'static str { match kind { DependencyKind::Normal | DependencyKind::Unknown => "dependencies", @@ -605,17 +665,30 @@ fn dependency_section_name(kind: DependencyKind) -> &'static str { } } +/// Returns true if the existing dependency version should be replaced with the exact new version. +/// Pinned versions (bare semver) are always replaced; range constraints are preserved. +fn should_update_dependency_version(existing: &str, new_version: &str) -> bool { + let trimmed = existing.trim(); + + if Version::parse(trimmed).is_ok() { + return trimmed != new_version; + } + + // Deferred to constraint validation in release.rs + false +} + fn update_standard_dependency_item(item: &mut Item, new_version: &str) -> bool { match item { Item::Value(Value::InlineTable(table)) => update_inline_dependency(table, new_version), Item::Table(table) => update_table_dependency(table, new_version), Item::Value(value) => { - if value.as_str() == Some(new_version) { - false - } else { - *item = Item::Value(Value::from(new_version)); - true + let current = value.as_str().unwrap_or_default(); + if !should_update_dependency_version(current, new_version) { + return false; } + *item = Item::Value(Value::from(new_version)); + true } _ => false, } @@ -630,11 +703,12 @@ fn update_inline_dependency(table: &mut InlineTable, new_version: &str) -> bool return false; } - let needs_update = table - .get("version") - .and_then(Value::as_str) - .map(|current| current != new_version) - .unwrap_or(true); + let current = table.get("version").and_then(Value::as_str); + let needs_update = match current { + Some(existing) => should_update_dependency_version(existing, new_version), + // No version field present — don't insert one + None => false, + }; if needs_update { table.insert("version", Value::from(new_version)); @@ -653,12 +727,15 @@ fn update_table_dependency(table: &mut Table, new_version: &str) -> bool { return false; } - let needs_update = table + let current = table .get("version") .and_then(Item::as_value) - .and_then(Value::as_str) - .map(|current| current != new_version) - .unwrap_or(true); + .and_then(Value::as_str); + let needs_update = match current { + Some(existing) => should_update_dependency_version(existing, new_version), + // No version field present — don't insert one + None => false, + }; if needs_update { table.insert("version", Item::Value(Value::from(new_version))); diff --git a/crates/sampo-core/src/adapters/cargo/cargo_tests.rs b/crates/sampo-core/src/adapters/cargo/cargo_tests.rs index 25cdf6d..9a0b389 100644 --- a/crates/sampo-core/src/adapters/cargo/cargo_tests.rs +++ b/crates/sampo-core/src/adapters/cargo/cargo_tests.rs @@ -469,3 +469,216 @@ mod constraint_validation { assert_not_satisfied("1.2", "2.0.0"); } } + +mod range_constraint_preservation { + use super::*; + use std::collections::BTreeMap; + use std::path::Path; + + #[test] + fn caret_range_preserved_when_satisfied_inline() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"^1.0\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "range constraint should not trigger an update" + ); + assert!(out.contains("\"^1.0\""), "caret range should be preserved"); + } + + #[test] + fn caret_range_preserved_when_not_satisfied_inline() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"^1.0\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "2.0.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "unsatisfied range should still be preserved" + ); + assert!( + out.contains("\"^1.0\""), + "caret range should be preserved even when not satisfied" + ); + } + + #[test] + fn pinned_version_updated_inline() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"1.0.0\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!(applied.contains(&("bar".to_string(), "1.3.0".to_string()))); + assert!( + out.contains("\"1.3.0\""), + "pinned version should be updated" + ); + } + + #[test] + fn simple_string_range_preserved() { + let input = + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n[dependencies]\nbar = \"^1.0\"\n"; + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "simple string range should not trigger update" + ); + assert!( + out.contains("\"^1.0\""), + "simple string caret range should be preserved" + ); + } + + #[test] + fn simple_string_pinned_updated() { + let input = + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n[dependencies]\nbar = \"1.0.0\"\n"; + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!(applied.contains(&("bar".to_string(), "1.3.0".to_string()))); + assert!( + out.contains("\"1.3.0\""), + "simple string pinned version should be updated" + ); + } + + #[test] + fn tilde_range_preserved() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"~1.2.0\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.2.5".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!(applied.is_empty(), "tilde range should not trigger update"); + assert!( + out.contains("\"~1.2.0\""), + "tilde range should be preserved" + ); + } + + #[test] + fn wildcard_range_preserved() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"1.*\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.5.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "wildcard range should not trigger update" + ); + assert!( + out.contains("\"1.*\""), + "wildcard range should be preserved" + ); + } + + #[test] + fn table_dependency_range_preserved() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies.bar]\nversion = \"^1.0\"\npath = \"../bar\"\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "table dep range should not trigger update" + ); + assert!( + out.contains("\"^1.0\""), + "table dep range should be preserved" + ); + } + + #[test] + fn table_dependency_pinned_updated() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies.bar]\nversion = \"1.0.0\"\npath = \"../bar\"\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.3.0".to_string()); + + let (out, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!(applied.contains(&("bar".to_string(), "1.3.0".to_string()))); + assert!( + out.contains("\"1.3.0\""), + "table dep pinned version should be updated" + ); + } + + #[test] + fn pinned_version_same_version_no_change() { + let input = concat!( + "[package]\nname=\"demo\"\nversion=\"0.1.0\"\n\n", + "[dependencies]\nbar = { version = \"1.0.0\", path = \"../bar\" }\n", + ); + let mut updates = BTreeMap::new(); + updates.insert("bar".to_string(), "1.0.0".to_string()); + + let (_, applied) = + update_manifest_versions(Path::new("/demo/Cargo.toml"), input, None, &updates, None) + .unwrap(); + + assert!( + applied.is_empty(), + "same pinned version should not trigger update" + ); + } +} diff --git a/crates/sampo-core/src/release.rs b/crates/sampo-core/src/release.rs index c8075b3..564405d 100644 --- a/crates/sampo-core/src/release.rs +++ b/crates/sampo-core/src/release.rs @@ -711,7 +711,6 @@ fn compute_plan_state( /// Validates dependency constraints before applying releases. /// Returns error for fixed/linked packages with violations, warnings otherwise. -#[allow(dead_code)] // Infrastructure for future use fn validate_dependency_constraints( releases: &ReleasePlan, workspace: &Workspace, @@ -733,6 +732,16 @@ fn validate_dependency_constraints( let linked_groups = resolve_config_groups(workspace, &config.linked_dependencies, "packages.linked")?; + // Load cargo metadata once for constraint extraction on Cargo packages. + let has_cargo = releases + .iter() + .any(|(id, _, _)| by_id.get(id).is_some_and(|p| p.kind == PackageKind::Cargo)); + let cargo_metadata = if has_cargo { + ManifestMetadata::load(workspace).ok() + } else { + None + }; + let mut violations: Vec = Vec::new(); let mut warnings: Vec = Vec::new(); @@ -753,15 +762,29 @@ fn validate_dependency_constraints( continue; }; - // Placeholder until per-adapter constraint extraction is implemented. - // Note: adapters receive the package name and must handle alias resolution - // themselves when parsing the manifest (e.g., `alias = { package = "name" }`). - let constraint_placeholder = "*"; + // Pinned versions (bare semver) will be updated during manifest updates, + // so only range constraints need validation here. + // Other ecosystems skip validation (their adapters return Skipped). + let constraint = match pkg_info.kind { + PackageKind::Cargo => { + if let Some(ref meta) = cargo_metadata { + // Skip pinned deps — they will be rewritten to the exact new version + if meta.is_dependency_pinned(&manifest_path, &dep_info.name) { + continue; + } + meta.get_dependency_constraint(&manifest_path, &dep_info.name) + .unwrap_or_else(|| "*".to_string()) + } else { + "*".to_string() + } + } + _ => "*".to_string(), + }; let check_result = adapter.check_dependency_constraint( &manifest_path, &dep_info.name, - constraint_placeholder, + &constraint, new_dep_version, )?; diff --git a/crates/sampo-core/src/release_tests.rs b/crates/sampo-core/src/release_tests.rs index d3cb5c3..907fdb0 100644 --- a/crates/sampo-core/src/release_tests.rs +++ b/crates/sampo-core/src/release_tests.rs @@ -123,6 +123,31 @@ mod tests { self } + /// Like `add_dependency`, but uses the raw constraint string as-is. + /// Useful for testing range constraints like `"^0.1"` or `"~1.0"`. + fn add_dependency_with_constraint( + &mut self, + from: &str, + to: &str, + constraint: &str, + ) -> &mut Self { + let from_dir = self.crates.get(from).expect("from crate must exist"); + let current_manifest = fs::read_to_string(from_dir.join("Cargo.toml")).unwrap(); + + let dependency_section = format!( + "\n[dependencies]\n{} = {{ path=\"../{}\", version=\"{}\" }}\n", + to, to, constraint + ); + + fs::write( + from_dir.join("Cargo.toml"), + current_manifest + &dependency_section, + ) + .unwrap(); + + self + } + fn write_changeset_to_dir( dir: &std::path::Path, packages: &[&str], @@ -1695,4 +1720,81 @@ tempfile = "3.0" "prerelease dir should keep the rewritten mixed changeset" ); } + + #[test] + fn range_constraint_preserved_through_release() { + let mut workspace = TestWorkspace::new(); + workspace + .add_crate("a", "0.1.0") + .add_crate("b", "0.1.0") + .add_dependency_with_constraint("a", "b", "^0.1") + .add_changeset(&["b"], Bump::Patch, "fix: b patch fix"); + + workspace.run_release(false).unwrap(); + + // b gets a patch bump -> 0.1.1 + workspace.assert_crate_version("b", "0.1.1"); + + // a's dependency constraint "^0.1" should be preserved (0.1.1 satisfies ^0.1) + workspace.assert_dependency_version("a", "b", "^0.1"); + } + + #[test] + fn pinned_version_updated_through_release() { + let mut workspace = TestWorkspace::new(); + workspace + .add_crate("a", "0.1.0") + .add_crate("b", "0.1.0") + .add_dependency("a", "b", "0.1.0") + .add_changeset(&["b"], Bump::Minor, "feat: b minor"); + + workspace.run_release(false).unwrap(); + + // b gets a minor bump -> 0.2.0 + workspace.assert_crate_version("b", "0.2.0"); + + // a's pinned version "0.1.0" should be updated to "0.2.0" + workspace.assert_dependency_version("a", "b", "0.2.0"); + } + + #[test] + fn constraint_violation_in_fixed_group_blocks_release() { + let mut workspace = TestWorkspace::new(); + workspace + .add_crate("a", "1.0.0") + .add_crate("b", "1.0.0") + .add_dependency_with_constraint("a", "b", "~1.0.0") // tilde: allows 1.0.x only + .set_config("[packages]\nfixed = [[\"cargo/a\", \"cargo/b\"]]\n") + .add_changeset(&["cargo/b"], Bump::Minor, "bump b minor"); // b -> 1.1.0, violates ~1.0.0 + + let result = workspace.run_release(true); + assert!( + result.is_err(), + "Expected release to fail due to constraint violation in fixed group" + ); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("constraint") || err_msg.contains("Constraint"), + "Error should mention constraint violation, got: {}", + err_msg + ); + } + + #[test] + fn constraint_violation_without_group_allows_release() { + let mut workspace = TestWorkspace::new(); + workspace + .add_crate("a", "1.0.0") + .add_crate("b", "1.0.0") + .add_dependency_with_constraint("a", "b", "~1.0.0") // tilde: allows 1.0.x only + .add_changeset(&["cargo/b"], Bump::Minor, "bump b minor"); // b -> 1.1.0, violates ~1.0.0 + // No fixed/linked config — violation is only a warning, release proceeds + + let result = workspace.run_release(true); + assert!( + result.is_ok(), + "Expected release to succeed with warning, got: {:?}", + result.err() + ); + } } From 462748674b43f821feb56d55f327928f1ed638ff Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 07:50:37 +0100 Subject: [PATCH 05/10] range constraints for npm --- .../steadfast-princess-vainamoinen.md | 2 +- crates/sampo-core/src/adapters/npm.rs | 428 +++++++++++++++++- .../sampo-core/src/adapters/npm/npm_tests.rs | 282 ++++++++++++ 3 files changed, 704 insertions(+), 8 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index ddc7c48..1519e33 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -6,4 +6,4 @@ cargo/sampo-github-action: minor **⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump doesn't satisfy a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. -**Note:** Constraint validation is currently implemented for Cargo packages only. Other ecosystems will skip validation with an informative message until constraint extraction is implemented for them. +**Note:** Constraint validation is currently implemented for Cargo and npm packages. Other ecosystems (Hex, PyPI, Packagist) will skip validation with an informative message until support is added. diff --git a/crates/sampo-core/src/adapters/npm.rs b/crates/sampo-core/src/adapters/npm.rs index 0ee430f..64cff9e 100644 --- a/crates/sampo-core/src/adapters/npm.rs +++ b/crates/sampo-core/src/adapters/npm.rs @@ -178,16 +178,430 @@ impl NpmAdapter { } } -/// Stub: returns `Skipped` until npm semver constraint parsing is implemented. +/// Check if a new version satisfies the npm dependency constraint found in `package.json`. +/// +/// Reads the real constraint from the manifest rather than relying on the passed-in +/// `_current_constraint` (which may be a placeholder like `"*"`). pub(super) fn check_dependency_constraint( - _manifest_path: &Path, - _dep_name: &str, + manifest_path: &Path, + dep_name: &str, _current_constraint: &str, - _new_version: &str, + new_version: &str, ) -> Result { - Ok(crate::types::ConstraintCheckResult::Skipped { - reason: "npm constraint validation not yet implemented".to_string(), - }) + use crate::types::ConstraintCheckResult; + + let manifest = load_package_json(manifest_path)?; + + let constraint = match find_dependency_constraint(&manifest, dep_name) { + Some(c) => c, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("dependency '{}' not found in manifest", dep_name), + }); + } + }; + + let trimmed = constraint.trim(); + + if trimmed.is_empty() { + return Ok(ConstraintCheckResult::Skipped { + reason: "empty constraint".to_string(), + }); + } + + if trimmed == "*" { + return Ok(ConstraintCheckResult::Satisfied); + } + + if trimmed.starts_with("workspace:") { + return Ok(ConstraintCheckResult::Skipped { + reason: "workspace protocol".to_string(), + }); + } + + for prefix in ["file:", "link:", "npm:", "git:", "http:", "https:"] { + if trimmed.starts_with(prefix) { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("'{}' protocol", prefix.trim_end_matches(':')), + }); + } + } + + if new_version.contains('-') { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release version".to_string(), + }); + } + + if constraint_contains_prerelease(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release constraint".to_string(), + }); + } + + if is_pinned_version(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pinned version".to_string(), + }); + } + + match npm_version_satisfies(trimmed, new_version) { + Some(true) => Ok(ConstraintCheckResult::Satisfied), + Some(false) => Ok(ConstraintCheckResult::NotSatisfied { + constraint: trimmed.to_string(), + new_version: new_version.to_string(), + }), + None => Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable constraint '{}'", trimmed), + }), + } +} + +/// Look up the constraint string for `dep_name` across all npm dependency sections. +fn find_dependency_constraint(manifest: &JsonValue, dep_name: &str) -> Option { + for key in [ + "dependencies", + "devDependencies", + "peerDependencies", + "optionalDependencies", + ] { + if let Some(deps) = manifest.get(key).and_then(JsonValue::as_object) + && let Some(value) = deps.get(dep_name).and_then(JsonValue::as_str) + { + return Some(value.to_string()); + } + } + None +} + +/// Heuristic: a constraint contains a pre-release tag when a digit is followed by `-` +/// then an alphanumeric character (e.g. `1.0.0-beta`), excluding hyphen-range separators +/// which always have surrounding spaces. +fn constraint_contains_prerelease(constraint: &str) -> bool { + let bytes = constraint.as_bytes(); + for i in 1..bytes.len().saturating_sub(1) { + if bytes[i] == b'-' && bytes[i - 1].is_ascii_digit() && bytes[i + 1].is_ascii_alphanumeric() + { + return true; + } + } + false +} + +/// A pinned version is a bare `M.m.p` string with no operator or wildcard. +fn is_pinned_version(s: &str) -> bool { + parse_version(s).is_some() +} + +/// Check if an npm semver version satisfies a constraint range string. +/// +/// Returns `None` when the constraint or version cannot be parsed. +fn npm_version_satisfies(constraint: &str, version_str: &str) -> Option { + let version = parse_version(version_str)?; + + for or_part in constraint.split("||") { + let trimmed = or_part.trim(); + if trimmed.is_empty() || trimmed == "*" { + return Some(true); + } + match satisfies_comparator_set(trimmed, version) { + Some(true) => return Some(true), + Some(false) => continue, + None => return None, + } + } + + Some(false) +} + +/// Collapse whitespace between an operator (`>=`, `<=`, `>`, `<`, `^`, `~`, `=`) +/// and the version number so that `>= 1.2.3` becomes `>=1.2.3`. +fn normalize_comparator_whitespace(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + let bytes = s.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let ch = bytes[i] as char; + result.push(ch); + if matches!(ch, '>' | '<' | '~' | '^' | '=') { + // Handle two-character operators (>=, <=) + if i + 1 < bytes.len() && bytes[i + 1] == b'=' { + i += 1; + result.push('='); + } + // Skip whitespace between operator and version + while i + 1 < bytes.len() && bytes[i + 1] == b' ' { + i += 1; + } + } + i += 1; + } + result +} + +/// Check if `version` satisfies every comparator in a space-separated set (AND semantics). +fn satisfies_comparator_set(set: &str, version: (u64, u64, u64)) -> Option { + if let Some(result) = try_hyphen_range(set, version) { + return Some(result); + } + + let normalized = normalize_comparator_whitespace(set); + for comp in normalized.split_whitespace() { + if !satisfies_comparator(comp, version)? { + return Some(false); + } + } + + Some(true) +} + +/// Try to parse `set` as a hyphen range (`A - B`). Returns `None` if the pattern +/// does not match so the caller can fall back to normal comparator parsing. +fn try_hyphen_range(set: &str, version: (u64, u64, u64)) -> Option { + let tokens: Vec<&str> = set.split_whitespace().collect(); + if tokens.len() != 3 || tokens[1] != "-" { + return None; + } + let lower = parse_partial_version(tokens[0])?; + let upper = parse_partial_version(tokens[2])?; + + let lower_full = (lower.0, lower.1.unwrap_or(0), lower.2.unwrap_or(0)); + if version < lower_full { + return Some(false); + } + + let upper_ok = match (upper.1, upper.2) { + (Some(m), Some(p)) => version <= (upper.0, m, p), + (Some(m), None) => version < (upper.0, m + 1, 0), + (None, _) => version < (upper.0 + 1, 0, 0), + }; + + Some(upper_ok) +} + +/// Evaluate a single comparator (`^`, `~`, `>=`, `>`, `<=`, `<`, `=`, or bare/x-range). +fn satisfies_comparator(comp: &str, version: (u64, u64, u64)) -> Option { + let comp = comp.trim(); + if comp.is_empty() || matches!(comp, "*" | "x" | "X") { + return Some(true); + } + + if let Some(rest) = comp.strip_prefix('^') { + let rest = rest.trim(); + if rest.is_empty() || matches!(rest, "*" | "x" | "X") { + return Some(true); + } + let partial = parse_partial_version(rest)?; + let (lower, upper) = expand_caret(partial.0, partial.1, partial.2); + return Some(version >= lower && version < upper); + } + + if let Some(rest) = comp.strip_prefix('~') { + let rest = rest.trim(); + if rest.is_empty() || matches!(rest, "*" | "x" | "X") { + return Some(true); + } + let partial = parse_partial_version(rest)?; + let (lower, upper) = expand_tilde(partial.0, partial.1, partial.2); + return Some(version >= lower && version < upper); + } + + // >= (checked before >) + if let Some(rest) = comp.strip_prefix(">=") { + let partial = parse_partial_version(rest.trim())?; + let target = (partial.0, partial.1.unwrap_or(0), partial.2.unwrap_or(0)); + return Some(version >= target); + } + + // > + if let Some(rest) = comp.strip_prefix('>') { + let partial = parse_partial_version(rest.trim())?; + return Some(match (partial.1, partial.2) { + (Some(m), Some(p)) => version > (partial.0, m, p), + (Some(m), None) => version >= (partial.0, m + 1, 0), + (None, _) => version >= (partial.0 + 1, 0, 0), + }); + } + + // <= (checked before <) + if let Some(rest) = comp.strip_prefix("<=") { + let partial = parse_partial_version(rest.trim())?; + return Some(match (partial.1, partial.2) { + (Some(m), Some(p)) => version <= (partial.0, m, p), + (Some(m), None) => version < (partial.0, m + 1, 0), + (None, _) => version < (partial.0 + 1, 0, 0), + }); + } + + // < + if let Some(rest) = comp.strip_prefix('<') { + let partial = parse_partial_version(rest.trim())?; + let target = (partial.0, partial.1.unwrap_or(0), partial.2.unwrap_or(0)); + return Some(version < target); + } + + // Explicit = + if let Some(rest) = comp.strip_prefix('=') { + let partial = parse_partial_version(rest.trim())?; + return Some(matches_partial(version, partial.0, partial.1, partial.2)); + } + + // Bare version or x-range + let partial = parse_partial_version(comp)?; + Some(matches_partial(version, partial.0, partial.1, partial.2)) +} + +/// Check if `version` falls within the range described by a partial version. +/// +/// A complete `(M, Some(m), Some(p))` requires exact equality. +/// A partial version acts as an x-range (e.g. `1.2` → `>=1.2.0 <1.3.0`). +fn matches_partial( + version: (u64, u64, u64), + major: u64, + minor: Option, + patch: Option, +) -> bool { + match (minor, patch) { + (Some(m), Some(p)) => version == (major, m, p), + (Some(m), None) => version >= (major, m, 0) && version < (major, m + 1, 0), + (None, _) => version >= (major, 0, 0) && version < (major + 1, 0, 0), + } +} + +/// Expand a caret range (`^M.m.p`) to inclusive lower and exclusive upper bounds. +/// +/// The caret allows changes that do not modify the left-most non-zero digit: +/// - `^1.2.3` → `[1.2.3, 2.0.0)` +/// - `^0.2.3` → `[0.2.3, 0.3.0)` +/// - `^0.0.3` → `[0.0.3, 0.0.4)` +fn expand_caret( + major: u64, + minor: Option, + patch: Option, +) -> ((u64, u64, u64), (u64, u64, u64)) { + let m = minor.unwrap_or(0); + let p = patch.unwrap_or(0); + let lower = (major, m, p); + + let upper = if major > 0 { + (major + 1, 0, 0) + } else if minor.is_none() { + // ^0 → [0.0.0, 1.0.0) + (1, 0, 0) + } else if m > 0 { + (0, m + 1, 0) + } else if patch.is_none() { + // ^0.0 or ^0.0.x → [0.0.0, 0.1.0) + (0, 1, 0) + } else { + // ^0.0.3 → [0.0.3, 0.0.4) + (0, 0, p + 1) + }; + + (lower, upper) +} + +/// Expand a tilde range (`~M.m.p`) to inclusive lower and exclusive upper bounds. +/// +/// The tilde pins to the minor version when one is specified, otherwise pins to major: +/// - `~1.2.3` → `[1.2.3, 1.3.0)` +/// - `~1.2` → `[1.2.0, 1.3.0)` +/// - `~1` → `[1.0.0, 2.0.0)` +fn expand_tilde( + major: u64, + minor: Option, + patch: Option, +) -> ((u64, u64, u64), (u64, u64, u64)) { + let m = minor.unwrap_or(0); + let p = patch.unwrap_or(0); + let lower = (major, m, p); + + let upper = if minor.is_some() { + (major, m + 1, 0) + } else { + (major + 1, 0, 0) + }; + + (lower, upper) +} + +/// Parse a complete semver version string (`M.m.p`) into its three numeric components. +/// +/// Rejects pre-release tags (contains `-`) and strips build metadata (`+…`). +/// Returns `None` for anything that is not exactly three numeric dot-separated parts. +fn parse_version(s: &str) -> Option<(u64, u64, u64)> { + let s = s.trim(); + let s = s.strip_prefix('v').unwrap_or(s); + + if s.contains('-') { + return None; + } + + let s = s.split('+').next()?; + let parts: Vec<&str> = s.split('.').collect(); + if parts.len() != 3 { + return None; + } + + let major = parts[0].parse::().ok()?; + let minor = parts[1].parse::().ok()?; + let patch = parts[2].parse::().ok()?; + + Some((major, minor, patch)) +} + +/// Parse a version that may have missing or wildcard parts. +/// +/// Returns `(major, Option, Option)` where `None` indicates a +/// wildcard (`*`, `x`, `X`) or an absent component. +fn parse_partial_version(s: &str) -> Option<(u64, Option, Option)> { + let s = s.trim(); + let s = s.strip_prefix('v').unwrap_or(s); + + if s.contains('-') { + return None; + } + + let s = s.split('+').next()?; + if s.is_empty() { + return None; + } + + let parts: Vec<&str> = s.split('.').collect(); + match parts.len() { + 1 => { + if is_wildcard(parts[0]) { + return Some((0, None, None)); + } + let major = parts[0].parse::().ok()?; + Some((major, None, None)) + } + 2 => { + let major = parts[0].parse::().ok()?; + if is_wildcard(parts[1]) { + return Some((major, None, None)); + } + let minor = parts[1].parse::().ok()?; + Some((major, Some(minor), None)) + } + 3 => { + let major = parts[0].parse::().ok()?; + if is_wildcard(parts[1]) { + return Some((major, None, None)); + } + let minor = parts[1].parse::().ok()?; + if is_wildcard(parts[2]) { + return Some((major, Some(minor), None)); + } + let patch = parts[2].parse::().ok()?; + Some((major, Some(minor), Some(patch))) + } + _ => None, + } +} + +fn is_wildcard(s: &str) -> bool { + matches!(s, "*" | "x" | "X") } pub(super) fn publish_dry_run( diff --git a/crates/sampo-core/src/adapters/npm/npm_tests.rs b/crates/sampo-core/src/adapters/npm/npm_tests.rs index 95bad7b..a34634c 100644 --- a/crates/sampo-core/src/adapters/npm/npm_tests.rs +++ b/crates/sampo-core/src/adapters/npm/npm_tests.rs @@ -1,9 +1,291 @@ use super::*; +use crate::types::ConstraintCheckResult; use std::collections::BTreeMap; use std::fs; use std::path::Path; use tempfile::tempdir; +// Write a minimal package.json with one dependency and check the constraint. +fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { + let temp = tempdir().unwrap(); + let manifest_path = temp.path().join("package.json"); + let content = format!( + r#"{{"name":"test-pkg","version":"1.0.0","dependencies":{{"test-dep":"{}"}}}}"#, + constraint + ); + fs::write(&manifest_path, &content).unwrap(); + check_dependency_constraint(&manifest_path, "test-dep", "*", new_version).unwrap() +} + +fn assert_satisfied(constraint: &str, new_version: &str) { + assert_eq!( + assert_constraint(constraint, new_version), + ConstraintCheckResult::Satisfied, + "expected '{}' to be satisfied by '{}'", + constraint, + new_version + ); +} + +fn assert_not_satisfied(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "expected '{}' to be not satisfied by '{}', got {:?}", + constraint, + new_version, + result + ); +} + +fn assert_skipped(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected '{}' to be skipped for '{}', got {:?}", + constraint, + new_version, + result + ); +} + +#[test] +fn check_dependency_constraint_caret_satisfied() { + assert_satisfied("^1.2.3", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_caret_exact_match() { + assert_satisfied("^1.2.3", "1.2.3"); +} + +#[test] +fn check_dependency_constraint_caret_zero_minor_satisfied() { + assert_satisfied("^0.2.3", "0.2.5"); +} + +#[test] +fn check_dependency_constraint_tilde_satisfied() { + assert_satisfied("~1.2.3", "1.2.9"); +} + +#[test] +fn check_dependency_constraint_gte_satisfied() { + assert_satisfied(">=1.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_range_and_satisfied() { + assert_satisfied(">=1.0.0 <3.0.0", "2.5.0"); +} + +#[test] +fn check_dependency_constraint_or_satisfied() { + assert_satisfied("^1.0.0 || ^2.0.0", "2.1.0"); +} + +#[test] +fn check_dependency_constraint_star_satisfied() { + assert_satisfied("*", "5.0.0"); +} + +#[test] +fn check_dependency_constraint_x_range_major_satisfied() { + assert_satisfied("1.x", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_x_range_minor_satisfied() { + assert_satisfied("1.2.x", "1.2.9"); +} + +#[test] +fn check_dependency_constraint_hyphen_range_satisfied() { + assert_satisfied("1.2.3 - 2.3.4", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_gt_satisfied() { + assert_satisfied(">1.0.0", "1.0.1"); +} + +#[test] +fn check_dependency_constraint_lte_satisfied() { + assert_satisfied("<=2.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_lt_satisfied() { + assert_satisfied("<2.0.0", "1.9.9"); +} + +#[test] +fn check_dependency_constraint_caret_not_satisfied() { + assert_not_satisfied("^1.2.3", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_caret_zero_minor_not_satisfied() { + assert_not_satisfied("^0.2.3", "0.3.0"); +} + +#[test] +fn check_dependency_constraint_caret_zero_zero_not_satisfied() { + assert_not_satisfied("^0.0.3", "0.0.4"); +} + +#[test] +fn check_dependency_constraint_tilde_not_satisfied() { + assert_not_satisfied("~1.2.3", "1.3.0"); +} + +#[test] +fn check_dependency_constraint_gte_not_satisfied() { + assert_not_satisfied(">=2.0.0", "1.9.9"); +} + +#[test] +fn check_dependency_constraint_lt_not_satisfied() { + assert_not_satisfied("<2.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_range_and_not_satisfied() { + assert_not_satisfied(">=1.0.0 <2.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_or_not_satisfied() { + assert_not_satisfied("^1.0.0 || ^2.0.0", "3.0.0"); +} + +#[test] +fn check_dependency_constraint_x_range_major_not_satisfied() { + assert_not_satisfied("1.x", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_x_range_minor_not_satisfied() { + assert_not_satisfied("1.2.x", "1.3.0"); +} + +#[test] +fn check_dependency_constraint_hyphen_range_not_satisfied() { + assert_not_satisfied("1.2.3 - 2.3.4", "3.0.0"); +} + +#[test] +fn check_dependency_constraint_gte_with_space_satisfied() { + assert_satisfied(">= 1.0.0", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_gte_with_space_not_satisfied() { + assert_not_satisfied(">= 1.0.0", "0.9.0"); +} + +#[test] +fn check_dependency_constraint_caret_with_space_satisfied() { + assert_satisfied("^ 1.2.3", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_tilde_with_space_satisfied() { + assert_satisfied("~ 1.2.3", "1.2.9"); +} + +#[test] +fn check_dependency_constraint_range_with_spaces_satisfied() { + assert_satisfied(">= 1.0.0 < 2.0.0", "1.5.0"); +} + +#[test] +fn check_dependency_constraint_range_with_spaces_not_satisfied() { + assert_not_satisfied(">= 1.0.0 < 2.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_workspace_star_skipped() { + assert_skipped("workspace:*", "1.0.0"); +} + +#[test] +fn check_dependency_constraint_workspace_caret_skipped() { + assert_skipped("workspace:^1.0.0", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_file_protocol_skipped() { + assert_skipped("file:../foo", "1.0.0"); +} + +#[test] +fn check_dependency_constraint_link_protocol_skipped() { + assert_skipped("link:../foo", "1.0.0"); +} + +#[test] +fn check_dependency_constraint_pinned_version_skipped() { + assert_skipped("1.2.3", "2.0.0"); +} + +#[test] +fn check_dependency_constraint_prerelease_version_skipped() { + assert_skipped("^1.0.0", "2.0.0-beta.1"); +} + +#[test] +fn check_dependency_constraint_dep_not_found_skipped() { + let temp = tempdir().unwrap(); + let manifest_path = temp.path().join("package.json"); + fs::write( + &manifest_path, + r#"{"name":"test-pkg","version":"1.0.0","dependencies":{}}"#, + ) + .unwrap(); + let result = check_dependency_constraint(&manifest_path, "missing-dep", "*", "1.0.0").unwrap(); + assert!(matches!(result, ConstraintCheckResult::Skipped { .. })); +} + +#[test] +fn check_dependency_constraint_finds_in_dev_dependencies() { + let temp = tempdir().unwrap(); + let manifest_path = temp.path().join("package.json"); + fs::write( + &manifest_path, + r#"{"name":"test-pkg","version":"1.0.0","devDependencies":{"test-dep":"^1.0.0"}}"#, + ) + .unwrap(); + let result = check_dependency_constraint(&manifest_path, "test-dep", "*", "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); +} + +#[test] +fn check_dependency_constraint_finds_in_peer_dependencies() { + let temp = tempdir().unwrap(); + let manifest_path = temp.path().join("package.json"); + fs::write( + &manifest_path, + r#"{"name":"test-pkg","version":"1.0.0","peerDependencies":{"test-dep":"^2.0.0"}}"#, + ) + .unwrap(); + let result = check_dependency_constraint(&manifest_path, "test-dep", "*", "2.1.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); +} + +#[test] +fn check_dependency_constraint_finds_in_optional_dependencies() { + let temp = tempdir().unwrap(); + let manifest_path = temp.path().join("package.json"); + fs::write( + &manifest_path, + r#"{"name":"test-pkg","version":"1.0.0","optionalDependencies":{"test-dep":"~1.2.0"}}"#, + ) + .unwrap(); + let result = check_dependency_constraint(&manifest_path, "test-dep", "*", "1.2.5").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); +} + #[test] fn npm_adapter_discovers_single_package() { let temp = tempdir().unwrap(); From 9bd8ee9711a5468e01fec4c78ce8788d749efae1 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 09:33:20 +0100 Subject: [PATCH 06/10] implement range constraint for Hex --- .../steadfast-princess-vainamoinen.md | 2 +- crates/sampo-core/src/adapters/hex.rs | 213 ++++++++++++- .../sampo-core/src/adapters/hex/hex_tests.rs | 282 ++++++++++++++++++ crates/sampo-core/src/adapters/hex/mix.rs | 16 + 4 files changed, 505 insertions(+), 8 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index 1519e33..f07af92 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -6,4 +6,4 @@ cargo/sampo-github-action: minor **⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump doesn't satisfy a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. -**Note:** Constraint validation is currently implemented for Cargo and npm packages. Other ecosystems (Hex, PyPI, Packagist) will skip validation with an informative message until support is added. +**Note:** Constraint validation is currently implemented for Cargo, npm, and Hex packages. Other ecosystems (PyPI, Packagist) will skip validation with an informative message until support is added. diff --git a/crates/sampo-core/src/adapters/hex.rs b/crates/sampo-core/src/adapters/hex.rs index ee227d5..dbd7196 100644 --- a/crates/sampo-core/src/adapters/hex.rs +++ b/crates/sampo-core/src/adapters/hex.rs @@ -120,16 +120,215 @@ impl HexAdapter { } } -/// Stub: returns `Skipped` until Hex/Elixir constraint parsing is implemented. pub(super) fn check_dependency_constraint( - _manifest_path: &Path, - _dep_name: &str, + manifest_path: &Path, + dep_name: &str, _current_constraint: &str, - _new_version: &str, + new_version: &str, ) -> Result { - Ok(crate::types::ConstraintCheckResult::Skipped { - reason: "Hex constraint validation not yet implemented".to_string(), - }) + use crate::types::ConstraintCheckResult; + + let constraint = match mix::find_dependency_constraint_value(manifest_path, dep_name)? { + Some(c) => c, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("dependency '{}' not found in manifest", dep_name), + }); + } + }; + + let trimmed = constraint.trim(); + if trimmed.is_empty() { + return Ok(ConstraintCheckResult::Skipped { + reason: "empty constraint".to_string(), + }); + } + + if new_version.trim().contains('-') { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release version".to_string(), + }); + } + + // Skip pinned (bare) versions without any operator or conjunction + if !trimmed.starts_with("~>") + && !trimmed.starts_with("==") + && !trimmed.starts_with(">=") + && !trimmed.starts_with("<=") + && !trimmed.starts_with('>') + && !trimmed.starts_with('<') + && !trimmed.starts_with('=') + && !trimmed.contains(" and ") + && !trimmed.contains(" or ") + && !trimmed.contains(" AND ") + && !trimmed.contains(" OR ") + && parse_hex_version(trimmed).is_some() + { + return Ok(ConstraintCheckResult::Skipped { + reason: "pinned version".to_string(), + }); + } + + let version = match parse_hex_version(new_version.trim()) { + Some(v) => v, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable version '{}'", new_version), + }); + } + }; + + match hex_version_satisfies(trimmed, version) { + Some(true) => Ok(ConstraintCheckResult::Satisfied), + Some(false) => Ok(ConstraintCheckResult::NotSatisfied { + constraint: trimmed.to_string(), + new_version: new_version.trim().to_string(), + }), + None => Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable constraint '{}'", trimmed), + }), + } +} + +/// Ignores pre-release tags. +fn parse_hex_version(s: &str) -> Option<(u64, u64, u64)> { + let s = s.trim(); + if s.is_empty() { + return None; + } + let base = s.split('-').next()?; + let parts: Vec<&str> = base.split('.').collect(); + match parts.len() { + 3 => { + let major = parts[0].parse().ok()?; + let minor = parts[1].parse().ok()?; + let patch = parts[2].parse().ok()?; + Some((major, minor, patch)) + } + 2 => { + let major = parts[0].parse().ok()?; + let minor = parts[1].parse().ok()?; + Some((major, minor, 0)) + } + _ => None, + } +} + +/// Returns `None` if the constraint is unparseable. +fn hex_version_satisfies(constraint: &str, version: (u64, u64, u64)) -> Option { + let lowered = constraint.to_ascii_lowercase(); + + if lowered.contains(" or ") { + let parts = split_on_keyword(constraint, " or "); + for part in &parts { + match hex_version_satisfies(part.trim(), version) { + Some(true) => return Some(true), + Some(false) => continue, + None => return None, + } + } + return Some(false); + } + + if lowered.contains(" and ") { + let parts = split_on_keyword(constraint, " and "); + for part in &parts { + match hex_version_satisfies(part.trim(), version) { + Some(true) => continue, + Some(false) => return Some(false), + None => return None, + } + } + return Some(true); + } + + satisfies_hex_comparator(constraint.trim(), version) +} + +/// Split a string on a keyword, case-insensitively. +fn split_on_keyword(s: &str, keyword: &str) -> Vec { + let lowered = s.to_ascii_lowercase(); + let keyword_lower = keyword.to_ascii_lowercase(); + let mut result = Vec::new(); + let mut last = 0; + for (idx, _) in lowered.match_indices(&keyword_lower) { + result.push(s[last..idx].to_string()); + last = idx + keyword.len(); + } + result.push(s[last..].to_string()); + result +} + +fn satisfies_hex_comparator(comp: &str, version: (u64, u64, u64)) -> Option { + let comp = comp.trim(); + if comp.is_empty() { + return None; + } + + if let Some(rest) = comp.strip_prefix("~>") { + return satisfies_pessimistic(rest.trim(), version); + } + + if let Some(rest) = comp.strip_prefix("==") { + let target = parse_hex_version(rest.trim())?; + return Some(version == target); + } + + if let Some(rest) = comp.strip_prefix(">=") { + let target = parse_hex_version(rest.trim())?; + return Some(version >= target); + } + + if let Some(rest) = comp.strip_prefix("<=") { + let target = parse_hex_version(rest.trim())?; + return Some(version <= target); + } + + if let Some(rest) = comp.strip_prefix('>') { + let target = parse_hex_version(rest.trim())?; + return Some(version > target); + } + + if let Some(rest) = comp.strip_prefix('<') { + let target = parse_hex_version(rest.trim())?; + return Some(version < target); + } + + if let Some(rest) = comp.strip_prefix('=') { + let target = parse_hex_version(rest.trim())?; + return Some(version == target); + } + + // Bare version (exact match) + let target = parse_hex_version(comp)?; + Some(version == target) +} + +/// Evaluate the `~>` (pessimistic/compatibility) operator. +/// +/// - `~> X.Y` (2 parts): `>= X.Y.0 and < (X+1).0.0` +/// - `~> X.Y.Z` (3 parts): `>= X.Y.Z and < X.(Y+1).0` +fn satisfies_pessimistic(version_str: &str, version: (u64, u64, u64)) -> Option { + let s = version_str.trim(); + let parts: Vec<&str> = s.split('.').collect(); + match parts.len() { + 2 => { + let major: u64 = parts[0].parse().ok()?; + let minor: u64 = parts[1].parse().ok()?; + let lower = (major, minor, 0); + let upper = (major + 1, 0, 0); + Some(version >= lower && version < upper) + } + 3 => { + let major: u64 = parts[0].parse().ok()?; + let minor: u64 = parts[1].parse().ok()?; + let patch: u64 = parts[2].parse().ok()?; + let lower = (major, minor, patch); + let upper = (major, minor + 1, 0); + Some(version >= lower && version < upper) + } + _ => None, + } } pub(super) fn publish_dry_run( diff --git a/crates/sampo-core/src/adapters/hex/hex_tests.rs b/crates/sampo-core/src/adapters/hex/hex_tests.rs index 326c7c1..66df13b 100644 --- a/crates/sampo-core/src/adapters/hex/hex_tests.rs +++ b/crates/sampo-core/src/adapters/hex/hex_tests.rs @@ -6,3 +6,285 @@ fn version_exists_rejects_empty_name() { .expect_err("expected empty package name to fail"); assert!(format!("{}", err).contains("Package name cannot be empty")); } + +mod constraint_validation { + use super::*; + use crate::types::ConstraintCheckResult; + use std::fs; + + fn write_mix_with_dep(dir: &Path, dep_constraint: &str) { + let content = format!( + r#" +defmodule Test.MixProject do + use Mix.Project + + def project do + [ + app: :test_app, + version: "0.1.0", + deps: deps() + ] + end + + defp deps do + [ + {{:test_dep, {dep_constraint}}} + ] + end +end +"# + ); + fs::create_dir_all(dir).unwrap(); + fs::write(dir.join("mix.exs"), content).unwrap(); + } + + fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { + let temp = tempfile::tempdir().unwrap(); + write_mix_with_dep(temp.path(), &format!(r#""{}""#, constraint)); + let manifest = temp.path().join("mix.exs"); + check_dependency_constraint(&manifest, "test_dep", "*", new_version).unwrap() + } + + fn assert_satisfied(constraint: &str, version: &str) { + let result = assert_constraint(constraint, version); + assert!( + matches!(result, ConstraintCheckResult::Satisfied), + "expected '{}' to satisfy '{}', got {:?}", + version, + constraint, + result + ); + } + + fn assert_not_satisfied(constraint: &str, version: &str) { + let result = assert_constraint(constraint, version); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "expected '{}' to NOT satisfy '{}', got {:?}", + version, + constraint, + result + ); + } + + fn assert_skipped(constraint: &str, version: &str) { + let result = assert_constraint(constraint, version); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected '{}' to be skipped for '{}', got {:?}", + version, + constraint, + result + ); + } + + // ~> operator (pessimistic) + + #[test] + fn pessimistic_two_part_satisfied() { + assert_satisfied("~> 2.0", "2.5.0"); + } + + #[test] + fn pessimistic_two_part_exact_lower_bound() { + assert_satisfied("~> 2.0", "2.0.0"); + } + + #[test] + fn pessimistic_two_part_not_satisfied_major_bump() { + assert_not_satisfied("~> 2.0", "3.0.0"); + } + + #[test] + fn pessimistic_two_part_not_satisfied_below() { + assert_not_satisfied("~> 2.1", "2.0.0"); + } + + #[test] + fn pessimistic_three_part_satisfied() { + assert_satisfied("~> 2.1.0", "2.1.5"); + } + + #[test] + fn pessimistic_three_part_exact_lower_bound() { + assert_satisfied("~> 2.1.0", "2.1.0"); + } + + #[test] + fn pessimistic_three_part_not_satisfied_minor_bump() { + assert_not_satisfied("~> 2.1.0", "2.2.0"); + } + + #[test] + fn pessimistic_three_part_not_satisfied_below() { + assert_not_satisfied("~> 2.1.3", "2.1.2"); + } + + #[test] + fn pessimistic_zero_minor_satisfied() { + assert_satisfied("~> 0.1.0", "0.1.5"); + } + + #[test] + fn pessimistic_zero_minor_not_satisfied() { + assert_not_satisfied("~> 0.1.0", "0.2.0"); + } + + // Comparison operators + + #[test] + fn eq_eq_satisfied() { + assert_satisfied("== 1.2.3", "1.2.3"); + } + + #[test] + fn eq_eq_not_satisfied() { + assert_not_satisfied("== 1.2.3", "1.2.4"); + } + + #[test] + fn gte_satisfied() { + assert_satisfied(">= 1.0.0", "2.0.0"); + } + + #[test] + fn gte_exact_satisfied() { + assert_satisfied(">= 1.0.0", "1.0.0"); + } + + #[test] + fn gte_not_satisfied() { + assert_not_satisfied(">= 2.0.0", "1.9.9"); + } + + #[test] + fn gt_satisfied() { + assert_satisfied("> 1.0.0", "1.0.1"); + } + + #[test] + fn gt_not_satisfied_equal() { + assert_not_satisfied("> 1.0.0", "1.0.0"); + } + + #[test] + fn lte_satisfied() { + assert_satisfied("<= 2.0.0", "2.0.0"); + } + + #[test] + fn lte_not_satisfied() { + assert_not_satisfied("<= 2.0.0", "2.0.1"); + } + + #[test] + fn lt_satisfied() { + assert_satisfied("< 2.0.0", "1.9.9"); + } + + #[test] + fn lt_not_satisfied() { + assert_not_satisfied("< 2.0.0", "2.0.0"); + } + + // and/or conjunctions + + #[test] + fn and_conjunction_satisfied() { + assert_satisfied(">= 1.0.0 and < 2.0.0", "1.5.0"); + } + + #[test] + fn and_conjunction_not_satisfied() { + assert_not_satisfied(">= 1.0.0 and < 2.0.0", "2.0.0"); + } + + #[test] + fn or_conjunction_satisfied_first() { + assert_satisfied("== 1.0.0 or == 2.0.0", "1.0.0"); + } + + #[test] + fn or_conjunction_satisfied_second() { + assert_satisfied("== 1.0.0 or == 2.0.0", "2.0.0"); + } + + #[test] + fn or_conjunction_not_satisfied() { + assert_not_satisfied("== 1.0.0 or == 2.0.0", "3.0.0"); + } + + // Skip cases + + #[test] + fn dep_not_found_skipped() { + let temp = tempfile::tempdir().unwrap(); + write_mix_with_dep(temp.path(), r#""~> 1.0""#); + let manifest = temp.path().join("mix.exs"); + let result = check_dependency_constraint(&manifest, "nonexistent", "*", "1.0.0").unwrap(); + assert!(matches!(result, ConstraintCheckResult::Skipped { .. })); + } + + #[test] + fn invalid_version_skipped() { + assert_skipped("~> 1.0", "invalid"); + } + + #[test] + fn empty_version_skipped() { + assert_skipped("~> 1.0", ""); + } + + // Bare pinned version → skipped + + #[test] + fn bare_pinned_version_skipped() { + assert_skipped("1.2.3", "1.2.3"); + } + + #[test] + fn bare_pinned_different_version_skipped() { + assert_skipped("1.2.3", "1.2.4"); + } + + // Pre-release version → skipped + + #[test] + fn prerelease_version_skipped() { + assert_skipped("~> 1.0", "1.0.0-rc.1"); + } + + // Path dependency (no requirement string) → skipped + + #[test] + fn path_dep_without_requirement_skipped() { + let temp = tempfile::tempdir().unwrap(); + let content = r#" +defmodule Test.MixProject do + use Mix.Project + + def project do + [ + app: :test_app, + version: "0.1.0", + deps: deps() + ] + end + + defp deps do + [ + {:test_dep, path: "../test_dep"} + ] + end +end +"#; + fs::write(temp.path().join("mix.exs"), content).unwrap(); + let manifest = temp.path().join("mix.exs"); + let result = check_dependency_constraint(&manifest, "test_dep", "*", "1.0.0").unwrap(); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "path dep without version requirement should be skipped, got {:?}", + result + ); + } +} diff --git a/crates/sampo-core/src/adapters/hex/mix.rs b/crates/sampo-core/src/adapters/hex/mix.rs index d5d5dde..1153a30 100644 --- a/crates/sampo-core/src/adapters/hex/mix.rs +++ b/crates/sampo-core/src/adapters/hex/mix.rs @@ -878,5 +878,21 @@ fn path_from_keywords(keywords: Node<'_>, source: &str, manifest_dir: &Path) -> None } +/// Find the version constraint for a named dependency in a Mix manifest. +pub(crate) fn find_dependency_constraint_value( + manifest_path: &Path, + dep_name: &str, +) -> Result> { + let text = fs::read_to_string(manifest_path) + .map_err(|e| SampoError::Io(crate::errors::io_error_with_path(e, manifest_path)))?; + let deps = collect_dependencies(&text, manifest_path.parent().unwrap_or(Path::new("."))); + for dep in deps { + if dep.name == dep_name { + return Ok(dep.requirement.map(|r| r.value)); + } + } + Ok(None) +} + #[cfg(test)] mod mix_tests; From 101ba706a9871b17cd765643fe621a4a2bf23728 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 09:48:44 +0100 Subject: [PATCH 07/10] oops --- crates/sampo-core/src/adapters/hex.rs | 6 +++ .../sampo-core/src/adapters/hex/hex_tests.rs | 44 +++++++++++++++++++ crates/sampo-core/src/adapters/hex/mix.rs | 2 +- 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/sampo-core/src/adapters/hex.rs b/crates/sampo-core/src/adapters/hex.rs index dbd7196..0a9728f 100644 --- a/crates/sampo-core/src/adapters/hex.rs +++ b/crates/sampo-core/src/adapters/hex.rs @@ -152,6 +152,7 @@ pub(super) fn check_dependency_constraint( // Skip pinned (bare) versions without any operator or conjunction if !trimmed.starts_with("~>") + && !trimmed.starts_with("!=") && !trimmed.starts_with("==") && !trimmed.starts_with(">=") && !trimmed.starts_with("<=") @@ -269,6 +270,11 @@ fn satisfies_hex_comparator(comp: &str, version: (u64, u64, u64)) -> Option= 1.0.0 and < 2.0.0 or >= 3.0.0", "1.5.0"); + assert_satisfied(">= 1.0.0 and < 2.0.0 or >= 3.0.0", "3.0.0"); + assert_not_satisfied(">= 1.0.0 and < 2.0.0 or >= 3.0.0", "2.5.0"); + } + + #[test] + fn satisfies_hex_comparator_not_equal_in_conjunction() { + assert_satisfied("!= 1.0.0 and >= 0.5.0", "0.8.0"); + assert_not_satisfied("!= 1.0.0 and >= 0.5.0", "1.0.0"); + } + + // Skip cases + #[test] fn bare_pinned_version_skipped() { assert_skipped("1.2.3", "1.2.3"); @@ -287,4 +312,23 @@ end result ); } + + #[test] + fn compute_requirement_not_equal() { + let temp = tempfile::tempdir().unwrap(); + write_mix_with_dep(temp.path(), r#""!= 1.0.0""#); + let manifest = temp.path().join("mix.exs"); + let input = fs::read_to_string(&manifest).unwrap(); + let mut new_versions = std::collections::BTreeMap::new(); + new_versions.insert("test_dep".to_string(), "2.0.0".to_string()); + let (output, updated) = + update_manifest_versions(&manifest, &input, None, &new_versions).unwrap(); + assert!( + output.contains("!= 2.0.0"), + "expected constraint updated to '!= 2.0.0', got:\n{}", + output + ); + assert_eq!(updated.len(), 1); + assert_eq!(updated[0], ("test_dep".to_string(), "2.0.0".to_string())); + } } diff --git a/crates/sampo-core/src/adapters/hex/mix.rs b/crates/sampo-core/src/adapters/hex/mix.rs index 1153a30..027fffb 100644 --- a/crates/sampo-core/src/adapters/hex/mix.rs +++ b/crates/sampo-core/src/adapters/hex/mix.rs @@ -566,7 +566,7 @@ fn compute_requirement(old: &str, new_version: &str) -> Option { return None; } - const OPERATORS: [&str; 7] = ["~>", "==", ">=", "<=", ">", "<", "="]; + const OPERATORS: [&str; 8] = ["~>", "==", "!=", ">=", "<=", ">", "<", "="]; for op in OPERATORS { if let Some(rest) = trimmed.strip_prefix(op) { let current = rest.trim_start(); From 5a609bb809f3910cd8a057aea831145b3c74e919 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 11:27:46 +0100 Subject: [PATCH 08/10] implement range constraint for Packagist --- .../steadfast-princess-vainamoinen.md | 2 +- crates/sampo-core/src/adapters/packagist.rs | 570 +++++++++++++++++- 2 files changed, 565 insertions(+), 7 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index f07af92..4832f85 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -6,4 +6,4 @@ cargo/sampo-github-action: minor **⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump doesn't satisfy a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. -**Note:** Constraint validation is currently implemented for Cargo, npm, and Hex packages. Other ecosystems (PyPI, Packagist) will skip validation with an informative message until support is added. +**Note:** Constraint validation is currently implemented for Cargo, npm, Hex, and Packagist packages. PyPI will skip validation with an informative message until support is added. diff --git a/crates/sampo-core/src/adapters/packagist.rs b/crates/sampo-core/src/adapters/packagist.rs index f5fccd4..3ed3181 100644 --- a/crates/sampo-core/src/adapters/packagist.rs +++ b/crates/sampo-core/src/adapters/packagist.rs @@ -328,14 +328,84 @@ pub(super) fn publish_dry_run( } pub(super) fn check_dependency_constraint( - _manifest_path: &Path, - _dep_name: &str, + manifest_path: &Path, + dep_name: &str, _current_constraint: &str, - _new_version: &str, + new_version: &str, ) -> Result { - Ok(crate::types::ConstraintCheckResult::Skipped { - reason: "packagist constraint validation not yet implemented".to_string(), - }) + use crate::types::ConstraintCheckResult; + + let text = fs::read_to_string(manifest_path) + .map_err(|e| SampoError::Io(crate::errors::io_error_with_path(e, manifest_path)))?; + let manifest: JsonValue = serde_json::from_str(&text).map_err(|e| { + SampoError::Release(format!("Failed to parse {}: {}", manifest_path.display(), e)) + })?; + + let constraint = match find_dependency_constraint(&manifest, dep_name) { + Some(c) => c, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("dependency '{}' not found in manifest", dep_name), + }); + } + }; + + let trimmed = constraint.trim(); + + if trimmed.is_empty() { + return Ok(ConstraintCheckResult::Skipped { + reason: "empty constraint".to_string(), + }); + } + + if trimmed == "*" { + return Ok(ConstraintCheckResult::Satisfied); + } + + // Stability flags (@dev, @beta, etc.) change resolution strategy, not semver range + if trimmed.contains('@') { + return Ok(ConstraintCheckResult::Skipped { + reason: "stability flag in constraint".to_string(), + }); + } + + if new_version.contains('-') { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release version".to_string(), + }); + } + + if constraint_contains_prerelease(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release constraint".to_string(), + }); + } + + if is_pinned_version(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pinned version".to_string(), + }); + } + + let version = match parse_composer_version(new_version) { + Some(v) => v, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable version '{}'", new_version), + }); + } + }; + + match composer_version_satisfies(trimmed, version) { + Some(true) => Ok(ConstraintCheckResult::Satisfied), + Some(false) => Ok(ConstraintCheckResult::NotSatisfied { + constraint: trimmed.to_string(), + new_version: new_version.to_string(), + }), + None => Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable constraint '{}'", trimmed), + }), + } } fn enforce_packagist_rate_limit() { @@ -480,6 +550,229 @@ fn raw_span(raw: &RawValue, source: &str) -> Result<(usize, usize)> { Ok((start, end)) } +fn find_dependency_constraint(manifest: &JsonValue, dep_name: &str) -> Option { + for key in ["require", "require-dev"] { + if let Some(deps) = manifest.get(key).and_then(JsonValue::as_object) + && let Some(value) = deps.get(dep_name).and_then(JsonValue::as_str) + { + return Some(value.to_string()); + } + } + None +} + +fn parse_composer_version(s: &str) -> Option<(u64, u64, u64)> { + let s = s.trim().strip_prefix('v').unwrap_or(s.trim()); + if s.is_empty() { + return None; + } + let base = s.split('-').next()?; + let parts: Vec<&str> = base.split('.').collect(); + match parts.len() { + 3 => Some(( + parts[0].parse().ok()?, + parts[1].parse().ok()?, + parts[2].parse().ok()?, + )), + 2 => Some((parts[0].parse().ok()?, parts[1].parse().ok()?, 0)), + _ => None, + } +} + +fn constraint_contains_prerelease(constraint: &str) -> bool { + let bytes = constraint.as_bytes(); + for i in 1..bytes.len().saturating_sub(1) { + if bytes[i] == b'-' + && bytes[i - 1].is_ascii_digit() + && bytes[i + 1].is_ascii_alphanumeric() + { + return true; + } + } + false +} + +/// A pinned version is a bare `M.m.p` string with no operator, wildcard, or conjunction. +fn is_pinned_version(s: &str) -> bool { + let s = s.trim(); + !s.starts_with('^') + && !s.starts_with('~') + && !s.starts_with(">=") + && !s.starts_with("<=") + && !s.starts_with('>') + && !s.starts_with('<') + && !s.starts_with('!') + && !s.contains("||") + && !s.contains(',') + && !s.contains('*') + && parse_composer_version(s).is_some() +} + +fn normalize_comparator_whitespace(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + let bytes = s.as_bytes(); + let mut i = 0; + while i < bytes.len() { + let ch = bytes[i] as char; + result.push(ch); + if matches!(ch, '>' | '<' | '~' | '^' | '=' | '!') { + if i + 1 < bytes.len() && bytes[i + 1] == b'=' { + i += 1; + result.push('='); + } + while i + 1 < bytes.len() && bytes[i + 1] == b' ' { + i += 1; + } + } + i += 1; + } + result +} + +/// Returns `None` if the constraint is unparseable. +fn composer_version_satisfies(constraint: &str, version: (u64, u64, u64)) -> Option { + for or_part in constraint.split("||") { + let trimmed = or_part.trim(); + if trimmed.is_empty() || trimmed == "*" { + return Some(true); + } + match satisfies_and_group(trimmed, version) { + Some(true) => return Some(true), + Some(false) => continue, + None => return None, + } + } + Some(false) +} + +fn satisfies_and_group(group: &str, version: (u64, u64, u64)) -> Option { + // Split on comma for explicit AND, then each part may contain space-separated comparators + for comma_part in group.split(',') { + let normalized = normalize_comparator_whitespace(comma_part.trim()); + for comp in normalized.split_whitespace() { + if !satisfies_single_comparator(comp, version)? { + return Some(false); + } + } + } + Some(true) +} + +fn satisfies_single_comparator(comp: &str, version: (u64, u64, u64)) -> Option { + let comp = comp.trim(); + if comp.is_empty() || comp == "*" { + return Some(true); + } + + // Caret + if let Some(rest) = comp.strip_prefix('^') { + let rest = rest.trim(); + let parsed = parse_composer_version(rest)?; + let (lower, upper) = expand_caret(parsed); + return Some(version >= lower && version < upper); + } + + // Tilde — Composer: ~1.2 allows up to <2.0.0, unlike npm + if let Some(rest) = comp.strip_prefix('~') { + let rest = rest.trim(); + let parsed = parse_composer_version(rest)?; + let parts_count = rest.split('-').next()?.split('.').count(); + let (lower, upper) = expand_tilde_composer(parsed, parts_count); + return Some(version >= lower && version < upper); + } + + // != + if let Some(rest) = comp.strip_prefix("!=") { + let parsed = parse_composer_version(rest.trim())?; + return Some(version != parsed); + } + + // >= + if let Some(rest) = comp.strip_prefix(">=") { + let parsed = parse_composer_version(rest.trim())?; + return Some(version >= parsed); + } + + // > + if let Some(rest) = comp.strip_prefix('>') { + let parsed = parse_composer_version(rest.trim())?; + return Some(version > parsed); + } + + // <= + if let Some(rest) = comp.strip_prefix("<=") { + let parsed = parse_composer_version(rest.trim())?; + return Some(version <= parsed); + } + + // < + if let Some(rest) = comp.strip_prefix('<') { + let parsed = parse_composer_version(rest.trim())?; + return Some(version < parsed); + } + + // Wildcard + if comp.contains('*') { + return Some(matches_wildcard(comp, version)); + } + + // Bare version — exact match + let parsed = parse_composer_version(comp)?; + Some(version == parsed) +} + +/// Expand a caret range to inclusive lower and exclusive upper bounds. +/// +/// Allows changes that do not modify the left-most non-zero digit: +/// - `^1.2.3` → `[1.2.3, 2.0.0)` +/// - `^0.2.3` → `[0.2.3, 0.3.0)` +/// - `^0.0.3` → `[0.0.3, 0.0.4)` +fn expand_caret(v: (u64, u64, u64)) -> ((u64, u64, u64), (u64, u64, u64)) { + let lower = v; + let upper = if v.0 > 0 { + (v.0 + 1, 0, 0) + } else if v.1 > 0 { + (0, v.1 + 1, 0) + } else { + (0, 0, v.2 + 1) + }; + (lower, upper) +} + +/// Expand a tilde range using Composer semantics. +/// +/// - `~1.2.3` (3 parts) → `[1.2.3, 1.3.0)` — pins minor +/// - `~1.2` (2 parts) → `[1.2.0, 2.0.0)` — pins major (differs from npm!) +fn expand_tilde_composer( + v: (u64, u64, u64), + parts_count: usize, +) -> ((u64, u64, u64), (u64, u64, u64)) { + let lower = v; + let upper = if parts_count >= 3 { + (v.0, v.1 + 1, 0) + } else { + (v.0 + 1, 0, 0) + }; + (lower, upper) +} + +fn matches_wildcard(pattern: &str, version: (u64, u64, u64)) -> bool { + let parts: Vec<&str> = pattern.split('.').collect(); + match parts.len() { + 1 => parts[0] == "*", + 2 => parts[0].parse::().is_ok_and(|maj| version.0 == maj), + 3 => { + parts[0] + .parse::() + .is_ok_and(|maj| version.0 == maj) + && parts[1] + .parse::() + .is_ok_and(|min| version.1 == min) + } + _ => false, + } +} + /// Compute a new Composer version constraint based on the old constraint and new version. fn compute_dependency_constraint(old_spec: &str, new_version: &str) -> Option { let trimmed = old_spec.trim(); @@ -916,4 +1209,269 @@ mod tests { let result = PackagistAdapter.is_publishable(&path).unwrap(); assert!(!result); } + + mod constraint_validation { + use super::*; + use crate::types::ConstraintCheckResult; + + fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = format!( + r#"{{"name":"vendor/test","version":"1.0.0","require":{{"test/dep":"{}"}}}}"#, + constraint + ); + fs::write(&manifest_path, &content).unwrap(); + check_dependency_constraint(&manifest_path, "test/dep", "*", new_version).unwrap() + } + + fn assert_satisfied(constraint: &str, new_version: &str) { + assert_eq!( + assert_constraint(constraint, new_version), + ConstraintCheckResult::Satisfied, + "expected '{}' to be satisfied by '{}'", + constraint, + new_version + ); + } + + fn assert_not_satisfied(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "expected '{}' to be not satisfied by '{}', got {:?}", + constraint, + new_version, + result + ); + } + + fn assert_skipped(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected '{}' to be skipped for '{}', got {:?}", + constraint, + new_version, + result + ); + } + + #[test] + fn caret_satisfied() { + assert_satisfied("^1.2.3", "1.5.0"); + } + + #[test] + fn caret_exact_match() { + assert_satisfied("^1.2.3", "1.2.3"); + } + + #[test] + fn caret_zero_minor_satisfied() { + assert_satisfied("^0.2.3", "0.2.5"); + } + + #[test] + fn caret_not_satisfied_major_bump() { + assert_not_satisfied("^1.2.3", "2.0.0"); + } + + #[test] + fn caret_zero_minor_not_satisfied() { + assert_not_satisfied("^0.2.3", "0.3.0"); + } + + #[test] + fn caret_zero_zero_patch_not_satisfied() { + assert_not_satisfied("^0.0.3", "0.0.4"); + } + + #[test] + fn tilde_two_parts_satisfied() { + assert_satisfied("~1.2", "1.5.0"); + } + + #[test] + fn tilde_two_parts_not_satisfied() { + assert_not_satisfied("~1.2", "2.0.0"); + } + + #[test] + fn tilde_three_parts_satisfied() { + assert_satisfied("~1.2.3", "1.2.9"); + } + + #[test] + fn tilde_three_parts_not_satisfied() { + assert_not_satisfied("~1.2.3", "1.3.0"); + } + + #[test] + fn gte_satisfied() { + assert_satisfied(">=1.0.0", "2.0.0"); + } + + #[test] + fn gte_not_satisfied() { + assert_not_satisfied(">=2.0.0", "1.9.9"); + } + + #[test] + fn gt_satisfied() { + assert_satisfied(">1.0.0", "1.0.1"); + } + + #[test] + fn gt_not_satisfied_equal() { + assert_not_satisfied(">1.0.0", "1.0.0"); + } + + #[test] + fn lte_satisfied() { + assert_satisfied("<=2.0.0", "2.0.0"); + } + + #[test] + fn lte_not_satisfied() { + assert_not_satisfied("<=2.0.0", "2.0.1"); + } + + #[test] + fn lt_satisfied() { + assert_satisfied("<2.0.0", "1.9.9"); + } + + #[test] + fn lt_not_satisfied() { + assert_not_satisfied("<2.0.0", "2.0.0"); + } + + #[test] + fn ne_satisfied() { + assert_satisfied("!=1.0.0", "2.0.0"); + } + + #[test] + fn ne_not_satisfied() { + assert_not_satisfied("!=1.0.0", "1.0.0"); + } + + #[test] + fn and_comma_satisfied() { + assert_satisfied(">=1.0.0,<2.0.0", "1.5.0"); + } + + #[test] + fn and_comma_not_satisfied() { + assert_not_satisfied(">=1.0.0,<2.0.0", "2.0.0"); + } + + #[test] + fn and_space_satisfied() { + assert_satisfied(">=1.0.0 <2.0.0", "1.5.0"); + } + + #[test] + fn and_space_not_satisfied() { + assert_not_satisfied(">=1.0.0 <2.0.0", "2.0.0"); + } + + #[test] + fn or_satisfied() { + assert_satisfied("^1.0.0 || ^2.0.0", "2.1.0"); + } + + #[test] + fn or_not_satisfied() { + assert_not_satisfied("^1.0.0 || ^2.0.0", "3.0.0"); + } + + #[test] + fn wildcard_star_satisfied() { + assert_satisfied("*", "5.0.0"); + } + + #[test] + fn wildcard_patch_satisfied() { + assert_satisfied("1.0.*", "1.0.5"); + } + + #[test] + fn wildcard_patch_not_satisfied() { + assert_not_satisfied("1.0.*", "1.1.0"); + } + + #[test] + fn wildcard_minor_satisfied() { + assert_satisfied("1.*", "1.5.0"); + } + + #[test] + fn wildcard_minor_not_satisfied() { + assert_not_satisfied("1.*", "2.0.0"); + } + + #[test] + fn whitespace_gte() { + assert_satisfied(">= 1.0.0", "1.5.0"); + } + + #[test] + fn whitespace_caret() { + assert_satisfied("^ 1.2.3", "1.5.0"); + } + + #[test] + fn whitespace_tilde() { + assert_satisfied("~ 1.2.3", "1.2.9"); + } + + #[test] + fn skip_pinned_version() { + assert_skipped("1.2.3", "2.0.0"); + } + + #[test] + fn skip_prerelease_version() { + assert_skipped("^1.0.0", "2.0.0-beta.1"); + } + + #[test] + fn skip_prerelease_constraint() { + assert_skipped("^1.0.0-beta", "2.0.0"); + } + + #[test] + fn skip_stability_flag() { + assert_skipped("^1.0@dev", "2.0.0"); + } + + #[test] + fn skip_dep_not_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = r#"{"name":"vendor/test","version":"1.0.0","require":{}}"#; + fs::write(&manifest_path, content).unwrap(); + let result = + check_dependency_constraint(&manifest_path, "missing/dep", "*", "1.0.0").unwrap(); + assert!(matches!(result, ConstraintCheckResult::Skipped { .. })); + } + + #[test] + fn dev_deps_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = r#"{"name":"vendor/test","version":"1.0.0","require-dev":{"test/dep":"^1.0.0"}}"#; + fs::write(&manifest_path, content).unwrap(); + let result = + check_dependency_constraint(&manifest_path, "test/dep", "*", "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn skip_empty_constraint() { + assert_skipped("", "1.0.0"); + } + } } From 2b2301b1725ac721559b79ad7e70cfb614c52a40 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 11:40:48 +0100 Subject: [PATCH 09/10] extract packagist tests into separate module file --- crates/sampo-core/src/adapters/packagist.rs | 603 +----------------- .../src/adapters/packagist/packagist_tests.rs | 582 +++++++++++++++++ 2 files changed, 591 insertions(+), 594 deletions(-) create mode 100644 crates/sampo-core/src/adapters/packagist/packagist_tests.rs diff --git a/crates/sampo-core/src/adapters/packagist.rs b/crates/sampo-core/src/adapters/packagist.rs index 3ed3181..fd5b67f 100644 --- a/crates/sampo-core/src/adapters/packagist.rs +++ b/crates/sampo-core/src/adapters/packagist.rs @@ -338,7 +338,11 @@ pub(super) fn check_dependency_constraint( let text = fs::read_to_string(manifest_path) .map_err(|e| SampoError::Io(crate::errors::io_error_with_path(e, manifest_path)))?; let manifest: JsonValue = serde_json::from_str(&text).map_err(|e| { - SampoError::Release(format!("Failed to parse {}: {}", manifest_path.display(), e)) + SampoError::Release(format!( + "Failed to parse {}: {}", + manifest_path.display(), + e + )) })?; let constraint = match find_dependency_constraint(&manifest, dep_name) { @@ -582,9 +586,7 @@ fn parse_composer_version(s: &str) -> Option<(u64, u64, u64)> { fn constraint_contains_prerelease(constraint: &str) -> bool { let bytes = constraint.as_bytes(); for i in 1..bytes.len().saturating_sub(1) { - if bytes[i] == b'-' - && bytes[i - 1].is_ascii_digit() - && bytes[i + 1].is_ascii_alphanumeric() + if bytes[i] == b'-' && bytes[i - 1].is_ascii_digit() && bytes[i + 1].is_ascii_alphanumeric() { return true; } @@ -762,12 +764,8 @@ fn matches_wildcard(pattern: &str, version: (u64, u64, u64)) -> bool { 1 => parts[0] == "*", 2 => parts[0].parse::().is_ok_and(|maj| version.0 == maj), 3 => { - parts[0] - .parse::() - .is_ok_and(|maj| version.0 == maj) - && parts[1] - .parse::() - .is_ok_and(|min| version.1 == min) + parts[0].parse::().is_ok_and(|maj| version.0 == maj) + && parts[1].parse::().is_ok_and(|min| version.1 == min) } _ => false, } @@ -891,587 +889,4 @@ fn format_command_display(cmd: &Command) -> String { } #[cfg(test)] -mod tests { - use super::*; - use std::collections::BTreeMap; - use std::path::Path; - - #[test] - fn compute_dependency_constraint_caret() { - assert_eq!( - compute_dependency_constraint("^1.0.0", "2.0.0"), - Some("^2.0.0".to_string()) - ); - assert_eq!(compute_dependency_constraint("^2.0.0", "2.0.0"), None); - } - - #[test] - fn compute_dependency_constraint_tilde() { - assert_eq!( - compute_dependency_constraint("~1.0.0", "2.0.0"), - Some("~2.0.0".to_string()) - ); - assert_eq!(compute_dependency_constraint("~2.0.0", "2.0.0"), None); - } - - #[test] - fn compute_dependency_constraint_exact() { - assert_eq!( - compute_dependency_constraint("1.0.0", "2.0.0"), - Some("^2.0.0".to_string()) - ); - assert_eq!(compute_dependency_constraint("2.0.0", "2.0.0"), None); - } - - #[test] - fn compute_dependency_constraint_skips_complex() { - // Complex constraints with logical operators should not be modified - assert_eq!(compute_dependency_constraint(">=1.0 <2.0", "2.0.0"), None); - assert_eq!(compute_dependency_constraint("^1.0 || ^2.0", "3.0.0"), None); - } - - #[test] - fn compute_dependency_constraint_skips_comparison_operators() { - assert_eq!(compute_dependency_constraint(">=1.0.0", "2.0.0"), None); - assert_eq!(compute_dependency_constraint("<=2.0.0", "1.0.0"), None); - assert_eq!(compute_dependency_constraint(">1.0.0", "2.0.0"), None); - assert_eq!(compute_dependency_constraint("<2.0.0", "1.0.0"), None); - } - - #[test] - fn compute_dependency_constraint_skips_wildcard() { - assert_eq!(compute_dependency_constraint("1.0.*", "1.1.0"), None); - assert_eq!(compute_dependency_constraint("2.*", "2.1.0"), None); - } - - #[test] - fn compute_dependency_constraint_empty_uses_caret() { - assert_eq!( - compute_dependency_constraint("", "1.0.0"), - Some("^1.0.0".to_string()) - ); - } - - #[test] - fn update_manifest_versions_updates_version() { - let input = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "require": {} -}"#; - - let new_version_by_name = BTreeMap::new(); - let (output, applied) = update_manifest_versions( - Path::new("composer.json"), - input, - Some("2.0.0"), - &new_version_by_name, - ) - .unwrap(); - - assert!(output.contains(r#""version": "2.0.0""#)); - assert!(applied.is_empty()); - } - - #[test] - fn update_manifest_versions_updates_dependencies() { - let input = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "require": { - "other/dep": "^1.0.0" - } -}"#; - - let mut new_version_by_name = BTreeMap::new(); - new_version_by_name.insert("other/dep".to_string(), "2.0.0".to_string()); - - let (output, applied) = update_manifest_versions( - Path::new("composer.json"), - input, - None, - &new_version_by_name, - ) - .unwrap(); - - assert!(output.contains(r#""other/dep": "^2.0.0""#)); - assert_eq!(applied.len(), 1); - assert_eq!(applied[0].0, "other/dep"); - assert_eq!(applied[0].1, "2.0.0"); - } - - #[test] - fn update_manifest_versions_updates_dev_dependencies() { - let input = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "require-dev": { - "dev/package": "^1.0.0" - } -}"#; - - let mut new_version_by_name = BTreeMap::new(); - new_version_by_name.insert("dev/package".to_string(), "3.0.0".to_string()); - - let (output, applied) = update_manifest_versions( - Path::new("composer.json"), - input, - None, - &new_version_by_name, - ) - .unwrap(); - - assert!(output.contains(r#""dev/package": "^3.0.0""#)); - assert_eq!(applied.len(), 1); - } - - #[test] - fn update_manifest_versions_preserves_tilde_constraint() { - let input = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "require": { - "other/dep": "~1.0.0" - } -}"#; - - let mut new_version_by_name = BTreeMap::new(); - new_version_by_name.insert("other/dep".to_string(), "2.0.0".to_string()); - - let (output, _) = update_manifest_versions( - Path::new("composer.json"), - input, - None, - &new_version_by_name, - ) - .unwrap(); - - assert!(output.contains(r#""other/dep": "~2.0.0""#)); - } - - #[test] - fn update_manifest_versions_no_changes_when_same_version() { - let input = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "require": { - "other/dep": "^1.0.0" - } -}"#; - - let mut new_version_by_name = BTreeMap::new(); - new_version_by_name.insert("other/dep".to_string(), "1.0.0".to_string()); - - let (output, applied) = update_manifest_versions( - Path::new("composer.json"), - input, - Some("1.0.0"), - &new_version_by_name, - ) - .unwrap(); - - // No changes when versions are the same - assert_eq!(output, input); - assert!(applied.is_empty()); - } - - #[test] - fn discover_packagist_valid_package() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/my-package", - "version": "1.2.3", - "require": {} -}"#; - std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); - - let packages = discover_packagist(temp.path()).unwrap(); - assert_eq!(packages.len(), 1); - - let pkg = &packages[0]; - assert_eq!(pkg.name, "vendor/my-package"); - assert_eq!(pkg.version, "1.2.3"); - assert_eq!(pkg.kind, PackageKind::Packagist); - assert_eq!(pkg.identifier, "packagist/vendor/my-package"); - } - - #[test] - fn discover_packagist_requires_vendor_format() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "my-package", - "version": "1.0.0" -}"#; - std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); - - let result = discover_packagist(temp.path()); - assert!(result.is_err()); - let err = result.unwrap_err(); - assert!(err.to_string().contains("vendor/package")); - } - - #[test] - fn discover_packagist_missing_name() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "version": "1.0.0" -}"#; - std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); - - let result = discover_packagist(temp.path()); - assert!(result.is_err()); - let err = result.unwrap_err(); - assert!(err.to_string().contains("missing name")); - } - - #[test] - fn is_publishable_valid_package() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/package", - "version": "1.0.0" -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path).unwrap(); - assert!(result); - } - - #[test] - fn is_publishable_abandoned_package() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "abandoned": true -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path).unwrap(); - assert!(!result); - } - - #[test] - fn is_publishable_abandoned_with_replacement() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/package", - "version": "1.0.0", - "abandoned": "vendor/new-package" -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path).unwrap(); - assert!(!result); - } - - #[test] - fn is_publishable_missing_vendor_prefix() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "package-without-vendor", - "version": "1.0.0" -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path); - assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("vendor/package")); - } - - #[test] - fn is_publishable_missing_version() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/package" -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path).unwrap(); - assert!(!result); - } - - #[test] - fn is_publishable_empty_version() { - let temp = tempfile::tempdir().unwrap(); - let manifest = r#"{ - "name": "vendor/package", - "version": "" -}"#; - let path = temp.path().join("composer.json"); - std::fs::write(&path, manifest).unwrap(); - - let result = PackagistAdapter.is_publishable(&path).unwrap(); - assert!(!result); - } - - mod constraint_validation { - use super::*; - use crate::types::ConstraintCheckResult; - - fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { - let temp = tempfile::tempdir().unwrap(); - let manifest_path = temp.path().join("composer.json"); - let content = format!( - r#"{{"name":"vendor/test","version":"1.0.0","require":{{"test/dep":"{}"}}}}"#, - constraint - ); - fs::write(&manifest_path, &content).unwrap(); - check_dependency_constraint(&manifest_path, "test/dep", "*", new_version).unwrap() - } - - fn assert_satisfied(constraint: &str, new_version: &str) { - assert_eq!( - assert_constraint(constraint, new_version), - ConstraintCheckResult::Satisfied, - "expected '{}' to be satisfied by '{}'", - constraint, - new_version - ); - } - - fn assert_not_satisfied(constraint: &str, new_version: &str) { - let result = assert_constraint(constraint, new_version); - assert!( - matches!(result, ConstraintCheckResult::NotSatisfied { .. }), - "expected '{}' to be not satisfied by '{}', got {:?}", - constraint, - new_version, - result - ); - } - - fn assert_skipped(constraint: &str, new_version: &str) { - let result = assert_constraint(constraint, new_version); - assert!( - matches!(result, ConstraintCheckResult::Skipped { .. }), - "expected '{}' to be skipped for '{}', got {:?}", - constraint, - new_version, - result - ); - } - - #[test] - fn caret_satisfied() { - assert_satisfied("^1.2.3", "1.5.0"); - } - - #[test] - fn caret_exact_match() { - assert_satisfied("^1.2.3", "1.2.3"); - } - - #[test] - fn caret_zero_minor_satisfied() { - assert_satisfied("^0.2.3", "0.2.5"); - } - - #[test] - fn caret_not_satisfied_major_bump() { - assert_not_satisfied("^1.2.3", "2.0.0"); - } - - #[test] - fn caret_zero_minor_not_satisfied() { - assert_not_satisfied("^0.2.3", "0.3.0"); - } - - #[test] - fn caret_zero_zero_patch_not_satisfied() { - assert_not_satisfied("^0.0.3", "0.0.4"); - } - - #[test] - fn tilde_two_parts_satisfied() { - assert_satisfied("~1.2", "1.5.0"); - } - - #[test] - fn tilde_two_parts_not_satisfied() { - assert_not_satisfied("~1.2", "2.0.0"); - } - - #[test] - fn tilde_three_parts_satisfied() { - assert_satisfied("~1.2.3", "1.2.9"); - } - - #[test] - fn tilde_three_parts_not_satisfied() { - assert_not_satisfied("~1.2.3", "1.3.0"); - } - - #[test] - fn gte_satisfied() { - assert_satisfied(">=1.0.0", "2.0.0"); - } - - #[test] - fn gte_not_satisfied() { - assert_not_satisfied(">=2.0.0", "1.9.9"); - } - - #[test] - fn gt_satisfied() { - assert_satisfied(">1.0.0", "1.0.1"); - } - - #[test] - fn gt_not_satisfied_equal() { - assert_not_satisfied(">1.0.0", "1.0.0"); - } - - #[test] - fn lte_satisfied() { - assert_satisfied("<=2.0.0", "2.0.0"); - } - - #[test] - fn lte_not_satisfied() { - assert_not_satisfied("<=2.0.0", "2.0.1"); - } - - #[test] - fn lt_satisfied() { - assert_satisfied("<2.0.0", "1.9.9"); - } - - #[test] - fn lt_not_satisfied() { - assert_not_satisfied("<2.0.0", "2.0.0"); - } - - #[test] - fn ne_satisfied() { - assert_satisfied("!=1.0.0", "2.0.0"); - } - - #[test] - fn ne_not_satisfied() { - assert_not_satisfied("!=1.0.0", "1.0.0"); - } - - #[test] - fn and_comma_satisfied() { - assert_satisfied(">=1.0.0,<2.0.0", "1.5.0"); - } - - #[test] - fn and_comma_not_satisfied() { - assert_not_satisfied(">=1.0.0,<2.0.0", "2.0.0"); - } - - #[test] - fn and_space_satisfied() { - assert_satisfied(">=1.0.0 <2.0.0", "1.5.0"); - } - - #[test] - fn and_space_not_satisfied() { - assert_not_satisfied(">=1.0.0 <2.0.0", "2.0.0"); - } - - #[test] - fn or_satisfied() { - assert_satisfied("^1.0.0 || ^2.0.0", "2.1.0"); - } - - #[test] - fn or_not_satisfied() { - assert_not_satisfied("^1.0.0 || ^2.0.0", "3.0.0"); - } - - #[test] - fn wildcard_star_satisfied() { - assert_satisfied("*", "5.0.0"); - } - - #[test] - fn wildcard_patch_satisfied() { - assert_satisfied("1.0.*", "1.0.5"); - } - - #[test] - fn wildcard_patch_not_satisfied() { - assert_not_satisfied("1.0.*", "1.1.0"); - } - - #[test] - fn wildcard_minor_satisfied() { - assert_satisfied("1.*", "1.5.0"); - } - - #[test] - fn wildcard_minor_not_satisfied() { - assert_not_satisfied("1.*", "2.0.0"); - } - - #[test] - fn whitespace_gte() { - assert_satisfied(">= 1.0.0", "1.5.0"); - } - - #[test] - fn whitespace_caret() { - assert_satisfied("^ 1.2.3", "1.5.0"); - } - - #[test] - fn whitespace_tilde() { - assert_satisfied("~ 1.2.3", "1.2.9"); - } - - #[test] - fn skip_pinned_version() { - assert_skipped("1.2.3", "2.0.0"); - } - - #[test] - fn skip_prerelease_version() { - assert_skipped("^1.0.0", "2.0.0-beta.1"); - } - - #[test] - fn skip_prerelease_constraint() { - assert_skipped("^1.0.0-beta", "2.0.0"); - } - - #[test] - fn skip_stability_flag() { - assert_skipped("^1.0@dev", "2.0.0"); - } - - #[test] - fn skip_dep_not_found() { - let temp = tempfile::tempdir().unwrap(); - let manifest_path = temp.path().join("composer.json"); - let content = r#"{"name":"vendor/test","version":"1.0.0","require":{}}"#; - fs::write(&manifest_path, content).unwrap(); - let result = - check_dependency_constraint(&manifest_path, "missing/dep", "*", "1.0.0").unwrap(); - assert!(matches!(result, ConstraintCheckResult::Skipped { .. })); - } - - #[test] - fn dev_deps_found() { - let temp = tempfile::tempdir().unwrap(); - let manifest_path = temp.path().join("composer.json"); - let content = r#"{"name":"vendor/test","version":"1.0.0","require-dev":{"test/dep":"^1.0.0"}}"#; - fs::write(&manifest_path, content).unwrap(); - let result = - check_dependency_constraint(&manifest_path, "test/dep", "*", "1.5.0").unwrap(); - assert_eq!(result, ConstraintCheckResult::Satisfied); - } - - #[test] - fn skip_empty_constraint() { - assert_skipped("", "1.0.0"); - } - } -} +mod packagist_tests; diff --git a/crates/sampo-core/src/adapters/packagist/packagist_tests.rs b/crates/sampo-core/src/adapters/packagist/packagist_tests.rs new file mode 100644 index 0000000..6d27692 --- /dev/null +++ b/crates/sampo-core/src/adapters/packagist/packagist_tests.rs @@ -0,0 +1,582 @@ +use super::*; +use std::collections::BTreeMap; +use std::path::Path; + +#[test] +fn compute_dependency_constraint_caret() { + assert_eq!( + compute_dependency_constraint("^1.0.0", "2.0.0"), + Some("^2.0.0".to_string()) + ); + assert_eq!(compute_dependency_constraint("^2.0.0", "2.0.0"), None); +} + +#[test] +fn compute_dependency_constraint_tilde() { + assert_eq!( + compute_dependency_constraint("~1.0.0", "2.0.0"), + Some("~2.0.0".to_string()) + ); + assert_eq!(compute_dependency_constraint("~2.0.0", "2.0.0"), None); +} + +#[test] +fn compute_dependency_constraint_exact() { + assert_eq!( + compute_dependency_constraint("1.0.0", "2.0.0"), + Some("^2.0.0".to_string()) + ); + assert_eq!(compute_dependency_constraint("2.0.0", "2.0.0"), None); +} + +#[test] +fn compute_dependency_constraint_skips_complex() { + // Complex constraints with logical operators should not be modified + assert_eq!(compute_dependency_constraint(">=1.0 <2.0", "2.0.0"), None); + assert_eq!(compute_dependency_constraint("^1.0 || ^2.0", "3.0.0"), None); +} + +#[test] +fn compute_dependency_constraint_skips_comparison_operators() { + assert_eq!(compute_dependency_constraint(">=1.0.0", "2.0.0"), None); + assert_eq!(compute_dependency_constraint("<=2.0.0", "1.0.0"), None); + assert_eq!(compute_dependency_constraint(">1.0.0", "2.0.0"), None); + assert_eq!(compute_dependency_constraint("<2.0.0", "1.0.0"), None); +} + +#[test] +fn compute_dependency_constraint_skips_wildcard() { + assert_eq!(compute_dependency_constraint("1.0.*", "1.1.0"), None); + assert_eq!(compute_dependency_constraint("2.*", "2.1.0"), None); +} + +#[test] +fn compute_dependency_constraint_empty_uses_caret() { + assert_eq!( + compute_dependency_constraint("", "1.0.0"), + Some("^1.0.0".to_string()) + ); +} + +#[test] +fn update_manifest_versions_updates_version() { + let input = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "require": {} +}"#; + + let new_version_by_name = BTreeMap::new(); + let (output, applied) = update_manifest_versions( + Path::new("composer.json"), + input, + Some("2.0.0"), + &new_version_by_name, + ) + .unwrap(); + + assert!(output.contains(r#""version": "2.0.0""#)); + assert!(applied.is_empty()); +} + +#[test] +fn update_manifest_versions_updates_dependencies() { + let input = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "require": { + "other/dep": "^1.0.0" + } +}"#; + + let mut new_version_by_name = BTreeMap::new(); + new_version_by_name.insert("other/dep".to_string(), "2.0.0".to_string()); + + let (output, applied) = update_manifest_versions( + Path::new("composer.json"), + input, + None, + &new_version_by_name, + ) + .unwrap(); + + assert!(output.contains(r#""other/dep": "^2.0.0""#)); + assert_eq!(applied.len(), 1); + assert_eq!(applied[0].0, "other/dep"); + assert_eq!(applied[0].1, "2.0.0"); +} + +#[test] +fn update_manifest_versions_updates_dev_dependencies() { + let input = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "require-dev": { + "dev/package": "^1.0.0" + } +}"#; + + let mut new_version_by_name = BTreeMap::new(); + new_version_by_name.insert("dev/package".to_string(), "3.0.0".to_string()); + + let (output, applied) = update_manifest_versions( + Path::new("composer.json"), + input, + None, + &new_version_by_name, + ) + .unwrap(); + + assert!(output.contains(r#""dev/package": "^3.0.0""#)); + assert_eq!(applied.len(), 1); +} + +#[test] +fn update_manifest_versions_preserves_tilde_constraint() { + let input = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "require": { + "other/dep": "~1.0.0" + } +}"#; + + let mut new_version_by_name = BTreeMap::new(); + new_version_by_name.insert("other/dep".to_string(), "2.0.0".to_string()); + + let (output, _) = update_manifest_versions( + Path::new("composer.json"), + input, + None, + &new_version_by_name, + ) + .unwrap(); + + assert!(output.contains(r#""other/dep": "~2.0.0""#)); +} + +#[test] +fn update_manifest_versions_no_changes_when_same_version() { + let input = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "require": { + "other/dep": "^1.0.0" + } +}"#; + + let mut new_version_by_name = BTreeMap::new(); + new_version_by_name.insert("other/dep".to_string(), "1.0.0".to_string()); + + let (output, applied) = update_manifest_versions( + Path::new("composer.json"), + input, + Some("1.0.0"), + &new_version_by_name, + ) + .unwrap(); + + // No changes when versions are the same + assert_eq!(output, input); + assert!(applied.is_empty()); +} + +#[test] +fn discover_packagist_valid_package() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/my-package", + "version": "1.2.3", + "require": {} +}"#; + std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); + + let packages = discover_packagist(temp.path()).unwrap(); + assert_eq!(packages.len(), 1); + + let pkg = &packages[0]; + assert_eq!(pkg.name, "vendor/my-package"); + assert_eq!(pkg.version, "1.2.3"); + assert_eq!(pkg.kind, PackageKind::Packagist); + assert_eq!(pkg.identifier, "packagist/vendor/my-package"); +} + +#[test] +fn discover_packagist_requires_vendor_format() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "my-package", + "version": "1.0.0" +}"#; + std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); + + let result = discover_packagist(temp.path()); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.to_string().contains("vendor/package")); +} + +#[test] +fn discover_packagist_missing_name() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "version": "1.0.0" +}"#; + std::fs::write(temp.path().join("composer.json"), manifest).unwrap(); + + let result = discover_packagist(temp.path()); + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.to_string().contains("missing name")); +} + +#[test] +fn is_publishable_valid_package() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/package", + "version": "1.0.0" +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path).unwrap(); + assert!(result); +} + +#[test] +fn is_publishable_abandoned_package() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "abandoned": true +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path).unwrap(); + assert!(!result); +} + +#[test] +fn is_publishable_abandoned_with_replacement() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/package", + "version": "1.0.0", + "abandoned": "vendor/new-package" +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path).unwrap(); + assert!(!result); +} + +#[test] +fn is_publishable_missing_vendor_prefix() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "package-without-vendor", + "version": "1.0.0" +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("vendor/package")); +} + +#[test] +fn is_publishable_missing_version() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/package" +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path).unwrap(); + assert!(!result); +} + +#[test] +fn is_publishable_empty_version() { + let temp = tempfile::tempdir().unwrap(); + let manifest = r#"{ + "name": "vendor/package", + "version": "" +}"#; + let path = temp.path().join("composer.json"); + std::fs::write(&path, manifest).unwrap(); + + let result = PackagistAdapter.is_publishable(&path).unwrap(); + assert!(!result); +} + +mod constraint_validation { + use super::*; + use crate::types::ConstraintCheckResult; + + fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = format!( + r#"{{"name":"vendor/test","version":"1.0.0","require":{{"test/dep":"{}"}}}}"#, + constraint + ); + fs::write(&manifest_path, &content).unwrap(); + check_dependency_constraint(&manifest_path, "test/dep", "*", new_version).unwrap() + } + + fn assert_satisfied(constraint: &str, new_version: &str) { + assert_eq!( + assert_constraint(constraint, new_version), + ConstraintCheckResult::Satisfied, + "expected '{}' to be satisfied by '{}'", + constraint, + new_version + ); + } + + fn assert_not_satisfied(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "expected '{}' to be not satisfied by '{}', got {:?}", + constraint, + new_version, + result + ); + } + + fn assert_skipped(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected '{}' to be skipped for '{}', got {:?}", + constraint, + new_version, + result + ); + } + + #[test] + fn caret_satisfied() { + assert_satisfied("^1.2.3", "1.5.0"); + } + + #[test] + fn caret_exact_match() { + assert_satisfied("^1.2.3", "1.2.3"); + } + + #[test] + fn caret_zero_minor_satisfied() { + assert_satisfied("^0.2.3", "0.2.5"); + } + + #[test] + fn caret_not_satisfied_major_bump() { + assert_not_satisfied("^1.2.3", "2.0.0"); + } + + #[test] + fn caret_zero_minor_not_satisfied() { + assert_not_satisfied("^0.2.3", "0.3.0"); + } + + #[test] + fn caret_zero_zero_patch_not_satisfied() { + assert_not_satisfied("^0.0.3", "0.0.4"); + } + + #[test] + fn tilde_two_parts_satisfied() { + assert_satisfied("~1.2", "1.5.0"); + } + + #[test] + fn tilde_two_parts_not_satisfied() { + assert_not_satisfied("~1.2", "2.0.0"); + } + + #[test] + fn tilde_three_parts_satisfied() { + assert_satisfied("~1.2.3", "1.2.9"); + } + + #[test] + fn tilde_three_parts_not_satisfied() { + assert_not_satisfied("~1.2.3", "1.3.0"); + } + + #[test] + fn gte_satisfied() { + assert_satisfied(">=1.0.0", "2.0.0"); + } + + #[test] + fn gte_not_satisfied() { + assert_not_satisfied(">=2.0.0", "1.9.9"); + } + + #[test] + fn gt_satisfied() { + assert_satisfied(">1.0.0", "1.0.1"); + } + + #[test] + fn gt_not_satisfied_equal() { + assert_not_satisfied(">1.0.0", "1.0.0"); + } + + #[test] + fn lte_satisfied() { + assert_satisfied("<=2.0.0", "2.0.0"); + } + + #[test] + fn lte_not_satisfied() { + assert_not_satisfied("<=2.0.0", "2.0.1"); + } + + #[test] + fn lt_satisfied() { + assert_satisfied("<2.0.0", "1.9.9"); + } + + #[test] + fn lt_not_satisfied() { + assert_not_satisfied("<2.0.0", "2.0.0"); + } + + #[test] + fn ne_satisfied() { + assert_satisfied("!=1.0.0", "2.0.0"); + } + + #[test] + fn ne_not_satisfied() { + assert_not_satisfied("!=1.0.0", "1.0.0"); + } + + #[test] + fn and_comma_satisfied() { + assert_satisfied(">=1.0.0,<2.0.0", "1.5.0"); + } + + #[test] + fn and_comma_not_satisfied() { + assert_not_satisfied(">=1.0.0,<2.0.0", "2.0.0"); + } + + #[test] + fn and_space_satisfied() { + assert_satisfied(">=1.0.0 <2.0.0", "1.5.0"); + } + + #[test] + fn and_space_not_satisfied() { + assert_not_satisfied(">=1.0.0 <2.0.0", "2.0.0"); + } + + #[test] + fn or_satisfied() { + assert_satisfied("^1.0.0 || ^2.0.0", "2.1.0"); + } + + #[test] + fn or_not_satisfied() { + assert_not_satisfied("^1.0.0 || ^2.0.0", "3.0.0"); + } + + #[test] + fn wildcard_star_satisfied() { + assert_satisfied("*", "5.0.0"); + } + + #[test] + fn wildcard_patch_satisfied() { + assert_satisfied("1.0.*", "1.0.5"); + } + + #[test] + fn wildcard_patch_not_satisfied() { + assert_not_satisfied("1.0.*", "1.1.0"); + } + + #[test] + fn wildcard_minor_satisfied() { + assert_satisfied("1.*", "1.5.0"); + } + + #[test] + fn wildcard_minor_not_satisfied() { + assert_not_satisfied("1.*", "2.0.0"); + } + + #[test] + fn whitespace_gte() { + assert_satisfied(">= 1.0.0", "1.5.0"); + } + + #[test] + fn whitespace_caret() { + assert_satisfied("^ 1.2.3", "1.5.0"); + } + + #[test] + fn whitespace_tilde() { + assert_satisfied("~ 1.2.3", "1.2.9"); + } + + #[test] + fn skip_pinned_version() { + assert_skipped("1.2.3", "2.0.0"); + } + + #[test] + fn skip_prerelease_version() { + assert_skipped("^1.0.0", "2.0.0-beta.1"); + } + + #[test] + fn skip_prerelease_constraint() { + assert_skipped("^1.0.0-beta", "2.0.0"); + } + + #[test] + fn skip_stability_flag() { + assert_skipped("^1.0@dev", "2.0.0"); + } + + #[test] + fn skip_dep_not_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = r#"{"name":"vendor/test","version":"1.0.0","require":{}}"#; + fs::write(&manifest_path, content).unwrap(); + let result = + check_dependency_constraint(&manifest_path, "missing/dep", "*", "1.0.0").unwrap(); + assert!(matches!(result, ConstraintCheckResult::Skipped { .. })); + } + + #[test] + fn dev_deps_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("composer.json"); + let content = + r#"{"name":"vendor/test","version":"1.0.0","require-dev":{"test/dep":"^1.0.0"}}"#; + fs::write(&manifest_path, content).unwrap(); + let result = check_dependency_constraint(&manifest_path, "test/dep", "*", "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn skip_empty_constraint() { + assert_skipped("", "1.0.0"); + } +} From e495da6b1c78952219a1461e57bf5fee63f7d5a3 Mon Sep 17 00:00:00 2001 From: Goulven CLEC'H Date: Mon, 23 Feb 2026 12:50:23 +0100 Subject: [PATCH 10/10] implement range constraint for pypi --- .../steadfast-princess-vainamoinen.md | 2 - crates/sampo-core/src/adapters/pypi.rs | 26 +- crates/sampo-core/src/adapters/pypi/pip.rs | 314 ++++++++++++++++ .../src/adapters/pypi/pip/pip_tests.rs | 346 ++++++++++++++++++ 4 files changed, 679 insertions(+), 9 deletions(-) diff --git a/.sampo/changesets/steadfast-princess-vainamoinen.md b/.sampo/changesets/steadfast-princess-vainamoinen.md index 4832f85..a12ae95 100644 --- a/.sampo/changesets/steadfast-princess-vainamoinen.md +++ b/.sampo/changesets/steadfast-princess-vainamoinen.md @@ -5,5 +5,3 @@ cargo/sampo-github-action: minor --- **⚠️ breaking change:** Sampo no longer overwrites range constraints for internal dependencies (or skips them silently in some cases). During release planning, if a planned version bump doesn't satisfy a range constraint (e.g. bumping `foo` to `2.0.0` when another package requires `foo = "^1.0"`), you'll get either an error (for packages in `fixed` or `linked` groups) or a warning, instead of silently skipping. Pinned versions (e.g. `foo = "1.2.3"`) are still bumped automatically. - -**Note:** Constraint validation is currently implemented for Cargo, npm, Hex, and Packagist packages. PyPI will skip validation with an informative message until support is added. diff --git a/crates/sampo-core/src/adapters/pypi.rs b/crates/sampo-core/src/adapters/pypi.rs index a0f9324..83edca4 100644 --- a/crates/sampo-core/src/adapters/pypi.rs +++ b/crates/sampo-core/src/adapters/pypi.rs @@ -4,6 +4,7 @@ use reqwest::StatusCode; use reqwest::blocking::Client; use serde_json::Value as JsonValue; use std::collections::BTreeMap; +use std::fs; use std::path::{Path, PathBuf}; use std::sync::{Mutex, OnceLock}; use std::thread; @@ -144,16 +145,27 @@ impl PyPIAdapter { } } -/// Stub: returns `Skipped` until PyPI/PEP 440 constraint parsing is implemented. pub(super) fn check_dependency_constraint( - _manifest_path: &Path, - _dep_name: &str, + manifest_path: &Path, + dep_name: &str, _current_constraint: &str, - _new_version: &str, + new_version: &str, ) -> Result { - Ok(crate::types::ConstraintCheckResult::Skipped { - reason: "PyPI constraint validation not yet implemented".to_string(), - }) + use crate::types::ConstraintCheckResult; + + let text = fs::read_to_string(manifest_path) + .map_err(|e| SampoError::Io(crate::errors::io_error_with_path(e, manifest_path)))?; + + let constraint = match pip::find_dependency_constraint(&text, dep_name)? { + Some(c) => c, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("dependency '{}' not found in manifest", dep_name), + }); + } + }; + + pip::check_pep440_constraint(&constraint, new_version) } pub(super) fn publish_dry_run( diff --git a/crates/sampo-core/src/adapters/pypi/pip.rs b/crates/sampo-core/src/adapters/pypi/pip.rs index f24bd19..db5f659 100644 --- a/crates/sampo-core/src/adapters/pypi/pip.rs +++ b/crates/sampo-core/src/adapters/pypi/pip.rs @@ -722,5 +722,319 @@ fn format_command_display(cmd: &Command) -> String { text } +pub(super) fn find_dependency_constraint(source: &str, dep_name: &str) -> Result> { + let doc: DocumentMut = source + .parse() + .map_err(|e| SampoError::Release(format!("Failed to parse pyproject.toml: {}", e)))?; + let Some(project) = doc.get("project").and_then(Item::as_table) else { + return Ok(None); + }; + let target = normalize_package_name(dep_name); + + let find_in_array = |arr: &toml_edit::Array| -> Option { + for item in arr.iter() { + let Some(spec) = item.as_str() else { continue }; + let Some(name) = extract_package_name(spec) else { + continue; + }; + if normalize_package_name(&name) != target { + continue; + } + return Some(extract_constraint_from_spec(spec)); + } + None + }; + + if let Some(deps) = project.get("dependencies").and_then(Item::as_array) + && let Some(c) = find_in_array(deps) + { + return Ok(Some(c)); + } + + if let Some(optional) = project + .get("optional-dependencies") + .and_then(Item::as_table) + { + for (_, group) in optional.iter() { + if let Some(arr) = group.as_array() + && let Some(c) = find_in_array(arr) + { + return Ok(Some(c)); + } + } + } + + Ok(None) +} + +fn extract_constraint_from_spec(spec: &str) -> String { + let trimmed = spec.trim(); + + let without_markers = match trimmed.find(';') { + Some(pos) => trimmed[..pos].trim(), + None => trimmed, + }; + + // URL dependencies have no parseable constraint + if without_markers.contains(" @ ") { + return String::new(); + } + + let after_extras = match (without_markers.find('['), without_markers.find(']')) { + (Some(start), Some(end)) if start < end => &without_markers[end + 1..], + _ => { + let name_end = without_markers + .find(|c: char| ['>', '<', '=', '!', '~'].contains(&c)) + .unwrap_or(without_markers.len()); + &without_markers[name_end..] + } + }; + + after_extras.trim().to_string() +} + +pub(super) fn check_pep440_constraint( + constraint: &str, + new_version: &str, +) -> Result { + use crate::types::ConstraintCheckResult; + + let trimmed = constraint.trim(); + + if trimmed.is_empty() { + return Ok(ConstraintCheckResult::Skipped { + reason: "empty constraint".to_string(), + }); + } + + if trimmed == "*" { + return Ok(ConstraintCheckResult::Satisfied); + } + + if new_version.contains('-') { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release version".to_string(), + }); + } + + if pep440_constraint_contains_prerelease(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pre-release constraint".to_string(), + }); + } + + if is_pep440_pinned_version(trimmed) { + return Ok(ConstraintCheckResult::Skipped { + reason: "pinned version".to_string(), + }); + } + + let version = match parse_pep440_version(new_version) { + Some(v) => v, + None => { + return Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable version '{}'", new_version), + }); + } + }; + + match pep440_version_satisfies(trimmed, version) { + Some(true) => Ok(ConstraintCheckResult::Satisfied), + Some(false) => Ok(ConstraintCheckResult::NotSatisfied { + constraint: trimmed.to_string(), + new_version: new_version.to_string(), + }), + None => Ok(ConstraintCheckResult::Skipped { + reason: format!("unparseable constraint '{}'", trimmed), + }), + } +} + +fn strip_post_release_suffix(s: &str) -> &str { + let lower = s.to_ascii_lowercase(); + if let Some(pos) = lower.rfind(".post") { + let after = &s[pos + 5..]; + if after.is_empty() || after.chars().all(|c| c.is_ascii_digit()) { + return &s[..pos]; + } + } + s +} + +fn parse_pep440_version(s: &str) -> Option<(u64, u64, u64)> { + let s = s.trim().strip_prefix('v').unwrap_or(s.trim()); + if s.is_empty() { + return None; + } + + let lower = s.to_ascii_lowercase(); + for marker in &[".dev", "a", "b", "rc"] { + if let Some(pos) = lower.find(marker) { + // Only treat as pre-release if preceded by a digit (avoids matching + // inside regular version segments like "abc") + if pos > 0 && s.as_bytes()[pos - 1].is_ascii_digit() { + return None; + } + } + } + + // .postN is stable, not pre-release + let s = strip_post_release_suffix(s); + + let parts: Vec<&str> = s.split('.').collect(); + match parts.len() { + 2 => Some((parts[0].parse().ok()?, parts[1].parse().ok()?, 0)), + 3 => Some(( + parts[0].parse().ok()?, + parts[1].parse().ok()?, + parts[2].parse().ok()?, + )), + _ => None, + } +} + +fn pep440_constraint_contains_prerelease(constraint: &str) -> bool { + let lower = constraint.to_ascii_lowercase(); + let bytes = lower.as_bytes(); + for marker in &["dev", "rc"] { + if let Some(pos) = lower.find(marker) + && (pos > 0 && bytes[pos - 1].is_ascii_digit() + || (pos > 0 && bytes[pos - 1] == b'.') + && pos > 1 + && bytes[pos - 2].is_ascii_digit()) + { + return true; + } + } + // Single-letter 'a'/'b' pre-release tags (e.g. `1.0a1`, `2.0b3`) + for i in 1..bytes.len() { + if (bytes[i] == b'a' || bytes[i] == b'b') + && bytes[i - 1].is_ascii_digit() + && i + 1 < bytes.len() + && bytes[i + 1].is_ascii_digit() + { + return true; + } + } + false +} + +/// A pinned version is a bare version number with no operator, wildcard, or comma. +fn is_pep440_pinned_version(s: &str) -> bool { + let s = s.trim(); + for op in VersionOperator::ALL { + if s.starts_with(op.as_str()) { + return false; + } + } + !s.contains(',') && !s.contains('*') && parse_pep440_version(s).is_some() +} + +/// Returns `None` if the constraint is unparseable. +fn pep440_version_satisfies(constraint: &str, version: (u64, u64, u64)) -> Option { + for part in constraint.split(',') { + match satisfies_single_pep440_specifier(part.trim(), version) { + Some(true) => continue, + Some(false) => return Some(false), + None => return None, + } + } + Some(true) +} + +fn satisfies_single_pep440_specifier(spec: &str, version: (u64, u64, u64)) -> Option { + let spec = spec.trim(); + if spec.is_empty() || spec == "*" { + return Some(true); + } + + // ~= + if let Some(rest) = spec.strip_prefix("~=") { + let rest = rest.trim(); + let parts_count = rest.split('.').count(); + let parsed = parse_pep440_version(rest)?; + let (lower, upper) = expand_compatible_release(parsed, parts_count); + return Some(version >= lower && version < upper); + } + + // == + if let Some(rest) = spec.strip_prefix("==") { + let rest = rest.trim(); + if rest.ends_with(".*") { + return Some(matches_pep440_wildcard(rest, version)); + } + let parsed = parse_pep440_version(rest)?; + return Some(version == parsed); + } + + // != + if let Some(rest) = spec.strip_prefix("!=") { + let rest = rest.trim(); + if rest.ends_with(".*") { + return Some(!matches_pep440_wildcard(rest, version)); + } + let parsed = parse_pep440_version(rest)?; + return Some(version != parsed); + } + + // >= + if let Some(rest) = spec.strip_prefix(">=") { + let parsed = parse_pep440_version(rest.trim())?; + return Some(version >= parsed); + } + + // <= + if let Some(rest) = spec.strip_prefix("<=") { + let parsed = parse_pep440_version(rest.trim())?; + return Some(version <= parsed); + } + + // > + if let Some(rest) = spec.strip_prefix('>') { + let parsed = parse_pep440_version(rest.trim())?; + return Some(version > parsed); + } + + // < + if let Some(rest) = spec.strip_prefix('<') { + let parsed = parse_pep440_version(rest.trim())?; + return Some(version < parsed); + } + + // Bare version — exact match + let parsed = parse_pep440_version(spec)?; + Some(version == parsed) +} + +/// Expand a PEP 440 compatible release (`~=`) to inclusive lower and exclusive upper bounds. +/// +/// - `~=1.4.2` (3 parts) → `[1.4.2, 1.5.0)` +/// - `~=1.4` (2 parts) → `[1.4.0, 2.0.0)` +fn expand_compatible_release( + v: (u64, u64, u64), + parts_count: usize, +) -> ((u64, u64, u64), (u64, u64, u64)) { + let lower = v; + let upper = if parts_count >= 3 { + (v.0, v.1 + 1, 0) + } else { + (v.0 + 1, 0, 0) + }; + (lower, upper) +} + +fn matches_pep440_wildcard(pattern: &str, version: (u64, u64, u64)) -> bool { + let stripped = pattern.trim_end_matches(".*"); + let parts: Vec<&str> = stripped.split('.').collect(); + match parts.len() { + 1 => parts[0].parse::().is_ok_and(|maj| version.0 == maj), + 2 => { + parts[0].parse::().is_ok_and(|maj| version.0 == maj) + && parts[1].parse::().is_ok_and(|min| version.1 == min) + } + _ => false, + } +} + #[cfg(test)] mod pip_tests; diff --git a/crates/sampo-core/src/adapters/pypi/pip/pip_tests.rs b/crates/sampo-core/src/adapters/pypi/pip/pip_tests.rs index c679f92..a59065d 100644 --- a/crates/sampo-core/src/adapters/pypi/pip/pip_tests.rs +++ b/crates/sampo-core/src/adapters/pypi/pip/pip_tests.rs @@ -1537,3 +1537,349 @@ version = "0.1.0" assert_eq!(paths.len(), 1); assert!(paths.iter().next().unwrap().ends_with("valid-pkg")); } + +mod constraint_validation { + use super::*; + use crate::types::ConstraintCheckResult; + + fn assert_constraint(constraint: &str, new_version: &str) -> ConstraintCheckResult { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = format!( + r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["test-dep{}"] +"#, + constraint + ); + fs::write(&manifest_path, &content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = match find_dependency_constraint(&text, "test-dep").unwrap() { + Some(c) => c, + None => { + return ConstraintCheckResult::Skipped { + reason: "dependency 'test-dep' not found in manifest".to_string(), + }; + } + }; + check_pep440_constraint(&constraint, new_version).unwrap() + } + + fn assert_satisfied(constraint: &str, new_version: &str) { + assert_eq!( + assert_constraint(constraint, new_version), + ConstraintCheckResult::Satisfied, + "expected '{}' to be satisfied by '{}'", + constraint, + new_version + ); + } + + fn assert_not_satisfied(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::NotSatisfied { .. }), + "expected '{}' to be not satisfied by '{}', got {:?}", + constraint, + new_version, + result + ); + } + + fn assert_skipped(constraint: &str, new_version: &str) { + let result = assert_constraint(constraint, new_version); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected '{}' to be skipped for '{}', got {:?}", + constraint, + new_version, + result + ); + } + + #[test] + fn compatible_release_3_part_satisfied() { + assert_satisfied("~=1.4.2", "1.4.5"); + } + + #[test] + fn compatible_release_2_part_satisfied() { + assert_satisfied("~=1.4", "1.5.0"); + } + + #[test] + fn compatible_release_3_part_not_satisfied() { + assert_not_satisfied("~=1.4.2", "1.5.0"); + } + + #[test] + fn compatible_release_2_part_not_satisfied() { + assert_not_satisfied("~=1.4", "2.0.0"); + } + + #[test] + fn exact_satisfied() { + assert_satisfied("==1.2.3", "1.2.3"); + } + + #[test] + fn exact_not_satisfied() { + assert_not_satisfied("==1.2.3", "1.2.4"); + } + + #[test] + fn exact_wildcard_satisfied() { + assert_satisfied("==1.2.*", "1.2.5"); + } + + #[test] + fn exact_wildcard_not_satisfied() { + assert_not_satisfied("==1.2.*", "1.3.0"); + } + + #[test] + fn not_equal_satisfied() { + assert_satisfied("!=1.0.0", "2.0.0"); + } + + #[test] + fn not_equal_not_satisfied() { + assert_not_satisfied("!=1.0.0", "1.0.0"); + } + + #[test] + fn not_equal_wildcard_satisfied() { + assert_satisfied("!=1.0.*", "2.0.0"); + } + + #[test] + fn not_equal_wildcard_not_satisfied() { + assert_not_satisfied("!=1.0.*", "1.0.5"); + } + + #[test] + fn gte_satisfied() { + assert_satisfied(">=1.0.0", "2.0.0"); + } + + #[test] + fn gte_not_satisfied() { + assert_not_satisfied(">=2.0.0", "1.9.9"); + } + + #[test] + fn gt_satisfied() { + assert_satisfied(">1.0.0", "1.0.1"); + } + + #[test] + fn gt_not_satisfied_equal() { + assert_not_satisfied(">1.0.0", "1.0.0"); + } + + #[test] + fn lte_satisfied() { + assert_satisfied("<=2.0.0", "2.0.0"); + } + + #[test] + fn lte_not_satisfied() { + assert_not_satisfied("<=2.0.0", "2.0.1"); + } + + #[test] + fn lt_satisfied() { + assert_satisfied("<2.0.0", "1.9.9"); + } + + #[test] + fn lt_not_satisfied() { + assert_not_satisfied("<2.0.0", "2.0.0"); + } + + #[test] + fn compound_and_satisfied() { + assert_satisfied(">=1.0.0,<2.0.0", "1.5.0"); + } + + #[test] + fn compound_and_not_satisfied() { + assert_not_satisfied(">=1.0.0,<2.0.0", "2.0.0"); + } + + #[test] + fn compound_and_whitespace_satisfied() { + assert_satisfied(">=1.0, <2.0", "1.5.0"); + } + + #[test] + fn skip_pinned_version() { + assert_skipped("1.2.3", "2.0.0"); + } + + #[test] + fn skip_prerelease_version() { + assert_skipped("~=1.0.0", "2.0.0-beta.1"); + } + + #[test] + fn skip_url_dependency() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["test-dep @ https://example.com/archive.tar.gz"] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "2.0.0").unwrap(); + assert!( + matches!(result, ConstraintCheckResult::Skipped { .. }), + "expected URL dependency to be skipped, got {:?}", + result + ); + } + + #[test] + fn skip_dep_not_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = [] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let result = find_dependency_constraint(&text, "missing-dep").unwrap(); + assert!(result.is_none()); + } + + #[test] + fn skip_empty_constraint() { + assert_skipped("", "1.0.0"); + } + + #[test] + fn skip_prerelease_in_constraint() { + assert_skipped(">=1.0.0a1", "2.0.0"); + } + + #[test] + fn extras_parsed_correctly() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["test-dep[extra]>=1.0.0"] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn marker_stripped_constraint_evaluated() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["test-dep>=1.0.0; python_version>=\"3.8\""] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn extras_and_marker_stripped() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["test-dep[extra]>=1.0.0; python_version>=\"3.8\""] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn pep503_normalization() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = ["Test_Dep>=1.0.0"] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn optional_dependency_found() { + let temp = tempfile::tempdir().unwrap(); + let manifest_path = temp.path().join("pyproject.toml"); + let content = r#"[project] +name = "test-pkg" +version = "1.0.0" +dependencies = [] + +[project.optional-dependencies] +dev = ["test-dep>=1.0.0"] +"#; + fs::write(&manifest_path, content).unwrap(); + + let text = fs::read_to_string(&manifest_path).unwrap(); + let constraint = find_dependency_constraint(&text, "test-dep") + .unwrap() + .unwrap(); + let result = check_pep440_constraint(&constraint, "1.5.0").unwrap(); + assert_eq!(result, ConstraintCheckResult::Satisfied); + } + + #[test] + fn post_release_version_not_skipped() { + // .post is stable, must not be skipped as pre-release + assert_satisfied(">=1.0.0", "1.0.0.post1"); + } + + #[test] + fn post_release_constraint_not_skipped() { + // .post in constraint is stable, must not be skipped + assert_satisfied(">=1.0.0.post1", "2.0.0"); + } +}