rust-guard: add MinIntegrity::from_policy_str to unify integrity parsers + extract_repo_from_item tests#7386
Conversation
…from_item tests - Add MinIntegrity::from_policy_str(level: &str) -> Option<Self> to unify the duplicate case-insensitive if/else chains in integrity_level_rank and integrity_for_level - Refactor integrity_level_rank to use match on from_policy_str - Refactor integrity_for_level to use match on from_policy_str - Add unit tests for from_policy_str (known levels, case insensitivity, whitespace trimming, unknown input, roundtrip with as_str) - Add unit tests for extract_repo_from_item covering all five lookup strategies plus priority ordering and empty fallback Closes #7369
There was a problem hiding this comment.
Pull request overview
This PR reduces duplication in the rust-guard policy integrity parsing by centralizing string-to-MinIntegrity conversion, and it adds missing unit test coverage for extract_repo_from_item, which influences repo attribution for labeled items.
Changes:
- Added
MinIntegrity::from_policy_str(&str) -> Option<MinIntegrity>to provide a single canonical, trimmed, case-insensitive parser for policy integrity tokens. - Refactored
integrity_level_rankandintegrity_for_levelto delegate parsing toMinIntegrity::from_policy_str, keeping default behaviors intact. - Added unit tests covering
MinIntegrity::from_policy_strparsing behavior andextract_repo_from_item’s primary lookup strategies and precedence.
Show a summary per file
| File | Description |
|---|---|
| guards/github-guard/rust-guard/src/labels/helpers.rs | Centralizes integrity parsing in MinIntegrity::from_policy_str, updates rank/label helpers to use it, and adds unit tests for integrity parsing + repo extraction. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
Fixed in 7a0c1ef. The main branch already had |
integrity_level_rankandintegrity_for_levelboth contained an identical 4-way case-insensitive if/else chain parsing integrity level strings. Adding a new level required updating both in sync with no compiler enforcement.Changes
MinIntegrity::from_policy_str(level: &str) -> Option<Self>— adds the reverse direction ofas_str(). Trims whitespace, case-insensitive match, returnsNonefor unrecognised values.integrity_level_rank/integrity_for_levelrefactored — both now delegate tomatch MinIntegrity::from_policy_str(level). The parsing logic lives in exactly one place; exhaustive match means the compiler will flag any missed case site when a new variant is added.MinIntegrity::from_policy_strunit tests — known levels, case insensitivity, whitespace trimming, unknown input (None), and roundtrip withas_str().extract_repo_from_itemunit tests — previously untested despite driving repo attribution for every labeled item. New tests cover all five lookup strategies (directfull_name,repository.full_name,base.repo.full_name,head.repo.full_name, URL parsing fallback) plus priority ordering and the empty fallback.