Skip to content

[rust-guard] Rust Guard: Dead production code + missing field name constants #3055

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Move is_forked_pull_request_with_callback out of production code

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

Problem

is_forked_pull_request_with_callback (line 275 in backend.rs) is a pub fn that lives in production code and is suppressed with #[allow(dead_code)]. It is only ever called from tests (lines 881, 887, 894 — all inside #[cfg(test)]). Because the crate compiles to a cdylib WASM binary, dead production functions inflate the binary size unnecessarily.

guards/github-guard/rust-guard/src/labels/backend.rs:274:#[allow(dead_code)]
guards/github-guard/rust-guard/src/labels/backend.rs:275:pub fn is_forked_pull_request_with_callback(...)

All three call sites are within the test module that begins at line 675.

Suggested Change

Move is_forked_pull_request_with_callback inside the #[cfg(test)] module block. Since GithubMcpCallback, Value, and SMALL_BUFFER_SIZE are already available to tests (they're production types imported at the top of the file), no other changes are needed.

Before

// In production code (line 274–275)
#[allow(dead_code)]
pub fn is_forked_pull_request_with_callback(
    callback: GithubMcpCallback,
    owner: &str,
    repo: &str,
    pull_number: &str,
) -> Option<bool> {
    // ... ~60 lines of logic ...
}

#[cfg(test)]
mod tests {
    // ... tests that call is_forked_pull_request_with_callback ...
}

After

// Production code — function removed entirely

#[cfg(test)]
mod tests {
    use super::*;  // already present

    // Move function here:
    fn is_forked_pull_request_with_callback(
        callback: GithubMcpCallback,
        owner: &str,
        repo: &str,
        pull_number: &str,
    ) -> Option<bool> {
        // ... same body, no changes needed ...
    }

    // Existing tests remain as-is
}

Why This Matters

  • Removes the #[allow(dead_code)] suppression (dead code in WASM = wasted bytes)
  • Makes intent clear: this function exists only to support testability
  • Follows the pattern already established for has_approval_label and secret_label (lines 58–59 in mod.rs use #[cfg(test)] pub use helpers::...)
  • Reduces WASM binary size (every function compiled to WASM has a cost)

Improvement 2: Add missing JSON field name constants to field_names

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

Problem

The field_names module in constants.rs was introduced to centralize JSON field string literals and prevent typos. It defines OWNER, REPO, ISSUE_NUMBER, PULL_NUMBER, SHA, MERGED_AT, and MERGED. However, several equally-common JSON field strings are missing and appear as raw literals scattered across multiple files:

Field string # non-test usages Files
"full_name" 8 helpers.rs, response_items.rs, response_paths.rs, backend.rs
"number" 6 helpers.rs
"private" 3 response_items.rs, response_paths.rs
"login" 2 helpers.rs

Suggested Change

Add the missing constants to the field_names module in constants.rs, then replace raw string literals with the constants.

Before (constants.rs, field_names module)

pub mod field_names {
    pub const OWNER: &str = "owner";
    pub const REPO: &str = "repo";
    pub const ISSUE_NUMBER: &str = "issue_number";
    pub const PULL_NUMBER: &str = "pull_number";
    pub const SHA: &str = "sha";
    pub const MERGED_AT: &str = "merged_at";
    pub const MERGED: &str = "merged";
}

After (constants.rs, field_names module)

pub mod field_names {
    pub const OWNER: &str = "owner";
    pub const REPO: &str = "repo";
    pub const ISSUE_NUMBER: &str = "issue_number";
    pub const PULL_NUMBER: &str = "pull_number";
    pub const SHA: &str = "sha";
    pub const MERGED_AT: &str = "merged_at";
    pub const MERGED: &str = "merged";
    // Commonly accessed response fields
    pub const FULL_NAME: &str = "full_name";
    pub const NUMBER: &str = "number";
    pub const PRIVATE: &str = "private";
    pub const LOGIN: &str = "login";
}

Usage sites in helpers.rs (example):

// Before
if let Some(name) = item.get("full_name").and_then(|v| v.as_str()) { ... }
let number = item.get("number").and_then(|v| v.as_u64()).unwrap_or(0);

// After
if let Some(name) = item.get(field_names::FULL_NAME).and_then(|v| v.as_str()) { ... }
let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0);

Usage sites in response_items.rs and response_paths.rs (example):

// Before
let is_private = get_bool_or(item, "private", false);
let full_name = get_str_or(item, "full_name", "unknown");

// After
let is_private = get_bool_or(item, field_names::PRIVATE, false);
let full_name = get_str_or(item, field_names::FULL_NAME, "unknown");

Note: field_names is already imported in helpers.rs and tool_rules.rs. The import in response_items.rs and response_paths.rs must also include field_names.

Why This Matters

  • Prevents silent typos in field names (e.g. "fullname" vs "full_name") that produce silent None at runtime
  • Makes grep/refactor easier — one constant to update vs. 8+ scattered literals
  • Consistent with the existing field_names convention already established for OWNER, REPO, etc.

Codebase Health Summary

  • Total Rust files: 9 (+ labels/README.md)
  • Total lines: 10,848
  • Areas analyzed: lib.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/helpers.rs, labels/constants.rs, labels/tool_rules.rs, labels/response_items.rs, labels/response_paths.rs
  • Areas with no further improvements: (none yet — first run)

Generated by Rust Guard Improver • Run: §23893763871

Generated by Rust Guard Improver ·

  • expires on Apr 9, 2026, 9:34 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