From 3c879e3da3cf886601848cfe2652bf4cf6dbcb3e Mon Sep 17 00:00:00 2001 From: Juncheng Fang Date: Tue, 23 Dec 2025 11:30:39 +0800 Subject: [PATCH 1/5] fix(cargo_provider): support workspace virtual manifests --- commitizen/providers/cargo_provider.py | 157 ++++++++++++++----------- 1 file changed, 87 insertions(+), 70 deletions(-) diff --git a/commitizen/providers/cargo_provider.py b/commitizen/providers/cargo_provider.py index ca00f05e7..a4e18e3e6 100644 --- a/commitizen/providers/cargo_provider.py +++ b/commitizen/providers/cargo_provider.py @@ -1,41 +1,39 @@ from __future__ import annotations -import fnmatch -import glob +import fnmatch, glob from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Iterable, Any, cast from tomlkit import TOMLDocument, dumps, parse from tomlkit.exceptions import NonExistentKey - from commitizen.providers.base_provider import TomlProvider if TYPE_CHECKING: from tomlkit.items import AoT -class CargoProvider(TomlProvider): - """ - Cargo version management +DictLike = dict[str, Any] - With support for `workspaces` - """ + +class CargoProvider(TomlProvider): + """Cargo version management for virtual workspace manifests + version.workspace=true members.""" filename = "Cargo.toml" lock_filename = "Cargo.lock" @property def lock_file(self) -> Path: - return Path() / self.lock_filename + return Path(self.lock_filename) def get(self, document: TOMLDocument) -> str: - out = _try_get_workspace(document)["package"]["version"] - if TYPE_CHECKING: - assert isinstance(out, str) - return out + t = _root_version_table(document) + v = t.get("version") + if not isinstance(v, str): + raise TypeError("expected root version to be a string") + return v def set(self, document: TOMLDocument, version: str) -> None: - _try_get_workspace(document)["package"]["version"] = version + _root_version_table(document)["version"] = version def set_version(self, version: str) -> None: super().set_version(version) @@ -43,63 +41,82 @@ def set_version(self, version: str) -> None: self.set_lock_version(version) def set_lock_version(self, version: str) -> None: - cargo_toml_content = parse(self.file.read_text()) - cargo_lock_content = parse(self.lock_file.read_text()) - packages = cargo_lock_content["package"] - + cargo_toml = parse(self.file.read_text()) + cargo_lock = parse(self.lock_file.read_text()) + packages = cargo_lock["package"] if TYPE_CHECKING: assert isinstance(packages, AoT) - try: - cargo_package_name = cargo_toml_content["package"]["name"] # type: ignore[index] - if TYPE_CHECKING: - assert isinstance(cargo_package_name, str) - for i, package in enumerate(packages): - if package["name"] == cargo_package_name: - cargo_lock_content["package"][i]["version"] = version # type: ignore[index] - break - except NonExistentKey: - workspace = cargo_toml_content.get("workspace", {}) - if TYPE_CHECKING: - assert isinstance(workspace, dict) - workspace_members = workspace.get("members", []) - excluded_workspace_members = workspace.get("exclude", []) - members_inheriting: list[str] = [] - - for member in workspace_members: - for path in glob.glob(member, recursive=True): - if any( - fnmatch.fnmatch(path, pattern) - for pattern in excluded_workspace_members - ): - continue - - cargo_file = Path(path) / "Cargo.toml" - package_content = parse(cargo_file.read_text()).get("package", {}) - if TYPE_CHECKING: - assert isinstance(package_content, dict) - try: - version_workspace = package_content["version"]["workspace"] - if version_workspace is True: - package_name = package_content["name"] - if TYPE_CHECKING: - assert isinstance(package_name, str) - members_inheriting.append(package_name) - except NonExistentKey: - pass - - for i, package in enumerate(packages): - if package["name"] in members_inheriting: - cargo_lock_content["package"][i]["version"] = version # type: ignore[index] - - self.lock_file.write_text(dumps(cargo_lock_content)) - - -def _try_get_workspace(document: TOMLDocument) -> dict: + root_pkg = _table_get(cargo_toml, "package") + if root_pkg is not None: + name = root_pkg.get("name") + if isinstance(name, str): + _lock_set_versions(packages, {name}, version) + self.lock_file.write_text(dumps(cargo_lock)) + return + + ws = _table_get(cargo_toml, "workspace") or {} + members = cast(list[str], ws.get("members", []) or []) + excludes = cast(list[str], ws.get("exclude", []) or []) + inheriting = _workspace_inheriting_member_names(members, excludes) + _lock_set_versions(packages, inheriting, version) + self.lock_file.write_text(dumps(cargo_lock)) + + +def _table_get(doc: TOMLDocument, key: str) -> DictLike | None: + """Return a dict-like table for `key` if present, else None (type-safe for Pylance).""" try: - workspace = document["workspace"] - if TYPE_CHECKING: - assert isinstance(workspace, dict) - return workspace + v = doc[key] # tomlkit returns Container/Table-like; typing is loose except NonExistentKey: - return document + return None + return cast(DictLike, v) if hasattr(v, "get") else None + + +def _root_version_table(doc: TOMLDocument) -> DictLike: + """Prefer [workspace.package]; fallback to [package].""" + ws = _table_get(doc, "workspace") + if ws is not None: + pkg = ws.get("package") + if hasattr(pkg, "get"): + return cast(DictLike, pkg) + pkg = _table_get(doc, "package") + if pkg is None: + raise NonExistentKey('expected either [workspace.package] or [package]') + return pkg + + +def _is_workspace_inherited_version(v: Any) -> bool: + return hasattr(v, "get") and cast(DictLike, v).get("workspace") is True + + +def _iter_member_dirs(members: Iterable[str], excludes: Iterable[str]) -> Iterable[Path]: + for pat in members: + for p in glob.glob(pat, recursive=True): + if any(fnmatch.fnmatch(p, ex) for ex in excludes): + continue + yield Path(p) + + +def _workspace_inheriting_member_names(members: Iterable[str], excludes: Iterable[str]) -> set[str]: + out: set[str] = set() + for d in _iter_member_dirs(members, excludes): + cargo_file = d / "Cargo.toml" + if not cargo_file.exists(): + continue + pkg = parse(cargo_file.read_text()).get("package") + if not hasattr(pkg, "get"): + continue + pkgd = cast(DictLike, pkg) + if _is_workspace_inherited_version(pkgd.get("version")): + name = pkgd.get("name") + if isinstance(name, str): + out.add(name) + return out + + +def _lock_set_versions(packages: Any, names: set[str], version: str) -> None: + if not names: + return + for i, p in enumerate(packages): + if getattr(p, "get", None) and p.get("name") in names: + packages[i]["version"] = version # type: ignore[index] From 037719dd461986ef73d5a48bbf30876febcb0e44 Mon Sep 17 00:00:00 2001 From: Juncheng Fang Date: Sun, 4 Jan 2026 17:23:18 +0800 Subject: [PATCH 2/5] style: run formatter --- commitizen/providers/cargo_provider.py | 30 ++++++++++++++++---------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/commitizen/providers/cargo_provider.py b/commitizen/providers/cargo_provider.py index a4e18e3e6..da7821945 100644 --- a/commitizen/providers/cargo_provider.py +++ b/commitizen/providers/cargo_provider.py @@ -1,14 +1,18 @@ from __future__ import annotations -import fnmatch, glob +import fnmatch +import glob from pathlib import Path -from typing import TYPE_CHECKING, Iterable, Any, cast +from typing import TYPE_CHECKING, Any, cast from tomlkit import TOMLDocument, dumps, parse from tomlkit.exceptions import NonExistentKey + from commitizen.providers.base_provider import TomlProvider if TYPE_CHECKING: + from collections.abc import Iterable + from tomlkit.items import AoT @@ -56,8 +60,8 @@ def set_lock_version(self, version: str) -> None: return ws = _table_get(cargo_toml, "workspace") or {} - members = cast(list[str], ws.get("members", []) or []) - excludes = cast(list[str], ws.get("exclude", []) or []) + members = cast("list[str]", ws.get("members", []) or []) + excludes = cast("list[str]", ws.get("exclude", []) or []) inheriting = _workspace_inheriting_member_names(members, excludes) _lock_set_versions(packages, inheriting, version) self.lock_file.write_text(dumps(cargo_lock)) @@ -69,7 +73,7 @@ def _table_get(doc: TOMLDocument, key: str) -> DictLike | None: v = doc[key] # tomlkit returns Container/Table-like; typing is loose except NonExistentKey: return None - return cast(DictLike, v) if hasattr(v, "get") else None + return cast("DictLike", v) if hasattr(v, "get") else None def _root_version_table(doc: TOMLDocument) -> DictLike: @@ -78,18 +82,20 @@ def _root_version_table(doc: TOMLDocument) -> DictLike: if ws is not None: pkg = ws.get("package") if hasattr(pkg, "get"): - return cast(DictLike, pkg) + return cast("DictLike", pkg) pkg = _table_get(doc, "package") if pkg is None: - raise NonExistentKey('expected either [workspace.package] or [package]') + raise NonExistentKey("expected either [workspace.package] or [package]") return pkg def _is_workspace_inherited_version(v: Any) -> bool: - return hasattr(v, "get") and cast(DictLike, v).get("workspace") is True + return hasattr(v, "get") and cast("DictLike", v).get("workspace") is True -def _iter_member_dirs(members: Iterable[str], excludes: Iterable[str]) -> Iterable[Path]: +def _iter_member_dirs( + members: Iterable[str], excludes: Iterable[str] +) -> Iterable[Path]: for pat in members: for p in glob.glob(pat, recursive=True): if any(fnmatch.fnmatch(p, ex) for ex in excludes): @@ -97,7 +103,9 @@ def _iter_member_dirs(members: Iterable[str], excludes: Iterable[str]) -> Iterab yield Path(p) -def _workspace_inheriting_member_names(members: Iterable[str], excludes: Iterable[str]) -> set[str]: +def _workspace_inheriting_member_names( + members: Iterable[str], excludes: Iterable[str] +) -> set[str]: out: set[str] = set() for d in _iter_member_dirs(members, excludes): cargo_file = d / "Cargo.toml" @@ -106,7 +114,7 @@ def _workspace_inheriting_member_names(members: Iterable[str], excludes: Iterabl pkg = parse(cargo_file.read_text()).get("package") if not hasattr(pkg, "get"): continue - pkgd = cast(DictLike, pkg) + pkgd = cast("DictLike", pkg) if _is_workspace_inherited_version(pkgd.get("version")): name = pkgd.get("name") if isinstance(name, str): From 88bdeafb5d452c05a8f9f03fa8fa08376f1b3c57 Mon Sep 17 00:00:00 2001 From: Juncheng Fang Date: Sun, 4 Jan 2026 17:42:08 +0800 Subject: [PATCH 3/5] fix(types): remove unused type ignore --- commitizen/providers/cargo_provider.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commitizen/providers/cargo_provider.py b/commitizen/providers/cargo_provider.py index da7821945..362b234f5 100644 --- a/commitizen/providers/cargo_provider.py +++ b/commitizen/providers/cargo_provider.py @@ -127,4 +127,4 @@ def _lock_set_versions(packages: Any, names: set[str], version: str) -> None: return for i, p in enumerate(packages): if getattr(p, "get", None) and p.get("name") in names: - packages[i]["version"] = version # type: ignore[index] + packages[i]["version"] = version From 9632226ea39978928b66c96f3b55dcb8c6cdc406 Mon Sep 17 00:00:00 2001 From: Juncheng Fang Date: Mon, 5 Jan 2026 09:29:54 +0800 Subject: [PATCH 4/5] fix(provider): harden cargo version parsing and lockfile updates - prefer [workspace.package] version with safe fallback to [package] - handle virtual workspace + version.workspace=true members when updating Cargo.lock - add regression tests for error/continue branches and workspace edge cases --- tests/providers/test_cargo_provider.py | 143 +++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/tests/providers/test_cargo_provider.py b/tests/providers/test_cargo_provider.py index ea15fdbf3..ba30270f4 100644 --- a/tests/providers/test_cargo_provider.py +++ b/tests/providers/test_cargo_provider.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING import pytest +from tomlkit.exceptions import NonExistentKey from commitizen.providers import get_provider from commitizen.providers.cargo_provider import CargoProvider @@ -451,3 +452,145 @@ def test_cargo_provider_workspace_member_without_workspace_key( assert file.read_text() == dedent(expected_workspace_toml) # The lock file should remain unchanged since the member doesn't inherit workspace version assert lock_file.read_text() == dedent(expected_lock_content) + + +def test_cargo_provider_get_raises_when_version_is_not_string( + config: BaseConfig, + chdir: Path, +) -> None: + """CargoProvider.get should raise when root version is not a string.""" + (chdir / CargoProvider.filename).write_text( + dedent( + """\ + [package] + name = "whatever" + version = 1 + """ + ) + ) + config.settings["version_provider"] = "cargo" + + provider = get_provider(config) + assert isinstance(provider, CargoProvider) + + with pytest.raises(TypeError, match=r"expected root version to be a string"): + provider.get_version() + + +def test_cargo_provider_get_raises_when_no_package_tables( + config: BaseConfig, + chdir: Path, +) -> None: + """_root_version_table should raise when neither [workspace.package] nor [package] exists.""" + (chdir / CargoProvider.filename).write_text( + dedent( + """\ + [workspace] + members = [] + """ + ) + ) + config.settings["version_provider"] = "cargo" + + provider = get_provider(config) + assert isinstance(provider, CargoProvider) + + with pytest.raises( + NonExistentKey, match=r"expected either \[workspace\.package\] or \[package\]" + ): + provider.get_version() + + +def test_workspace_member_dir_without_cargo_toml_is_ignored( + config: BaseConfig, + chdir: Path, +) -> None: + """Cover: if not cargo_file.exists(): continue""" + workspace_toml = """\ + [workspace] + members = ["missing_manifest"] + + [workspace.package] + version = "0.1.0" + """ + lock_content = """\ + [[package]] + name = "missing_manifest" + version = "0.1.0" + source = "registry+https://github.com/rust-lang/crates.io-index" + checksum = "123abc" + """ + expected_workspace_toml = """\ + [workspace] + members = ["missing_manifest"] + + [workspace.package] + version = "42.1" + """ + + (chdir / CargoProvider.filename).write_text(dedent(workspace_toml)) + os.mkdir(chdir / "missing_manifest") # directory exists, but Cargo.toml does NOT + + (chdir / CargoProvider.lock_filename).write_text(dedent(lock_content)) + + config.settings["version_provider"] = "cargo" + provider = get_provider(config) + assert isinstance(provider, CargoProvider) + + provider.set_version("42.1") + assert (chdir / CargoProvider.filename).read_text() == dedent( + expected_workspace_toml + ) + # lock should remain unchanged since member cannot be inspected => not inheriting + assert (chdir / CargoProvider.lock_filename).read_text() == dedent(lock_content) + + +def test_workspace_member_with_non_table_package_is_ignored( + config: BaseConfig, + chdir: Path, +) -> None: + """Cover: if not hasattr(pkg, "get"): continue""" + workspace_toml = """\ + [workspace] + members = ["bad_package"] + + [workspace.package] + version = "0.1.0" + """ + member_toml = """\ + package = "oops" + """ + lock_content = """\ + [[package]] + name = "bad_package" + version = "0.1.0" + source = "registry+https://github.com/rust-lang/crates.io-index" + checksum = "123abc" + """ + expected_workspace_toml = """\ + [workspace] + members = ["bad_package"] + + [workspace.package] + version = "42.1" + """ + + (chdir / CargoProvider.filename).write_text(dedent(workspace_toml)) + + os.mkdir(chdir / "bad_package") + (chdir / "bad_package" / "Cargo.toml").write_text( + dedent(member_toml) + ) # package is str, not table + + (chdir / CargoProvider.lock_filename).write_text(dedent(lock_content)) + + config.settings["version_provider"] = "cargo" + provider = get_provider(config) + assert isinstance(provider, CargoProvider) + + provider.set_version("42.1") + assert (chdir / CargoProvider.filename).read_text() == dedent( + expected_workspace_toml + ) + # lock should remain unchanged since package is not a table => not inheriting + assert (chdir / CargoProvider.lock_filename).read_text() == dedent(lock_content) From 5327c04e44f7797c896bf140e81da51e3184454a Mon Sep 17 00:00:00 2001 From: Juncheng Fang Date: Tue, 6 Jan 2026 09:31:29 +0800 Subject: [PATCH 5/5] refactor(cargo_provider): improve readability and maintainability --- commitizen/providers/cargo_provider.py | 123 ++++++++++++++++++------- 1 file changed, 89 insertions(+), 34 deletions(-) diff --git a/commitizen/providers/cargo_provider.py b/commitizen/providers/cargo_provider.py index 362b234f5..02ea41095 100644 --- a/commitizen/providers/cargo_provider.py +++ b/commitizen/providers/cargo_provider.py @@ -45,48 +45,72 @@ def set_version(self, version: str) -> None: self.set_lock_version(version) def set_lock_version(self, version: str) -> None: - cargo_toml = parse(self.file.read_text()) - cargo_lock = parse(self.lock_file.read_text()) - packages = cargo_lock["package"] + cargo_toml_content = parse(self.file.read_text()) + cargo_lock_content = parse(self.lock_file.read_text()) + packages = cargo_lock_content["package"] + if TYPE_CHECKING: assert isinstance(packages, AoT) - root_pkg = _table_get(cargo_toml, "package") + root_pkg = _table_get(cargo_toml_content, "package") if root_pkg is not None: name = root_pkg.get("name") if isinstance(name, str): _lock_set_versions(packages, {name}, version) - self.lock_file.write_text(dumps(cargo_lock)) + self.lock_file.write_text(dumps(cargo_lock_content)) return - ws = _table_get(cargo_toml, "workspace") or {} - members = cast("list[str]", ws.get("members", []) or []) - excludes = cast("list[str]", ws.get("exclude", []) or []) - inheriting = _workspace_inheriting_member_names(members, excludes) + ws = _table_get(cargo_toml_content, "workspace") or {} + member_globs = cast("list[str]", ws.get("members", []) or []) + exclude_globs = cast("list[str]", ws.get("exclude", []) or []) + inheriting = _workspace_inheriting_member_names(member_globs, exclude_globs) _lock_set_versions(packages, inheriting, version) - self.lock_file.write_text(dumps(cargo_lock)) + self.lock_file.write_text(dumps(cargo_lock_content)) def _table_get(doc: TOMLDocument, key: str) -> DictLike | None: - """Return a dict-like table for `key` if present, else None (type-safe for Pylance).""" + """Get a TOML table by key as a dict-like object. + + Returns: + The value at `doc[key]` cast to a dict-like table (supports `.get`) if it + exists and is table/container-like; otherwise returns None. + + Rationale: + tomlkit returns loosely-typed Container/Table objects; using a small + helper keeps call sites readable and makes type-checkers happier. + """ try: - v = doc[key] # tomlkit returns Container/Table-like; typing is loose + value = doc[key] except NonExistentKey: return None - return cast("DictLike", v) if hasattr(v, "get") else None + return cast("DictLike", value) if hasattr(value, "get") else None def _root_version_table(doc: TOMLDocument) -> DictLike: - """Prefer [workspace.package]; fallback to [package].""" - ws = _table_get(doc, "workspace") - if ws is not None: - pkg = ws.get("package") - if hasattr(pkg, "get"): - return cast("DictLike", pkg) - pkg = _table_get(doc, "package") - if pkg is None: + """Return the table that owns the "root" version field. + + This provider supports two layouts: + + 1) Workspace virtual manifests: + [workspace.package] + version = "x.y.z" + + 2) Regular crate（non-workspace root manifest）: + [package] + version = "x.y.z" + + The selected table is where `get()` reads from and `set()` writes to. + """ + workspace_table = _table_get(doc, "workspace") + if workspace_table is not None: + workspace_package_table = workspace_table.get("package") + if hasattr(workspace_package_table, "get"): + return cast("DictLike", workspace_package_table) + + package_table = _table_get(doc, "package") + if package_table is None: raise NonExistentKey("expected either [workspace.package] or [package]") - return pkg + return package_table def _is_workspace_inherited_version(v: Any) -> bool: @@ -94,19 +118,38 @@ def _is_workspace_inherited_version(v: Any) -> bool: def _iter_member_dirs( - members: Iterable[str], excludes: Iterable[str] + member_globs: Iterable[str], exclude_globs: Iterable[str] ) -> Iterable[Path]: - for pat in members: - for p in glob.glob(pat, recursive=True): - if any(fnmatch.fnmatch(p, ex) for ex in excludes): + """Yield workspace member directories matched by `member_globs`, excluding `exclude_globs`. + + Cargo workspaces define members/exclude as glob patterns (e.g. "crates/*"). + This helper expands those patterns and yields the corresponding directories + as `Path` objects, skipping any matches that satisfy an exclude glob. + + Kept as a helper to make call sites read as domain logic ("iterate member dirs") + rather than glob/filter plumbing. + """ + for member_glob in member_globs: + for match in glob.glob(member_glob, recursive=True): + if any(fnmatch.fnmatch(match, ex) for ex in exclude_globs): continue - yield Path(p) + yield Path(match) def _workspace_inheriting_member_names( members: Iterable[str], excludes: Iterable[str] ) -> set[str]: - out: set[str] = set() + """Return workspace member crate names that inherit the workspace version. + + A member is considered "inheriting" when its Cargo.toml has: + [package] + version.workspace = true + + This scans `members` globs (respecting `excludes`) and returns the set of + `[package].name` values for matching crates. Missing/invalid Cargo.toml files + are ignored. + """ + inheriting_member_names: set[str] = set() for d in _iter_member_dirs(members, excludes): cargo_file = d / "Cargo.toml" if not cargo_file.exists(): @@ -118,13 +161,25 @@ def _workspace_inheriting_member_names( if _is_workspace_inherited_version(pkgd.get("version")): name = pkgd.get("name") if isinstance(name, str): - out.add(name) - return out + inheriting_member_names.add(name) + return inheriting_member_names + + +def _lock_set_versions(packages: Any, package_names: set[str], version: str) -> None: + """Update Cargo.lock package entries in-place. + Args: + packages: `Cargo.lock` parsed TOML "package" array (AoT-like). Mutated in-place. + package_names: Set of package names whose `version` field should be updated. + version: New version string to write. -def _lock_set_versions(packages: Any, names: set[str], version: str) -> None: - if not names: + Notes: + We use `enumerate` + index assignment because tomlkit AoT entries may be + Container-like and direct mutation patterns vary; indexed assignment is + reliable for updating the underlying document. + """ + if not package_names: return - for i, p in enumerate(packages): - if getattr(p, "get", None) and p.get("name") in names: + for i, pkg_entry in enumerate(packages): + if getattr(pkg_entry, "get", None) and pkg_entry.get("name") in package_names: packages[i]["version"] = version