Deferred from PR #1107 review.
Original reviewer comment: #1107 (comment)
Context: Greptile observed that several handlers in crates/codegraph-core/src/extractors/verilog.rs use node.child(i)? inside a for i in 0..node.child_count() loop. The ? operator returns from the enclosing function on None rather than skipping the iteration. In practice child_count() always yields valid indices so this works correctly today, but if let Some(child) = node.child(i) is more defensively correct and consistent with the handlers below (handle_package_import, handle_include_directive).
Scope: sweep find_class_name, find_class_superclass, find_function_or_task_name and any similar patterns in the file to use if let Some instead. Same sweep should also check the other native extractors for the same anti-pattern.
Why follow-up: orthogonal style cleanup that doesn't affect correctness on the current grammar — kept out of #1107 to keep that PR focused on the Verilog class extraction fix.
Deferred from PR #1107 review.
Original reviewer comment: #1107 (comment)
Context: Greptile observed that several handlers in
crates/codegraph-core/src/extractors/verilog.rsusenode.child(i)?inside afor i in 0..node.child_count()loop. The?operator returns from the enclosing function onNonerather than skipping the iteration. In practicechild_count()always yields valid indices so this works correctly today, butif let Some(child) = node.child(i)is more defensively correct and consistent with the handlers below (handle_package_import,handle_include_directive).Scope: sweep
find_class_name,find_class_superclass,find_function_or_task_nameand any similar patterns in the file to useif let Someinstead. Same sweep should also check the other native extractors for the same anti-pattern.Why follow-up: orthogonal style cleanup that doesn't affect correctness on the current grammar — kept out of #1107 to keep that PR focused on the Verilog class extraction fix.