fix(CODEWIKI-005-2): CU-86akn96pk 2 review findings across 2 files - #98
flamingo[bot] wants to merge 2 commits into
Conversation
![flamingo[bot]](https://avatars.githubusercontent.com/in/3426709?s=60&v=4){kind=small}
| resolved_base = self.namespace_resolver.resolve(base_name) | ||
| self.call_relationships.append(CallRelationship( | ||
| caller=self._get_component_id(class_name), | ||
| callee=resolved_base.replace("\\", "."), | ||
| callee=self._get_component_id_for_resolved_name(resolved_base), | ||
| call_line=node.start_point[0] + 1, | ||
| is_resolved=False | ||
| )) |
There was a problem hiding this comment.
🦩 🔴 PHP component IDs fall back to a dot-joined base class instead of the '::' FQDN format
Added new helper _get_component_id_for_resolved_name in TreeSitterPHPAnalyzer (php.py) that takes an already-resolved dotted/backslash name, splits it into a module path and final component name, and joins them with :: (falling back to name::name if no separator exists). Updated all five call sites in _extract_relationships that previously did resolved_X.replace("\\", ".") for callee (base class/extends, interface implements, object creation/new, static scoped calls, and property-promotion constructor types) to use this helper instead, so every emitted callee now follows the <module.path>::<ComponentName> FQDN format instead of a plain dotted string. The _add_use_relationships method (which builds file-level use import relationships, not component-to-component base/interface relationships) was left unchanged since the finding specifically concerns base/interface resolution in relationship extraction: a complete fix might also want to reconsider whether "single-segment" names (no namespace) should resolve to the file's own module rather than a degenerate Name::Name, which is unverified without more context on how downstream consumers treat unqualified/global PHP classes.
🤖 Prompt for AI agents
In codewiki/src/be/dependency_analyzer/analyzers/php.py around line 357, review and complete this code-review fix: PHP component IDs fall back to a dot-joined base class instead of the '::' FQDN format.
What the draft fix changed: Added new helper `_get_component_id_for_resolved_name` in `TreeSitterPHPAnalyzer` (php.py) that takes an already-resolved dotted/backslash name, splits it into a module path and final component name, and joins them with `::` (falling back to `name::name` if no separator exists). Updated all five call sites in `_extract_relationships` that previously did `resolved_X.replace("\\", ".")` for `callee` (base class/extends, interface implements, object creation/new, static scoped calls, and property-promotion constructor types) to use this helper instead, so every emitted `callee` now follows the `<module.path>::<ComponentName>` FQDN format instead of a plain dotted string. The `_add_use_relationships` method (which builds file-level `use` import relationships, not component-to-component base/interface relationships) was left unchanged since the finding specifically concerns base/interface resolution in relationship extraction: a complete fix might also want to reconsider whether "single-segment" names (no namespace) should resolve to the file's own module rather than a degenerate `Name::Name`, which is unverified without more context on how downstream consumers treat unqualified/global PHP classes.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer
| if child.type == "method_definition": | ||
| method_name = self._get_method_name(child) | ||
| if method_name: | ||
| method_key = f"{self._get_module_path()}.{class_name}.{method_name}" | ||
| method_key = self._get_component_id(method_name, class_name, is_method=True) | ||
| method_node = self._create_method_node(child, method_name, class_name) | ||
| if method_node: | ||
| self.top_level_nodes[method_key] = method_node | ||
| elif child.type == "field_definition": | ||
| # Handle arrow function properties | ||
| field_name = self._get_field_name(child) | ||
| if field_name and self._is_arrow_function_field(child): | ||
| method_key = f"{self._get_module_path()}.{class_name}.{field_name}" | ||
| method_key = self._get_component_id(field_name, class_name, is_method=True) | ||
| method_node = self._create_method_node(child, field_name, class_name) | ||
| if method_node: | ||
| self.top_level_nodes[method_key] = method_node |
There was a problem hiding this comment.
🦩 🔴 JavaScript analyzer's _extract_methods_from_class uses dot-separated method_key instead of the '::' FQDN separator used elsewhere
Changed _extract_methods_from_class (both the method_definition branch and the field_definition/arrow-function branch) to build method_key via self._get_component_id(method_name, class_name, is_method=True) instead of the ad-hoc f"{self._get_module_path()}.{class_name}.{method_name}", aligning the top_level_nodes key with the ::-separated FQDN convention used by _get_component_id/node.id. Also updated the matching lookup in _extract_call_from_node (the method_key built for this./super. call resolution) to use _get_component_id(callee_name, current_class, is_method=True) so it stays consistent with the new key format used to populate top_level_nodes — without this companion change, method-call resolution against top_level_nodes would have silently broken since the stored keys no longer match the old dot-separated lookup format. Other dot-separated ids elsewhere in the file (e.g. inheritance/call relationship caller/callee ids, JSDoc type ids) were left unchanged since the finding is scoped to _extract_methods_from_class's method_key/top_level_nodes inconsistency, and touching those would exceed the minimal fix intended here.
🤖 Prompt for AI agents
In codewiki/src/be/dependency_analyzer/analyzers/javascript.py around line 172, review and complete this code-review fix: JavaScript analyzer's _extract_methods_from_class uses dot-separated method_key instead of the '::' FQDN separator used elsewhere.
What the draft fix changed: Changed `_extract_methods_from_class` (both the `method_definition` branch and the `field_definition`/arrow-function branch) to build `method_key` via `self._get_component_id(method_name, class_name, is_method=True)` instead of the ad-hoc `f"{self._get_module_path()}.{class_name}.{method_name}"`, aligning the `top_level_nodes` key with the `::`-separated FQDN convention used by `_get_component_id`/`node.id`. Also updated the matching lookup in `_extract_call_from_node` (the `method_key` built for `this.`/`super.` call resolution) to use `_get_component_id(callee_name, current_class, is_method=True)` so it stays consistent with the new key format used to populate `top_level_nodes` — without this companion change, method-call resolution against `top_level_nodes` would have silently broken since the stored keys no longer match the old dot-separated lookup format. Other dot-separated ids elsewhere in the file (e.g. inheritance/call relationship caller/callee ids, JSDoc type ids) were left unchanged since the finding is scoped to `_extract_methods_from_class`'s `method_key`/`top_level_nodes` inconsistency, and touching those would exceed the minimal fix intended here.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer
flamingo
Bot
changed the title
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
codewiki/src/be/dependency_analyzer/analyzers/php.py:357codewiki/src/be/dependency_analyzer/analyzers/javascript.py:172What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
9aa9712c-4ca2-4571-94bc-95e5cff4b3cdMerging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.
ClickUp task: CU-86akn96pk CodeWiki review findings sweep (9 PRs)