🦀 Rust Guard Improvement Report
Improvement 1: Remove Two Stale #[allow(dead_code)] Annotations in backend.rs
Category: Dead Code
File(s): guards/github-guard/rust-guard/src/labels/backend.rs
Effort: Small (< 5 min)
Risk: Low
Problem
Two #[allow(dead_code)] annotations in backend.rs suppress compiler warnings for functions that are actually called in production. The annotations are stale — likely left over from an earlier time when these functions were not yet wired up.
is_repo_private (line 56): Called by tool_rules.rs (×2), response_items.rs (×3), helpers.rs (×2), and response_paths.rs (×3) — 10 production call sites.
get_issue_author_association (line 405): Called by helpers.rs:1119 to enrich issue integrity when author_association is absent from list responses.
These stale #[allow(dead_code)] attributes are misleading — they suggest the functions are unused, making a reviewer question whether they should be removed.
Suggested Change
Remove the two #[allow(dead_code)] lines only.
Before
/// Check whether a repository is private using the backend MCP server.
// ...
#[allow(dead_code)] // ← stale: called in 10 places
pub fn is_repo_private(owner: &str, repo: &str) -> Option(bool) {
is_repo_private_with_callback(crate::invoke_backend, owner, repo)
}
// ...
#[allow(dead_code)] // ← stale: called in helpers.rs:1119
pub fn get_issue_author_association(owner: &str, repo: &str, issue_number: &str) -> Option(String) {
get_issue_author_association_with_callback(crate::invoke_backend, owner, repo, issue_number)
}
After
/// Check whether a repository is private using the backend MCP server.
// ...
pub fn is_repo_private(owner: &str, repo: &str) -> Option(bool) {
is_repo_private_with_callback(crate::invoke_backend, owner, repo)
}
// ...
pub fn get_issue_author_association(owner: &str, repo: &str, issue_number: &str) -> Option(String) {
get_issue_author_association_with_callback(crate::invoke_backend, owner, repo, issue_number)
}
Why This Matters
Stale #[allow(dead_code)] annotations create noise that desensitises reviewers to real dead code warnings. They make it harder to identify functions that are genuinely unused (like is_owner or is_forked_pull_request_with_callback below). Removing them lets the compiler enforce correctness and makes code reviews easier.
Improvement 2: Remove is_forked_pull_request_with_callback (Superseded by get_pull_request_facts_with_callback)
Category: Dead Code
File(s): guards/github-guard/rust-guard/src/labels/backend.rs
Effort: Medium (15–20 min)
Risk: Low
Problem
is_forked_pull_request_with_callback (lines 215–282) is a standalone function that checks whether a PR is from a fork by calling pull_request_read. It:
- Has
#[allow(dead_code)] — it has zero production call sites.
- Is only called from 3 unit tests within the same file (lines 727–744).
- Duplicates a subset of the logic already inside
get_pull_request_facts_with_callback (lines 285–357), which also extracts is_forked (stored as PullRequestFacts.is_forked) alongside author_association, author_login, and is_merged.
The _with_callback pattern exists to allow injection of a mock backend in tests. get_pull_request_facts_with_callback already follows this same pattern and its result includes is_forked. There is no production wrapper (is_forked_pull_request) that calls it — making it a dead-end function.
Suggested Change
- Delete
is_forked_pull_request_with_callback (lines 215–282).
- Convert the 3 tests that call it into tests on
get_pull_request_facts_with_callback, verifying facts.is_forked — this preserves the test coverage.
Before
/// Determine whether a pull request is from a fork.
// ...
#[allow(dead_code)]
pub fn is_forked_pull_request_with_callback(
callback: GithubMcpCallback,
owner: &str,
repo: &str,
pull_number: &str,
) -> Option(bool) {
// ... ~60 lines fetching PR and extracting base/head full_name ...
}
// In tests:
#[test]
fn test_is_forked_pull_request_direct() {
let result =
is_forked_pull_request_with_callback(direct_pr_callback, "owner", "repo", "123");
assert_eq!(result, Some(false));
}
#[test]
fn test_is_forked_pull_request_forked() {
let result = is_forked_pull_request_with_callback(fork_pr_callback, "owner", "repo", "123");
assert_eq!(result, Some(true));
}
#[test]
fn test_is_forked_pull_request_wrapped_response() {
let result =
is_forked_pull_request_with_callback(wrapped_fork_pr_callback, "owner", "repo", "123");
assert_eq!(result, Some(true));
}
After
// is_forked_pull_request_with_callback deleted entirely.
// Tests migrated to get_pull_request_facts_with_callback:
#[test]
fn test_pull_request_facts_direct_pr_not_forked() {
let facts = get_pull_request_facts_with_callback(direct_pr_callback, "owner", "repo", "123");
assert_eq!(facts.map(|f| f.is_forked), Some(Some(false)));
}
#[test]
fn test_pull_request_facts_forked_pr() {
let facts = get_pull_request_facts_with_callback(fork_pr_callback, "owner", "repo", "123");
assert_eq!(facts.map(|f| f.is_forked), Some(Some(true)));
}
#[test]
fn test_pull_request_facts_wrapped_response_forked() {
let facts =
get_pull_request_facts_with_callback(wrapped_fork_pr_callback, "owner", "repo", "123");
assert_eq!(facts.map(|f| f.is_forked), Some(Some(true)));
}
Why This Matters
- Removes ~70 lines of dead code (function + tests), reducing maintenance surface.
- Migrating the fork-detection tests to
get_pull_request_facts_with_callback gives that function — the one that's actually used in production — its first direct unit tests, which is a net coverage gain.
- Follows the single-function principle: the PR lookup is done once (
get_pull_request_facts), not via two independent functions that make the same network call.
Codebase Health Summary
- Total Rust files: 10
- Total lines: 8,510
- Areas analyzed:
backend.rs, helpers.rs, tool_rules.rs, response_items.rs, response_paths.rs, mod.rs, constants.rs, lib.rs, permissions.rs, tools.rs
- Areas with no further improvements:
constants.rs, response_items.rs, response_paths.rs
Generated by Rust Guard Improver • Run: §23430373144
Generated by Rust Guard Improver · ◷
🦀 Rust Guard Improvement Report
Improvement 1: Remove Two Stale
#[allow(dead_code)]Annotations inbackend.rsCategory: Dead Code
File(s):
guards/github-guard/rust-guard/src/labels/backend.rsEffort: Small (< 5 min)
Risk: Low
Problem
Two
#[allow(dead_code)]annotations inbackend.rssuppress compiler warnings for functions that are actually called in production. The annotations are stale — likely left over from an earlier time when these functions were not yet wired up.is_repo_private(line 56): Called bytool_rules.rs(×2),response_items.rs(×3),helpers.rs(×2), andresponse_paths.rs(×3) — 10 production call sites.get_issue_author_association(line 405): Called byhelpers.rs:1119to enrich issue integrity whenauthor_associationis absent from list responses.These stale
#[allow(dead_code)]attributes are misleading — they suggest the functions are unused, making a reviewer question whether they should be removed.Suggested Change
Remove the two
#[allow(dead_code)]lines only.Before
After
Why This Matters
Stale
#[allow(dead_code)]annotations create noise that desensitises reviewers to real dead code warnings. They make it harder to identify functions that are genuinely unused (likeis_owneroris_forked_pull_request_with_callbackbelow). Removing them lets the compiler enforce correctness and makes code reviews easier.Improvement 2: Remove
is_forked_pull_request_with_callback(Superseded byget_pull_request_facts_with_callback)Category: Dead Code
File(s):
guards/github-guard/rust-guard/src/labels/backend.rsEffort: Medium (15–20 min)
Risk: Low
Problem
is_forked_pull_request_with_callback(lines 215–282) is a standalone function that checks whether a PR is from a fork by callingpull_request_read. It:#[allow(dead_code)]— it has zero production call sites.get_pull_request_facts_with_callback(lines 285–357), which also extractsis_forked(stored asPullRequestFacts.is_forked) alongsideauthor_association,author_login, andis_merged.The
_with_callbackpattern exists to allow injection of a mock backend in tests.get_pull_request_facts_with_callbackalready follows this same pattern and its result includesis_forked. There is no production wrapper (is_forked_pull_request) that calls it — making it a dead-end function.Suggested Change
is_forked_pull_request_with_callback(lines 215–282).get_pull_request_facts_with_callback, verifyingfacts.is_forked— this preserves the test coverage.Before
After
Why This Matters
get_pull_request_facts_with_callbackgives that function — the one that's actually used in production — its first direct unit tests, which is a net coverage gain.get_pull_request_facts), not via two independent functions that make the same network call.Codebase Health Summary
backend.rs,helpers.rs,tool_rules.rs,response_items.rs,response_paths.rs,mod.rs,constants.rs,lib.rs,permissions.rs,tools.rsconstants.rs,response_items.rs,response_paths.rsGenerated by Rust Guard Improver • Run: §23430373144