Summary
collect_companion_in_group() and module_has_harness_descriptor() in src/hir/mod.rs use nested iteration patterns that produce O(n²) complexity within each module scope. Neither function currently carries a docstring noting this behaviour or justifying it by bounded input size. Two further domain-architecture concerns have also been identified (see below).
Problem
1. O(n²) complexity — undocumented
Without complexity documentation:
- Future contributors may not recognise the performance profile of these functions.
- It is unclear whether the quadratic behaviour is acceptable because module sizes are bounded in practice, or whether a lookup-map optimisation should be pursued.
2. Undocumented detection heuristic
The detection logic in collect_companion_in_group() / module_has_harness_descriptor() matches any same-named sibling module that contains a const item — it is not restricted to rstest-synthesised companion modules. This undocumented heuristic is prone to false positives (e.g. a hand-written mod parse { pub const VERSION: &str = "1"; } adjacent to a function named parse would be treated as a harness companion). The function should either be renamed to reflect what it actually matches, or carry a prominent docstring warning about the heuristic and its limitations.
3. Redundant ancestor walk
The parent-module iteration inside collect_companion_in_group() repeats traversal work that is already available at the call site's scope. This redundancy adds unnecessary complexity and makes the function harder to reason about. The redundant walk should be documented (with a justification) or eliminated by passing the required context from the call site.
Proposed resolution
- Add docstrings to both functions documenting the O(n²) complexity and justifying it with a bounded input-size argument (e.g. the number of items in a module scope is small and fixed at compile time).
- Document the detection heuristic explicitly — state what structural shape is matched, note the false-positive risk, and either rename the function to reflect the actual predicate or add a
# Limitations section to its docstring.
- Document or eliminate the redundant ancestor walk in
collect_companion_in_group(): either add a comment explaining why the re-traversal is necessary, or refactor to accept the parent context as a parameter.
- Optionally, replace the nested iteration with pre-filtering and a lookup map to reduce the worst-case complexity.
Context
Originally identified during review of PR #221. The functions are not part of that PR's diff, so all fixes are tracked here as a follow-up. Items 2 and 3 were added following the Domain Architecture check raised in the same PR review.
Raised by @leynos.
Summary
collect_companion_in_group()andmodule_has_harness_descriptor()insrc/hir/mod.rsuse nested iteration patterns that produce O(n²) complexity within each module scope. Neither function currently carries a docstring noting this behaviour or justifying it by bounded input size. Two further domain-architecture concerns have also been identified (see below).Problem
1. O(n²) complexity — undocumented
Without complexity documentation:
2. Undocumented detection heuristic
The detection logic in
collect_companion_in_group()/module_has_harness_descriptor()matches any same-named sibling module that contains aconstitem — it is not restricted to rstest-synthesised companion modules. This undocumented heuristic is prone to false positives (e.g. a hand-writtenmod parse { pub const VERSION: &str = "1"; }adjacent to a function namedparsewould be treated as a harness companion). The function should either be renamed to reflect what it actually matches, or carry a prominent docstring warning about the heuristic and its limitations.3. Redundant ancestor walk
The parent-module iteration inside
collect_companion_in_group()repeats traversal work that is already available at the call site's scope. This redundancy adds unnecessary complexity and makes the function harder to reason about. The redundant walk should be documented (with a justification) or eliminated by passing the required context from the call site.Proposed resolution
# Limitationssection to its docstring.collect_companion_in_group(): either add a comment explaining why the re-traversal is necessary, or refactor to accept the parent context as a parameter.Context
Originally identified during review of PR #221. The functions are not part of that PR's diff, so all fixes are tracked here as a follow-up. Items 2 and 3 were added following the Domain Architecture check raised in the same PR review.
Raised by @leynos.