Uh oh!
There was an error while loading. Please reload this page.
fix(providers/cargo): don't crash on workspace member with fixed version - #2002
Conversation
When iterating workspace members in `CargoProvider.set_lock_version`, the code subscripted `package["version"]["workspace"]` inside a try/except block that only caught `NonExistentKey`. If a member's `Cargo.toml` declared a hardcoded version (`version = "x.y.z"`) instead of `version.workspace = true`, `package["version"]` was a tomlkit `String` and the subscript raised `TypeError: string indices must be integers, not 'str'`, which was not caught and propagated as a crash. Replace the exception-driven check with an `isinstance` type guard so the inheritance lookup only runs when `version` is an actual table. Fixes#2001 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Commitizen bump previewMerging this PR will produce the following bump: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## master #2002 +/- ##
==========================================
- Coverage 98.24% 98.23% -0.01%
==========================================
Files 61 61 Lines 2785 2782 -3 ==========================================
- Hits 2736 2733 -3
Misses 49 49 ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a Cargo provider crash when a workspace member pins its own package version instead of inheriting workspace.package.version.
Changes:
- Replaces exception-driven nested access with a type guard before checking
version.workspace. - Adds a regression test for workspace members using
version = "x.y.z".
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
commitizen/providers/cargo_provider.py | Safely detects members inheriting workspace version before updating Cargo.lock. |
tests/providers/test_cargo_provider.py | Adds regression coverage for fixed-version workspace members. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Description
Fix a
TypeError: string indices must be integers, not 'str'crash inCargoProviderwhen bumping a Cargo workspace whose members include a crate with a hardcoded version.While iterating workspace members in
CargoProvider.set_lock_version, the code subscriptedpackage["version"]["workspace"]inside atry/exceptblock that only caughtNonExistentKey. If a member'sCargo.tomldeclared a hardcoded version (version = "x.y.z") instead ofversion.workspace = true,package["version"]was a tomlkitStringand the subscript raisedTypeError, which the existing handler did not catch.The fix replaces the exception-driven check with an
isinstancetype guard so the inheritance lookup only runs whenversionis an actual table.Checklist
Was generative AI tooling used to co-author this PR?
Generated-by: GitHub Copilot CLI following the guidelines
Code Changes
uv run poe alllocally to ensure this change passes linter check and testsDocumentation Changes
No documentation changes.
Expected Behavior
cz bumpagainst a Cargo workspace whose members include a crate with a hardcodedversion = "x.y.z"(real Cargo syntax for opting out of the workspace-inherited version) succeeds:[workspace.package]is bumped,version.workspace = trueget their lock entries bumped,TypeErroris raised.Steps to Test This Pull Request
A regression test is added in
tests/providers/test_cargo_provider.py:test_cargo_provider_workspace_member_with_fixed_version.Manual repro (also covered by the new test):
Create a workspace
Cargo.toml:Add a member
pinned/Cargo.toml:Create a matching
Cargo.lockand configure commitizen withversion_provider = "cargo".Run
cz bump --yes. Before this fix, it crashes withTypeError: string indices must be integers, not 'str'. With the fix, the workspace version is bumped and the pinned member's lock entry is preserved.Verified locally:
uv run poe format-- cleanuv run poe lint-- ruff + mypy cleanuv run pytest tests/providers/test_cargo_provider.py-- 7 passed (including the new regression test)uv run prek run --files commitizen/providers/cargo_provider.py tests/providers/test_cargo_provider.py-- cleanThe new test was also confirmed to fail on the unpatched code with the exact
TypeErrorfrom the bug report.Additional Context