Simplify redundant sensitive argument detection functions in runner.rs

[coderabbitai]

Problem

In src/runner.rs, the functions contains_sensitive_keyword and is_sensitive_arg create unnecessary indirection. The is_sensitive_arg function simply delegates to contains_sensitive_keyword without adding any value or functionality.

Proposed Solution

Combine these functions into a single is_sensitive_arg function that directly contains the logic for detecting sensitive keywords ("password", "token", "secret") in command-line arguments.

This will:

• Reduce code complexity
• Eliminate unnecessary function call overhead
• Improve code readability and maintainability

Context

This issue was identified during code review of PR #45 which adds verbose logging functionality.

References:

• Pull Request: Add verbose logging via tracing #45
• Comment: Add verbose logging via tracing #45 (comment)
• Issue: Simplify redundant sensitive argument detection functions in runner.rs #51

Requested by: @leynos

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.

Once a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll examine the current state of the codebase to assess whether this issue is still relevant.

---

🏁 Script executed:

#!/bin/bash
# First, let's check if the runner.rs file exists and examine its contents
if [ -f "src/runner.rs" ]; then
    echo "=== Contents of src/runner.rs ==="
    cat src/runner.rs
else
    echo "runner.rs not found in src/ directory"
fi

Length of output: 14366

---

Let me examine the specific functions mentioned in the issue to confirm the current state and assess the criticality.

Now let me check the usage of these functions to understand the impact:

Now I'll apply the appropriate label and provide my assessment:

@leynos The issue is still relevant. After examining the current codebase, both functions contains_sensitive_keyword and is_sensitive_arg exist with the exact redundancy described in the original issue:

• contains_sensitive_keyword (Line 240-244): Contains the actual logic for detecting sensitive keywords
• is_sensitive_arg (Line 251-253): Simply delegates to contains_sensitive_keyword without adding any value

Criticality Assessment: LOW

This issue has minimal impact because:

• No user impact: This is purely an internal code organization issue
• No functional bugs: The code works correctly as intended
• Minimal performance impact: The extra function call overhead is negligible
• Small blast radius: Limited to internal helper functions in runner.rs

Impact of Required Changes:

• Code changes: Simple refactoring - merge the logic from contains_sensitive_keyword into is_sensitive_arg and remove the redundant function
• Testing: Update doctest examples that reference the removed function
• Risk: Very low - straightforward refactoring with no behavioral changes
• Effort: Minimal - approximately 10-15 lines of code changes

The fix would eliminate unnecessary indirection and slightly improve code readability, but it's a nice-to-have rather than a critical issue.

🐰💭 A small improvement that tidies up the code garden!

---

🏁 Script executed:

#!/bin/bash
# Let's look specifically at the functions mentioned in the issue
echo "=== Examining the sensitive argument detection functions ==="
rg -n -A 10 -B 2 "fn contains_sensitive_keyword|fn is_sensitive_arg" src/runner.rs

Length of output: 1860

---

🏁 Script executed:

#!/bin/bash
# Check where these functions are used in the codebase
echo "=== Usage of contains_sensitive_keyword ==="
rg -n "contains_sensitive_keyword" src/runner.rs

echo -e "\n=== Usage of is_sensitive_arg ==="
rg -n "is_sensitive_arg" src/runner.rs

Length of output: 1161

---

🏁 Script executed:

#!/bin/bash
# Apply the "low" criticality label to the issue
gh issue edit 51 --add-label "low"

Length of output: 78

---

@leynos The issue is still relevant. After examining the current codebase, both functions contains_sensitive_keyword and is_sensitive_arg exist with the exact redundancy described in the original issue:

• contains_sensitive_keyword (Line 297-300): Contains the actual logic for detecting sensitive keywords
• is_sensitive_arg (Line 310-312): Simply delegates to contains_sensitive_keyword without adding any value

Criticality Assessment: LOW

This issue has minimal impact because:

• No user impact: This is purely an internal code organization issue
• No functional bugs: The code works correctly as intended
• Minimal performance impact: The extra function call overhead is negligible
• Small blast radius: Limited to internal helper functions in runner.rs

Impact of Required Changes:

• Code changes: Simple refactoring - merge the logic from contains_sensitive_keyword into is_sensitive_arg and remove the redundant function
• Testing: Update doctest examples that reference the removed function (Lines 293-295) and the doc module wrapper (Lines 34-36)
• Risk: Very low - straightforward refactoring with no behavioral changes
• Effort: Minimal - approximately 10-15 lines of code changes

The fix would eliminate unnecessary indirection and slightly improve code readability, but it's a nice-to-have rather than a critical issue. The low label has been applied to the issue.

🐰💭 A small improvement that tidies up the code garden!