Skip to content

[rust-guard] Rust Guard: Remove dead parameters from issue_integrity + fix heap-allocating string comparison #2148

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Remove Dead Parameters from issue_integrity

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

Problem

issue_integrity declares two parameters, _owner: &str and _repo: &str, that are never read inside its body — the leading underscore signals this explicitly. Despite being unused, both call sites are forced to compute and pass values for them:

  • response_items.rs line 208: repo_owner is computed solely for this call:
    let repo_owner = repo_full_name.split('/').next().unwrap_or("");
  • response_paths.rs line 225: owner is computed solely for this call:
    let owner = repo_for_labels.split('/').next().unwrap_or("");

Neither variable is used for anything else. The dead params add noise to the signature and force unnecessary string work at each call site.

Suggested Change

Remove _owner and _repo from the function signature, and remove the now-redundant computations from both call sites.

Before

// labels/helpers.rs (line 812)
pub fn issue_integrity(
    item: &Value,
    repo_full_name: &str,
    _owner: &str,   // ← never read
    _repo: &str,    // ← never read
    repo_private: bool,
    ctx: &PolicyContext,
) -> Vec(String) {

// response_items.rs (line 208)
let repo_owner = repo_full_name.split('/').next().unwrap_or("");  // ← only for the call below
let integrity = issue_integrity(
    item,
    &repo_full_name,
    repo_owner,    // ← thrown away
    &arg_repo,     // ← thrown away
    repo_private,
    ctx,
);

// response_paths.rs (line 225)
let owner = repo_for_labels.split('/').next().unwrap_or("");  // ← only for the call below
let integrity = issue_integrity(
    item,
    repo_for_labels,
    owner,     // ← thrown away
    &arg_repo, // ← thrown away
    item_repo_private,
    ctx,
);

After

// labels/helpers.rs
pub fn issue_integrity(
    item: &Value,
    repo_full_name: &str,
    repo_private: bool,
    ctx: &PolicyContext,
) -> Vec(String) {

// response_items.rs
let integrity = issue_integrity(
    item,
    &repo_full_name,
    repo_private,
    ctx,
);

// response_paths.rs
let integrity = issue_integrity(
    item,
    repo_for_labels,
    item_repo_private,
    ctx,
);

Why This Matters

Removes misleading API surface — the parameter names with _ prefix are a code smell signalling the function's contract doesn't match its callers. Cleaning this up also eliminates small unnecessary string splits at every issue in a list_issues / search_issues response.


Improvement 2: Replace Heap-Allocating Value::String(…) Comparison

Category: Performance (WASM-specific)
File(s): guards/github-guard/rust-guard/src/labels/tool_rules.rs
Effort: Small (< 15 min)
Risk: Low

Problem

In the actions_get branch (line ~279), a Value::String(String::new(…)) is constructed and immediately discarded just to compare a JSON field against a string literal:

if tool_name == "actions_get"
    && tool_args.get("method")
        == Some(&Value::String("download_workflow_run_artifact".to_string()))

"download_workflow_run_artifact".to_string() allocates a heap String on every actions_get call — even when the condition is false. In a WASM environment where allocations are relatively expensive and heap fragmentation matters, this is wasteful.

Suggested Change

Use the zero-allocation idiom: extract the string with .as_str() and compare against a &str literal.

Before

// labels/tool_rules.rs (~line 279)
if tool_name == "actions_get"
    && tool_args.get("method")
        == Some(&Value::String("download_workflow_run_artifact".to_string()))
{
    // Artifacts may contain secrets
    secrecy = secret_label();
}

After

if tool_name == "actions_get"
    && tool_args
        .get("method")
        .and_then(|v| v.as_str())
        == Some("download_workflow_run_artifact")
{
    // Artifacts may contain secrets
    secrecy = secret_label();
}

Why This Matters

The == Some(&Value::String(...)) pattern is an easy footgun in serde_json code: it silently forces a heap allocation on every comparison. The .and_then(|v| v.as_str()) == Some("...") idiom is idiomatic, allocation-free, and more readable. Consistent use of this pattern across the codebase will reduce GC pressure in the WASM runtime.


Codebase Health Summary

  • Total Rust files: 10
  • Total lines: ~6,231
  • Areas analyzed: lib.rs, permissions.rs, tools.rs, labels/mod.rs, labels/backend.rs, labels/helpers.rs, labels/response_items.rs, labels/response_paths.rs, labels/tool_rules.rs, labels/constants.rs
  • Areas with no further improvements identified: labels/constants.rs, labels/backend.rs (per previous runs)

Generated by Rust Guard Improver • Run: §23288089490

References:

Generated by Rust Guard Improver ·

  • expires on Mar 26, 2026, 9:27 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