Skip to content

[rust-guard] Rust Guard: Deduplicate identical loop bodies in backend.rsΒ #5527

@github-actions

Description

@github-actions

πŸ¦€ Rust Guard Improvement Report

Improvement 1: Extract find_org_in_items helper in owner_is_org_from_items

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

Problem

owner_is_org_from_items (line ~1336) contains two loops with an identical body β€” the only difference is how the items slice is obtained:

// search_repositories shape: { items: [...] }
if let Some(items) = value.get("items").and_then(|v| v.as_array()) {
    for item in items {
        let item_repo_id = repo_id_from_repo_object(item);
        if item_repo_id
            .as_deref()
            .map(|id| id.eq_ignore_ascii_case(repo_id))
            .unwrap_or(false)
        {
            return owner_type_from_repo_object(item);
        }
    }
}

// Plain array response
if let Some(items) = value.as_array() {
    for item in items {                          // ← identical body
        let item_repo_id = repo_id_from_repo_object(item);
        if item_repo_id
            .as_deref()
            .map(|id| id.eq_ignore_ascii_case(repo_id))
            .unwrap_or(false)
        {
            return owner_type_from_repo_object(item);
        }
    }
}

Suggested Change

Extract a private helper find_org_in_items:

Before

fn owner_is_org_from_items(value: &Value, repo_id: &str) -> Option<bool> {
    if let Some(items) = value.get("items").and_then(|v| v.as_array()) {
        for item in items {
            let item_repo_id = repo_id_from_repo_object(item);
            if item_repo_id
                .as_deref()
                .map(|id| id.eq_ignore_ascii_case(repo_id))
                .unwrap_or(false)
            {
                return owner_type_from_repo_object(item);
            }
        }
    }

    if let Some(items) = value.as_array() {
        for item in items {
            let item_repo_id = repo_id_from_repo_object(item);
            if item_repo_id
                .as_deref()
                .map(|id| id.eq_ignore_ascii_case(repo_id))
                .unwrap_or(false)
            {
                return owner_type_from_repo_object(item);
            }
        }
    }

    // ... single-object fallback ...
}

After

fn find_org_in_items(items: &[Value], repo_id: &str) -> Option<bool> {
    items.iter().find_map(|item| {
        let item_repo_id = repo_id_from_repo_object(item)?;
        if item_repo_id.eq_ignore_ascii_case(repo_id) {
            owner_type_from_repo_object(item)
        } else {
            None
        }
    })
}

fn owner_is_org_from_items(value: &Value, repo_id: &str) -> Option<bool> {
    if let Some(items) = value.get("items").and_then(|v| v.as_array()) {
        if let Some(result) = find_org_in_items(items, repo_id) {
            return Some(result);
        }
    }

    if let Some(items) = value.as_array() {
        if let Some(result) = find_org_in_items(items, repo_id) {
            return Some(result);
        }
    }

    // ... single-object fallback ...
}

Why This Matters

Eliminates ~12 duplicated lines. The helper is also independently testable, making it easier to add regression tests for the repo-ID matching logic without the outer response-shape dispatch getting in the way.


Improvement 2: Merge duplicate field lookups in repo_id_from_repo_object

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

Problem

repo_id_from_repo_object (line ~1389) has two consecutive blocks with identical structure β€” only the JSON field name differs ("full_name" vs "fullName"):

if let Some(full_name) = item.get("full_name").and_then(|v| v.as_str()) {
    if !full_name.is_empty() {
        return Some(full_name.to_string());
    }
}

if let Some(full_name) = item.get("fullName").and_then(|v| v.as_str()) {
    if !full_name.is_empty() {
        return Some(full_name.to_string());
    }
}

Suggested Change

Replace with a single for-loop over both field names:

Before

fn repo_id_from_repo_object(item: &Value) -> Option<String> {
    if let Some(full_name) = item.get("full_name").and_then(|v| v.as_str()) {
        if !full_name.is_empty() {
            return Some(full_name.to_string());
        }
    }

    if let Some(full_name) = item.get("fullName").and_then(|v| v.as_str()) {
        if !full_name.is_empty() {
            return Some(full_name.to_string());
        }
    }

    // ... remaining fallback logic ...
}

After

fn repo_id_from_repo_object(item: &Value) -> Option<String> {
    for field in &["full_name", "fullName"] {
        if let Some(full_name) = item.get(field).and_then(|v| v.as_str()) {
            if !full_name.is_empty() {
                return Some(full_name.to_string());
            }
        }
    }

    // ... remaining fallback logic (unchanged) ...
}

Why This Matters

Reduces 8 lines to 5 and makes it trivial to add a third camelCase alias in the future (e.g. "repository_full_name") without another copy-paste block. The logic is unchanged β€” field priority is preserved by loop ordering.


Codebase Health Summary

  • Total Rust files: 10
  • Total lines: ~13,689
  • Areas analyzed: all modules
  • Areas with no further improvements: tools.rs (BLOCKED_TOOLS, test coverage, write/delete predicates all addressed), labels/constants.rs (scope names and order constants added), lib.rs (difc_mode const, &'static str fields, parse_integrity/scope_string tests, blocked-tool logic all addressed)

Generated by Rust Guard Improver β€’ Run: Β§25727264919

Generated by Rust Guard Improver Β· ● 1.4M Β· β—·

  • expires on May 19, 2026, 10:04 AM UTC

Metadata

Metadata

Assignees

No one assigned

    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