diff --git a/.changeset/changesets/rancorously-immortal-goosefish.md b/.changeset/changesets/rancorously-immortal-goosefish.md new file mode 100644 index 0000000..f9fadaf --- /dev/null +++ b/.changeset/changesets/rancorously-immortal-goosefish.md @@ -0,0 +1,5 @@ +--- +category: fixed +changeset-operations: patch +--- +Enforce `none_bump_behavior: disallow` during changeset creation diff --git a/.changeset/changesets/thriftily-muscular-marmoset.md b/.changeset/changesets/thriftily-muscular-marmoset.md new file mode 100644 index 0000000..d3e706e --- /dev/null +++ b/.changeset/changesets/thriftily-muscular-marmoset.md @@ -0,0 +1,5 @@ +--- +category: fixed +cargo-changeset: patch +--- +Enforce `none_bump_behavior: disallow` during changeset creation, preventing `none` bumps in both interactive and non-interactive modes. diff --git a/crates/cargo-changeset/src/commands/add.rs b/crates/cargo-changeset/src/commands/add.rs index 01b912c..f29a915 100644 --- a/crates/cargo-changeset/src/commands/add.rs +++ b/crates/cargo-changeset/src/commands/add.rs @@ -29,12 +29,16 @@ pub(super) fn run(args: AddArgs, start_path: &Path, writer: &dyn CliWriter) -> R ); } + let (root_config, _) = project_provider.load_configs(&project)?; + let none_bump_behavior = root_config.none_bump_behavior(); + let changeset_writer = FileSystemChangesetIO::new(project.root()); let input = build_input(&args)?; let result = if is_interactive() { - let interaction_provider = TerminalInteractionProvider::new(args.editor); + let interaction_provider = + TerminalInteractionProvider::new(args.editor, none_bump_behavior); let operation = AddOperation::new(project_provider, changeset_writer, interaction_provider); operation.execute(start_path, &input)? } else { diff --git a/crates/cargo-changeset/src/interaction/mod.rs b/crates/cargo-changeset/src/interaction/mod.rs index fa163b5..b6f6a64 100644 --- a/crates/cargo-changeset/src/interaction/mod.rs +++ b/crates/cargo-changeset/src/interaction/mod.rs @@ -7,10 +7,11 @@ use std::process::Command; use dialoguer::{Confirm, Input, MultiSelect, Select}; use strum::{EnumMessage, VariantArray}; -use changeset_core::{ChangeCategory, PackageInfo}; +use changeset_core::{ChangeCategory, NoneBumpBehavior, PackageInfo}; use changeset_git::DEFAULT_BASE_BRANCH; use changeset_manifest::{ - ChangelogLocation, ComparisonLinks, NoneBumpBehavior, ZeroVersionBehavior, + ChangelogLocation, ComparisonLinks, NoneBumpBehavior as ManifestNoneBumpBehavior, + ZeroVersionBehavior, }; use changeset_operations::traits::{ BumpSelection, CategorySelection, ChangelogSettingsInput, DescriptionInput, @@ -31,12 +32,16 @@ pub(crate) use selection_options::{ pub(crate) struct TerminalInteractionProvider { use_editor: bool, + none_bump_behavior: NoneBumpBehavior, } impl TerminalInteractionProvider { #[must_use] - pub(crate) fn new(use_editor: bool) -> Self { - Self { use_editor } + pub(crate) fn new(use_editor: bool, none_bump_behavior: NoneBumpBehavior) -> Self { + Self { + use_editor, + none_bump_behavior, + } } } @@ -78,6 +83,27 @@ impl InteractionProvider for TerminalInteractionProvider { } fn select_bump_type(&self, package_name: &str) -> Result { + if self.none_bump_behavior == NoneBumpBehavior::Disallow { + let options: Vec<(changeset_core::BumpType, &str)> = BumpTypeSelectionOption::VARIANTS + .iter() + .filter(|v| !matches!(v, BumpTypeSelectionOption::None)) + .filter_map(|v| { + let label = v.get_message()?; + Some(((*v).into(), label)) + }) + .collect(); + let selection = select_from_options( + &format!("Select bump type for '{package_name}'"), + &options, + 0, + ) + .map_err(cli_error_to_operation_error)?; + return Ok(match selection { + Some(bump) => BumpSelection::Selected(bump), + None => BumpSelection::Cancelled, + }); + } + let selection = select_variant::( &format!("Select bump type for '{package_name}'"), 0, @@ -281,7 +307,7 @@ impl InitInteractionProvider for TerminalInitInteractionProvider { let none_bump_behavior = select_none_bump_behavior().map_err(cli_error_to_operation_error)?; let none_bump_promote_message_template = - if none_bump_behavior == NoneBumpBehavior::PromoteToPatch { + if none_bump_behavior == ManifestNoneBumpBehavior::PromoteToPatch { Some( prompt_none_bump_promote_message_template() .map_err(cli_error_to_operation_error)?, @@ -519,7 +545,7 @@ fn select_zero_version_behavior() -> CliResult { .into()) } -fn select_none_bump_behavior() -> CliResult { +fn select_none_bump_behavior() -> CliResult { let selection = select_variant::("Select none bump behavior", 0)?; Ok(selection diff --git a/crates/cargo-changeset/tests/add_command.rs b/crates/cargo-changeset/tests/add_command.rs index 952eefe..3bdf260 100644 --- a/crates/cargo-changeset/tests/add_command.rs +++ b/crates/cargo-changeset/tests/add_command.rs @@ -15,6 +15,8 @@ fn create_workspace_with_underscored_crate() -> TempDir { } mod non_interactive { + use indoc::indoc; + use super::*; #[test] @@ -413,6 +415,91 @@ mod non_interactive { ); } + #[test] + fn add_with_bump_none_rejected_when_disallowed() { + let workspace = WorkspaceBuilder::single_package("test-crate", "1.0.0") + .workspace_toml_extra("[package.metadata.changeset]\nnone-bump-behavior = \"disallow\"") + .build(); + + assert_cmd::cargo::cargo_bin_cmd!("cargo-changeset") + .arg("add") + .arg("--bump") + .arg("none") + .arg("-m") + .arg("Internal refactoring") + .current_dir(workspace.path()) + .assert() + .failure() + .stdout(indoc! {" + Using package: test-crate (1.0.0) + "}) + .stderr(indoc! {" + error: changesets with bump type 'none' are disallowed; affected packages: test-crate + "}); + } + + #[test] + fn add_with_package_bump_none_rejected_when_disallowed() { + let workspace = WorkspaceBuilder::virtual_workspace() + .crate_member("crate-a", "0.1.0") + .crate_member("crate-b", "0.2.0") + .workspace_toml_extra( + "[workspace.metadata.changeset]\nnone-bump-behavior = \"disallow\"", + ) + .build(); + + assert_cmd::cargo::cargo_bin_cmd!("cargo-changeset") + .arg("add") + .arg("--package-bump") + .arg("crate-a:none") + .arg("-m") + .arg("Internal refactoring") + .current_dir(workspace.path()) + .assert() + .failure() + .stdout("") + .stderr(indoc! {" + error: changesets with bump type 'none' are disallowed; affected packages: crate-a + "}); + } + + #[test] + fn add_with_bump_patch_succeeds_when_none_disallowed() { + let workspace = WorkspaceBuilder::single_package("test-crate", "1.0.0") + .workspace_toml_extra("[package.metadata.changeset]\nnone-bump-behavior = \"disallow\"") + .build(); + + assert_cmd::cargo::cargo_bin_cmd!("cargo-changeset") + .arg("add") + .arg("--bump") + .arg("patch") + .arg("-m") + .arg("Fix a bug") + .current_dir(workspace.path()) + .assert() + .success() + .stderr(""); + + let changeset_dir = workspace.path().join(".changeset/changesets"); + let files: Vec<_> = fs::read_dir(&changeset_dir) + .expect("read dir") + .filter_map(|e| e.ok()) + .filter(|e| e.path().extension().is_some_and(|ext| ext == "md")) + .collect(); + assert_eq!(files.len(), 1, "should have one changeset file"); + + let content = fs::read_to_string(files[0].path()).expect("read changeset file"); + assert_eq!( + content, + indoc! {" + --- + test-crate: patch + --- + Fix a bug + "}, + ); + } + #[test] fn add_with_stdin_message() { let workspace = create_single_crate_workspace(); @@ -1089,6 +1176,106 @@ MOCK_EDITOR_EOF ); } + #[test] + fn interactive_bump_type_menu_hides_none_when_disallowed() { + let workspace = WorkspaceBuilder::virtual_workspace() + .crate_member("crate-a", "0.1.0") + .crate_member("crate-b", "0.2.0") + .workspace_toml_extra( + "[workspace.metadata.changeset]\nnone-bump-behavior = \"disallow\"", + ) + .build(); + let mut session = spawn_add_in_workspace(&workspace); + + session.wait_for("Select packages to include in changeset"); + session.toggle_item(0); + session.confirm(); + session.wait_for("Select bump type for 'crate-a'"); + session.assert_screen( + "bump type menu without none option", + indoc! {" + Select packages to include in changeset: crate-a (0.1.0) + Select bump type for 'crate-a': + > patch - Bug fixes (backwards compatible) + minor - New features (backwards compatible) + major - Breaking changes"}, + ); + + session.cancel(); + session.wait_for_exit(); + } + + #[test] + fn interactive_full_flow_with_none_disallowed() { + let workspace = WorkspaceBuilder::single_package("test-crate", "1.0.0") + .workspace_toml_extra("[package.metadata.changeset]\nnone-bump-behavior = \"disallow\"") + .build(); + let mut session = spawn_add_in_workspace(&workspace); + + session.wait_for("Select bump type for 'test-crate'"); + session.assert_screen( + "single-crate bump type menu without none", + indoc! {" + Using package: test-crate (1.0.0) + Select bump type for 'test-crate': + > patch - Bug fixes (backwards compatible) + minor - New features (backwards compatible) + major - Breaking changes"}, + ); + session.confirm(); + + session.wait_for("Select change category"); + session.assert_screen( + "category menu after bump type", + indoc! {" + Using package: test-crate (1.0.0) + Select bump type for 'test-crate': patch - Bug fixes (backwards compatible) + Select change category: + > changed - General changes (default) + added - New features + fixed - Bug fixes + deprecated - Deprecated features + removed - Removed features + security - Security fixes"}, + ); + session.confirm(); + + session.wait_for("Enter description (press Enter 3 times to finish):"); + session.assert_screen( + "description prompt", + indoc! {" + Using package: test-crate (1.0.0) + Select bump type for 'test-crate': patch - Bug fixes (backwards compatible) + Select change category: changed - General changes (default) + + Enter description (press Enter 3 times to finish):"}, + ); + session.type_line("Test with none disallowed"); + session.type_line(""); + session.type_line(""); + + session.wait_for("Created changeset"); + session.wait_for_exit(); + + let changeset_dir = workspace.path().join(".changeset/changesets"); + assert!( + changeset_dir.exists(), + ".changeset/changesets directory should exist" + ); + + let files: Vec<_> = fs::read_dir(&changeset_dir) + .expect("read dir") + .filter_map(|e| e.ok()) + .filter(|e| e.path().extension().is_some_and(|ext| ext == "md")) + .collect(); + assert_eq!(files.len(), 1, "should have one changeset file"); + + let content = fs::read_to_string(files[0].path()).expect("read file"); + assert!(content.contains("test-crate")); + assert!(content.contains("patch")); + assert!(content.contains("Test with none disallowed")); + } + #[test] fn interactive_editor_filters_comments() { let workspace = create_single_crate_workspace(); diff --git a/crates/changeset-operations/src/operations/add.rs b/crates/changeset-operations/src/operations/add.rs index df1a21a..b29471b 100644 --- a/crates/changeset-operations/src/operations/add.rs +++ b/crates/changeset-operations/src/operations/add.rs @@ -2,7 +2,9 @@ use std::borrow::Cow; use std::collections::HashMap; use std::path::{Path, PathBuf}; -use changeset_core::{BumpType, ChangeCategory, Changeset, PackageInfo, PackageRelease}; +use changeset_core::{ + BumpType, ChangeCategory, Changeset, NoneBumpBehavior, PackageInfo, PackageRelease, +}; use indexmap::IndexSet; use crate::Result; @@ -139,7 +141,9 @@ where Vec::new() }; - let Some(releases) = self.collect_releases(&packages, input)? else { + let Some(releases) = + self.collect_releases(&packages, input, root_config.none_bump_behavior())? + else { return Ok(AddResult::Cancelled); }; @@ -203,6 +207,7 @@ where &self, packages: &[PackageInfo], input: &AddInput, + none_bump_behavior: NoneBumpBehavior, ) -> Result>> { let mut releases = Vec::with_capacity(packages.len()); @@ -221,6 +226,19 @@ where releases.push(PackageRelease::new(package.name().clone(), bump_type)); } + if none_bump_behavior == NoneBumpBehavior::Disallow { + let disallowed: Vec = releases + .iter() + .filter(|r| r.bump_type().is_noop()) + .map(|r| r.name().clone()) + .collect(); + if !disallowed.is_empty() { + return Err(OperationError::NoneBumpDisallowed { + packages: disallowed, + }); + } + } + Ok(Some(releases)) } @@ -1024,6 +1042,183 @@ mod tests { } } + #[test] + fn none_bump_rejected_when_disallowed_explicit_bump() { + let root_config = changeset_project::RootChangesetConfig::default() + .with_none_bump_behavior(NoneBumpBehavior::Disallow); + let project_provider = + MockProjectProvider::single_package("my-crate", "1.0.0").with_root_config(root_config); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider::all_cancelled(); + + let operation = AddOperation::new(project_provider, writer, interaction); + + let input = AddInput { + packages: vec!["my-crate".to_string()], + bump: Some(BumpType::None), + description: Some("Internal change".to_string()), + ..Default::default() + }; + + let result = operation.execute(Path::new("/any"), &input); + + assert!(result.is_err()); + let err = result.expect_err("should reject none bump when disallowed"); + assert!(matches!( + err, + crate::OperationError::NoneBumpDisallowed { ref packages } + if packages == &["my-crate"] + )); + } + + #[test] + fn none_bump_rejected_when_disallowed_package_bumps() { + let root_config = changeset_project::RootChangesetConfig::default() + .with_none_bump_behavior(NoneBumpBehavior::Disallow); + let project_provider = + MockProjectProvider::workspace(vec![("crate-a", "1.0.0"), ("crate-b", "2.0.0")]) + .with_root_config(root_config); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider::all_cancelled(); + + let operation = AddOperation::new(project_provider, writer, interaction); + + let mut package_bumps = HashMap::new(); + package_bumps.insert("crate-a".to_string(), BumpType::None); + package_bumps.insert("crate-b".to_string(), BumpType::Patch); + + let input = AddInput { + package_bumps, + description: Some("Some change".to_string()), + ..Default::default() + }; + + let result = operation.execute(Path::new("/any"), &input); + + assert!(result.is_err()); + let err = result.expect_err("should reject none bump when disallowed"); + assert!(matches!( + err, + crate::OperationError::NoneBumpDisallowed { ref packages } + if packages == &["crate-a"] + )); + } + + #[test] + fn none_bump_allowed_when_behavior_is_allow() { + let root_config = changeset_project::RootChangesetConfig::default() + .with_none_bump_behavior(NoneBumpBehavior::Allow); + let project_provider = + MockProjectProvider::single_package("my-crate", "1.0.0").with_root_config(root_config); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider::all_cancelled(); + + let operation = AddOperation::new(project_provider, writer, interaction); + + let input = AddInput { + packages: vec!["my-crate".to_string()], + bump: Some(BumpType::None), + description: Some("Internal change".to_string()), + ..Default::default() + }; + + let result = operation + .execute(Path::new("/any"), &input) + .expect("none bump should be allowed when behavior is Allow"); + + assert!(matches!(result, AddResult::Created { .. })); + } + + #[test] + fn none_bump_allowed_when_behavior_is_promote_to_patch() { + let project_provider = MockProjectProvider::single_package("my-crate", "1.0.0"); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider::all_cancelled(); + + let operation = AddOperation::new(project_provider, writer, interaction); + + let input = AddInput { + packages: vec!["my-crate".to_string()], + bump: Some(BumpType::None), + description: Some("Internal change".to_string()), + ..Default::default() + }; + + let result = operation + .execute(Path::new("/any"), &input) + .expect("none bump should be allowed when behavior is PromoteToPatch (default)"); + + assert!(matches!(result, AddResult::Created { .. })); + } + + #[test] + fn mixed_bumps_with_none_rejected_when_disallowed() { + let root_config = changeset_project::RootChangesetConfig::default() + .with_none_bump_behavior(NoneBumpBehavior::Disallow); + let project_provider = MockProjectProvider::workspace(vec![ + ("crate-a", "1.0.0"), + ("crate-b", "2.0.0"), + ("crate-c", "3.0.0"), + ]) + .with_root_config(root_config); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider::all_cancelled(); + + let operation = AddOperation::new(project_provider, writer, interaction); + + let mut package_bumps = HashMap::new(); + package_bumps.insert("crate-a".to_string(), BumpType::None); + package_bumps.insert("crate-b".to_string(), BumpType::Patch); + package_bumps.insert("crate-c".to_string(), BumpType::None); + + let input = AddInput { + package_bumps, + description: Some("Some change".to_string()), + ..Default::default() + }; + + let result = operation.execute(Path::new("/any"), &input); + + assert!(result.is_err()); + let err = result.expect_err("should reject none bumps when disallowed"); + match err { + crate::OperationError::NoneBumpDisallowed { ref packages } => { + assert_eq!(packages.len(), 2); + assert!(packages.contains(&"crate-a".to_string())); + assert!(packages.contains(&"crate-c".to_string())); + } + _ => panic!("Expected NoneBumpDisallowed error, got: {err:?}"), + } + } + + #[test] + fn none_bump_rejected_when_disallowed_interactive() { + let packages = vec![make_package("my-crate", "1.0.0")]; + let root_config = changeset_project::RootChangesetConfig::default() + .with_none_bump_behavior(NoneBumpBehavior::Disallow); + let project_provider = + MockProjectProvider::single_package("my-crate", "1.0.0").with_root_config(root_config); + let writer = MockChangesetWriter::new(); + let interaction = MockInteractionProvider { + package_selection: PackageSelection::Selected(packages), + bump_selections: std::sync::Mutex::new(vec![BumpType::None]), + category_selection: CategorySelection::Selected(ChangeCategory::Changed), + description: DescriptionInput::Provided("test".to_string()), + }; + + let operation = AddOperation::new(project_provider, writer, interaction); + + let result = operation.execute(Path::new("/any"), &AddInput::default()); + + assert!(result.is_err()); + let err = result.expect_err("should reject none bump from interactive selection"); + assert!(matches!( + err, + crate::OperationError::NoneBumpDisallowed { ref packages } + if packages == &["my-crate"] + )); + } + #[test] fn single_package_never_calls_discover_additional_packages() { let project_provider =