refactor(objc): use if let Some in for-loop instead of ?

[carlos-alm]

Summary

• Replaces let child = struct_decl.child(i)?; inside the for i in 0..struct_decl.child_count() loop in extract_property_name with if let Some(child) = struct_decl.child(i).
• The ? operator short-circuits the entire enclosing function on the first None, not just the current loop iteration. In practice node.child(i) returns Some for every i < child_count(), so this is harmless today, but if let Some is more defensively correct and matches the pattern already used in the Verilog extractor after refactor(verilog): use if let Some in for-loops instead of ? #1155.
• Completes the broader sweep tracked in follow-up: use if let Some(child) = ... over node.child(i)? in Verilog extractor for-loops #1116. objc.rs was the only remaining occurrence of the anti-pattern in crates/codegraph-core/src/extractors/ (verified via grep -rnE "for\s+\w+\s+in\s+0\.\." crates/codegraph-core/src/extractors/ followed by checking each loop body).

Test plan

• `cargo test -p codegraph-core --lib extractors::objc` (9/9 pass)

Closes #1116

[claude]

Claude finished @carlos-alm's task —— View job

---

Code Review Analysis

I'm reviewing this PR that refactors Objective-C extractor code to use if let Some instead of ? operator in for-loops.

Todo List:

• Read the changed file to understand the code modifications
• Examine related files (Verilog extractor) for pattern comparison
• Analyze technical correctness of the ? vs if let Some change
• Apply 5-phase review framework
• Provide final assessment and recommendation

Root Cause Analysis

Problem Being Solved: The ? operator inside a for i in 0..node.child_count() loop short-circuits the entire enclosing function on the first None, rather than just continuing to the next loop iteration. While node.child(i) returns Some for every i < child_count() in practice (making this currently harmless), using if let Some(child) = node.child(i) is more defensively correct.

Why This Approach: This change aligns with the established pattern used consistently across all other extractors in the codebase. I verified that:

1. The Verilog extractor (lines 151, 165) uses the same if let Some pattern after PR refactor(verilog): use if let Some in for-loops instead of ? #1155
2. All other extractors in the codebase use if let Some consistently (grep confirmed 80+ occurrences)
3. No remaining instances of the .child(i)? anti-pattern exist (confirmed via grep)

Risk Assessment: Very low risk. This is a defensive coding improvement with no behavioral change in practice.

Technical Analysis

The change is technically sound:

Before (objc.rs:496):

for i in 0..struct_decl.child_count() {
    let child = struct_decl.child(i)?;  // ❌ Short-circuits function on None
    if child.kind() == "struct_declarator" {
        // ...
    }
}

After (objc.rs:496-506):

for i in 0..struct_decl.child_count() {
    if let Some(child) = struct_decl.child(i) {  // ✅ Continues loop on None  
        if child.kind() == "struct_declarator" {
            // ...
        }
    }
}

This matches the pattern used throughout the codebase and is more explicit about the intent.

Backlog Compliance

• Zero-dep: ✓ — No new dependencies added
• Foundation-aligned: ✓ — Pure refactoring, aligns with P7 (correctness over convenience)
• Problem-fit: 3 — Code quality improvement, indirectly helps AI agents by maintaining extraction correctness
• Breaking: No — Pure internal refactoring with identical behavior
• Tier: 1

Critical Concerns

None. This is a textbook defensive coding improvement.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: Excellent defensive refactoring that completes a systematic cleanup across the extractor codebase. The change is technically sound, well-motivated, follows established patterns, and has zero behavioral impact. The commit message and PR description are thorough and accurate. Only minor deduction for being a relatively small improvement rather than a major feature.

[greptile-apps]

Greptile Summary

This PR fixes a subtle control-flow issue in extract_property_name in objc.rs: the ? operator inside the for i in 0..child_count() loop would have returned None from the enclosing function on the first missing child, rather than simply skipping that iteration. The fix replaces it with if let Some(child), restricting the None-handling to the loop body.

• Control-flow correction: if let Some(child) = struct_decl.child(i) now skips missing children without aborting the function, matching the intent of the loop.
• Consistency: Aligns objc.rs with the pattern already adopted in the Verilog extractor after refactor(verilog): use if let Some in for-loops instead of ? #1155, completing the sweep tracked in follow-up: use if let Some(child) = ... over node.child(i)? in Verilog extractor for-loops #1116.

Confidence Score: 5/5

Safe to merge — the change narrows an overly-broad early-return to the loop iteration where it belongs, with no functional impact on the happy path.

The only changed code is the substitution of ? with if let Some inside a bounded for-loop. In practice node.child(i) never returns None for i < child_count(), so behavior is identical today; the fix simply makes the code correct by construction and prevents a surprising early-return if tree-sitter's guarantees ever change.

No files require special attention.

Important Files Changed

Filename	Overview
crates/codegraph-core/src/extractors/objc.rs	Replaces ? with if let Some inside a for-loop in extract_property_name, correctly scoping None-handling to the iteration rather than the whole function.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[extract_property_name] --> B[find_child struct_declaration]
    B -->|None| C[return None]
    B -->|Some| D["for i in 0..child_count()"]
    D --> E["struct_decl.child(i)"]
    E -->|None| F[skip iteration]
    F --> D
    E -->|Some child| G{kind == struct_declarator?}
    G -->|No| D
    G -->|Yes| H[unwrap_property_declarator]
    H -->|non-empty| I[return Some name]
    H -->|empty| D
    D -->|exhausted| J[return None]

Loading

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/1116-objc-i..." | Re-trigger Greptile}

[github-actions]

Codegraph Impact Analysis

1 functions changed → 3 callers affected across 1 files

• extract_property_name in crates/codegraph-core/src/extractors/objc.rs:493 (3 transitive callers)