Skip to content

[rust-guard] Rust Guard: Extract shared first_matching_label helper and add label-check tests #8031

Description

@github-actions

Improvement 1: Extract first_matching_label from duplicated approval/refusal functions

Category: Code Duplication
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low

Problem

first_matching_approval_label (line ~391) and first_matching_refusal_label (line ~411) are near-identical private functions. The only differences are which ctx field they access (approval_labels vs refusal_labels) and the variable name in the closure (al vs rl):

fn first_matching_approval_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    if ctx.approval_labels.is_empty() {
        return None;
    }
    let label_names = extract_github_label_names(item);
    label_names.into_iter().find(|name| {
        ctx.approval_labels
            .iter()
            .any(|al| al.eq_ignore_ascii_case(name))
    })
}

fn first_matching_refusal_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    if ctx.refusal_labels.is_empty() {
        return None;
    }
    let label_names = extract_github_label_names(item);
    label_names.into_iter().find(|name| {
        ctx.refusal_labels
            .iter()
            .any(|rl| rl.eq_ignore_ascii_case(name))
    })
}

Suggested Change

Extract a shared private helper first_matching_label that takes the list directly, then delegate both functions to it:

Before

fn first_matching_approval_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    if ctx.approval_labels.is_empty() {
        return None;
    }
    let label_names = extract_github_label_names(item);
    label_names.into_iter().find(|name| {
        ctx.approval_labels
            .iter()
            .any(|al| al.eq_ignore_ascii_case(name))
    })
}

fn first_matching_refusal_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    if ctx.refusal_labels.is_empty() {
        return None;
    }
    let label_names = extract_github_label_names(item);
    label_names.into_iter().find(|name| {
        ctx.refusal_labels
            .iter()
            .any(|rl| rl.eq_ignore_ascii_case(name))
    })
}

After

/// Return the first item label that matches any entry in `label_list` (case-insensitive).
/// Returns `None` immediately when `label_list` is empty (feature disabled).
fn first_matching_label<'a>(item: &'a Value, label_list: &[String]) -> Option<&'a str> {
    if label_list.is_empty() {
        return None;
    }
    extract_github_label_names(item)
        .into_iter()
        .find(|name| label_list.iter().any(|l| l.eq_ignore_ascii_case(name)))
}

fn first_matching_approval_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    first_matching_label(item, &ctx.approval_labels)
}

fn first_matching_refusal_label<'a>(item: &'a Value, ctx: &PolicyContext) -> Option<&'a str> {
    first_matching_label(item, &ctx.refusal_labels)
}

Why This Matters

  • Eliminates 12 lines of copied logic — any future change to matching semantics only needs to happen once.
  • Makes the symmetry between approval and refusal label handling visually obvious.
  • first_matching_label is also reusable if additional label-list fields are added to PolicyContext.

Improvement 2: Add unit tests for label-checking helper functions

Category: Test Coverage
File(s): guards/github-guard/rust-guard/src/labels/helpers.rs
Effort: Small (< 15 min)
Risk: Low

Problem

Four pub functions are exposed in #[cfg(test)] mode and used by apply_post_integrity_adjustments, but have zero direct unit tests:

  • has_approval_label (~line 385) — wraps first_matching_approval_label
  • has_refusal_label (~line 406) — wraps first_matching_refusal_label
  • has_promotion_label (~line 492) — wraps item_has_config_label for promotion_label
  • has_demotion_label (~line 498) — wraps item_has_config_label for demotion_label

The case-insensitive matching, empty-list early-return, and missing-labels-field behaviors are untested in isolation.

Suggested Change

Add the following test block to the tests module in helpers.rs:

Before

(no tests for these functions)

After

// =========================================================================
// Tests for label-checking helpers
// =========================================================================

#[test]
fn test_has_approval_label_matches_case_insensitively() {
    let ctx = PolicyContext {
        approval_labels: vec!["approved".to_string()],
        ..Default::default()
    };
    let item = serde_json::json!({"labels": [{"name": "APPROVED"}]});
    assert!(has_approval_label(&item, &ctx));
}

#[test]
fn test_has_approval_label_no_match_returns_false() {
    let ctx = PolicyContext {
        approval_labels: vec!["approved".to_string()],
        ..Default::default()
    };
    let item = serde_json::json!({"labels": [{"name": "other-label"}]});
    assert!(!has_approval_label(&item, &ctx));
}

#[test]
fn test_has_approval_label_empty_list_returns_false() {
    let ctx = PolicyContext::default(); // approval_labels is empty
    let item = serde_json::json!({"labels": [{"name": "approved"}]});
    assert!(!has_approval_label(&item, &ctx));
}

#[test]
fn test_has_approval_label_no_labels_field_returns_false() {
    let ctx = PolicyContext {
        approval_labels: vec!["approved".to_string()],
        ..Default::default()
    };
    let item = serde_json::json!({"number": 1, "title": "no labels field"});
    assert!(!has_approval_label(&item, &ctx));
}

#[test]
fn test_has_refusal_label_matches_case_insensitively() {
    let ctx = PolicyContext {
        refusal_labels: vec!["spam".to_string()],
        ..Default::default()
    };
    let item = serde_json::json!({"labels": [{"name": "SPAM"}]});
    assert!(has_refusal_label(&item, &ctx));
}

#[test]
fn test_has_refusal_label_empty_list_returns_false() {
    let ctx = PolicyContext::default();
    let item = serde_json::json!({"labels": [{"name": "spam"}]});
    assert!(!has_refusal_label(&item, &ctx));
}

#[test]
fn test_has_promotion_label_matches_case_insensitively() {
    let ctx = PolicyContext {
        promotion_label: "trusted".to_string(),
        ..Default::default()
    };
    let item = serde_json::json!({"labels": [{"name": "TRUSTED"}]});
    assert!(has_promotion_label(&item, &ctx));
}

#[test]
fn test_has_promotion_label_empty_string_returns_false() {
    let ctx = PolicyContext::default(); // promotion_label is ""
    let item = serde_json::json!({"labels": [{"name": "trusted"}]});
    assert!(!has_promotion_label(&item, &ctx));
}

#[test]
fn test_has_demotion_label_matches_case_insensitively() {
    let ctx = PolicyContext {
        demotion_label: "needs-review".to_string(),
        ..Default::default()
    };
    let item = serde_json::json!({"labels": [{"name": "NEEDS-REVIEW"}]});
    assert!(has_demotion_label(&item, &ctx));
}

#[test]
fn test_has_demotion_label_empty_string_returns_false() {
    let ctx = PolicyContext::default(); // demotion_label is ""
    let item = serde_json::json!({"labels": [{"name": "needs-review"}]});
    assert!(!has_demotion_label(&item, &ctx));
}

Why This Matters

  • Pins the case-insensitive matching behavior so it can't silently regress.
  • Explicitly documents the "empty list = feature disabled" contract for all four helpers.
  • Follows the pattern of other recently-added unit tests (commit_integrity, is_default_branch_ref, etc.).

Codebase Health Summary

  • Total Rust files: 9 (lib.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/constants.rs, labels/helpers.rs, labels/response_items.rs, labels/response_paths.rs, labels/tool_rules.rs)
  • Total lines: ~17,321
  • Areas analyzed: All source files
  • Areas with no further improvements: labels/constants.rs, labels/mod.rs (thin wrappers / re-exports)

Generated by Rust Guard Improver • Run: §28091085981

Generated by Rust Guard Improver · 499.5 AIC · ⊞ 34.1K ·

  • expires on Jul 1, 2026, 10:16 AM UTC

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions