diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 74db0b32..a8397b34 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -18,6 +18,7 @@ import subprocess import sys import threading +from bisect import bisect_right from dataclasses import dataclass, field from functools import lru_cache from pathlib import Path, PurePath @@ -4668,6 +4669,7 @@ def _resolve_call_targets( ) is_cpp = any(node.language == "cpp" for node in nodes) + is_go = any(node.language == "go" for node in nodes) def cpp_resolution_extra( extra: dict, @@ -4702,6 +4704,12 @@ def cpp_resolution_extra( callable_symbols[bare].append(entry) source_scopes[qualified] = node.parent_name + go_receivers = { + self._node_qualified(node): node.extra["go_receiver"] + for node in nodes + if node.language == "go" and "go_receiver" in node.extra + } + def candidate_entries( target: str, edge_kind: str, @@ -4721,6 +4729,30 @@ def candidate_entries( continue receiver = edge.extra.get("receiver") has_receiver = bool(receiver) + if ( + is_go + and edge.kind == "CALLS" + and edge.extra.get("go_method_receiver") + and receiver == go_receivers.get(edge.source) + ): + candidates = [ + qualified + for qualified, parent_scope in candidate_entries( + edge.target, edge.kind, + ) + if parent_scope == source_scopes.get(edge.source) + ] + if len(candidates) == 1: + edge = EdgeInfo( + kind=edge.kind, + source=edge.source, + target=candidates[0], + file_path=edge.file_path, + line=edge.line, + extra=edge.extra, + ) + resolved.append(edge) + continue if edge.kind in ("CALLS", "REFERENCES") and "::" not in edge.target: # JS/TS calls retain their full member expression as evidence # (``app.handle``) while keeping the method name (``handle``) @@ -6225,6 +6257,9 @@ def _extract_from_tree( import_map: Optional[dict[str, str]] = None, defined_names: Optional[set[str]] = None, _depth: int = 0, + _go_receiver_bindings: Optional[ + tuple[str, tuple[int, ...], tuple[int, ...]] + ] = None, ) -> None: """Recursively walk the AST and extract nodes/edges.""" if _depth > self._MAX_AST_DEPTH: @@ -6445,7 +6480,7 @@ def _extract_from_tree( if self._extract_calls( child, source, language, file_path, nodes, edges, enclosing_class, enclosing_func, - import_map, defined_names, _depth, + import_map, defined_names, _depth, _go_receiver_bindings, ): continue @@ -6489,6 +6524,7 @@ def _extract_from_tree( enclosing_func=enclosing_func, import_map=import_map, defined_names=defined_names, _depth=_depth + 1, + _go_receiver_bindings=_go_receiver_bindings, ) def _elixir_call_identifier(self, node) -> Optional[str]: @@ -10400,6 +10436,14 @@ def _extract_functions( # Java: detect Temporal method-level annotations and Kafka listeners method_extra: dict = {} + go_receiver_bindings = None + if language == "go" and child.type == "method_declaration": + receiver_name = self._get_go_receiver_name(child) + if receiver_name: + method_extra["go_receiver"] = receiver_name + go_receiver_bindings = self._go_receiver_binding_index( + child, receiver_name, + ) if julia_qualifier: method_extra["julia_module_qualifier"] = julia_qualifier if language == "java" and deco_list: @@ -10558,6 +10602,7 @@ def _extract_functions( enclosing_class=recursive_class, enclosing_func=identity_name, import_map=import_map, defined_names=defined_names, _depth=_depth + 1, + _go_receiver_bindings=go_receiver_bindings, ) return True @@ -10604,6 +10649,9 @@ def _extract_calls( import_map: Optional[dict[str, str]], defined_names: Optional[set[str]], _depth: int, + _go_receiver_bindings: Optional[ + tuple[str, tuple[int, ...], tuple[int, ...]] + ] = None, ) -> bool: """Extract call expressions, including test runner special cases. @@ -10726,7 +10774,7 @@ def _extract_calls( call_extra["member_call"] = member_call if ( language in self._TYPED_CALL_LANGUAGES - or language in ("cpp", "rust") + or language in ("cpp", "go", "rust") ): receiver, method_name = self._get_member_call_receiver_method( child, language, @@ -10735,6 +10783,15 @@ def _extract_calls( call_name = method_name if receiver: call_extra["receiver"] = receiver + if ( + language == "go" + and _go_receiver_bindings is not None + and receiver == _go_receiver_bindings[0] + and not self._go_receiver_is_shadowed( + child, _go_receiver_bindings, + ) + ): + call_extra["go_method_receiver"] = True if language == "java" and child.type == "method_reference": call_extra["call_syntax"] = "method_reference" @@ -10876,6 +10933,29 @@ def _get_member_call_receiver_method( method.text.decode("utf-8", errors="replace"), ) + if language == "go" and node.type == "call_expression": + callee = node.child_by_field_name("function") + if callee is None or callee.type != "selector_expression": + return None, None + receiver = callee.child_by_field_name("operand") + method = callee.child_by_field_name("field") + if receiver is None or method is None: + return None, None + while receiver.type == "parenthesized_expression": + if len(receiver.named_children) != 1: + break + receiver = receiver.named_children[0] + if ( + receiver.type == "unary_expression" + and receiver.children + and receiver.children[0].type == "*" + ): + receiver = receiver.child_by_field_name("operand") or receiver + return ( + receiver.text.decode("utf-8", errors="replace"), + method.text.decode("utf-8", errors="replace"), + ) + if language == "cpp" and node.type == "call_expression": callee = node.child_by_field_name("function") if callee is None or callee.type != "field_expression": @@ -14723,6 +14803,187 @@ def _get_go_receiver_type(self, node) -> Optional[str]: return None return None + @staticmethod + def _get_go_receiver_name(node) -> Optional[str]: + """Return the variable name from a Go method receiver.""" + receiver = node.child_by_field_name("receiver") + if receiver is None: + return None + parameter = next( + (child for child in receiver.children if child.type == "parameter_declaration"), + None, + ) + if parameter is None: + return None + name = next( + (child for child in parameter.children if child.type == "identifier"), + None, + ) + return name.text.decode("utf-8", errors="replace") if name else None + + _GO_LEXICAL_SCOPE_TYPES = frozenset({ + "block", "communication_case", "default_case", "expression_case", "type_case", + }) + _GO_DECLARATION_NAME_FIELDS = { + "const_spec": "name", + "type_alias": "name", + "type_spec": "name", + "var_spec": "name", + } + _GO_INIT_SCOPE_PARENTS = frozenset({ + "expression_switch_statement", "if_statement", "type_switch_statement", + }) + _GO_PARAMETER_TYPES = frozenset({ + "parameter_declaration", "variadic_parameter_declaration", + }) + + @staticmethod + def _go_field_binds_name(node, field: str, name: str) -> bool: + """Return whether a grammar field declares ``name``.""" + for field_node in node.children_by_field_name(field): + candidates = ( + field_node.named_children + if field_node.type == "expression_list" + else (field_node,) + ) + if any( + candidate.type in {"identifier", "type_identifier"} + and candidate.text.decode("utf-8", errors="replace") == name + for candidate in candidates + ): + return True + return False + + @classmethod + def _go_function_parameters_bind_name(cls, node, name: str) -> bool: + """Return whether a function's parameters or named results bind ``name``.""" + for field_name in ("parameters", "result"): + for parameter_list in node.children_by_field_name(field_name): + if any( + parameter.type in cls._GO_PARAMETER_TYPES + and cls._go_field_binds_name(parameter, "name", name) + for parameter in parameter_list.named_children + ): + return True + return False + + def _go_receiver_binding_index( + self, method, receiver_name: str, + ) -> tuple[str, tuple[int, ...], tuple[int, ...]]: + """Build merged lexical-shadow intervals for one Go method.""" + body = method.child_by_field_name("body") + if body is None: + return receiver_name, (), () + + def scope_key(scope) -> tuple[str, int, int]: + return scope.type, scope.start_byte, scope.end_byte + + intervals: list[tuple[int, int]] = [] + receiver_scopes = {scope_key(body)} + stack = [(body, body)] + while stack: + node, scope = stack.pop() + if node.type in self._GO_LEXICAL_SCOPE_TYPES: + scope = node + + if node.type == "func_literal": + function_body = node.child_by_field_name("body") + if ( + function_body is not None + and self._go_function_parameters_bind_name(node, receiver_name) + ): + intervals.append((function_body.start_byte, function_body.end_byte)) + receiver_scopes.add(scope_key(function_body)) + + if ( + node.type == "type_switch_statement" + and self._go_field_binds_name(node, "alias", receiver_name) + ): + for case in node.named_children: + if case.type not in {"default_case", "type_case"}: + continue + statements = next( + ( + child for child in case.named_children + if child.type == "statement_list" + ), + None, + ) + if statements is not None: + intervals.append((statements.start_byte, case.end_byte)) + receiver_scopes.add(scope_key(case)) + + declaration_field = self._GO_DECLARATION_NAME_FIELDS.get(node.type) + if ( + declaration_field is not None + and self._go_field_binds_name(node, declaration_field, receiver_name) + ): + intervals.append((node.end_byte, scope.end_byte)) + receiver_scopes.add(scope_key(scope)) + + if ( + node.type == "short_var_declaration" + and self._go_field_binds_name(node, "left", receiver_name) + ): + parent = node.parent + if parent is not None and parent.type == "for_clause": + binding_scope = parent.parent + elif parent is not None and parent.type in self._GO_INIT_SCOPE_PARENTS: + binding_scope = parent + else: + binding_scope = scope + if ( + binding_scope is not None + and scope_key(binding_scope) not in receiver_scopes + ): + intervals.append((node.end_byte, binding_scope.end_byte)) + receiver_scopes.add(scope_key(binding_scope)) + + if ( + node.type == "range_clause" + and any(child.type == ":=" for child in node.children) + and self._go_field_binds_name(node, "left", receiver_name) + ): + loop = node.parent + loop_body = loop.child_by_field_name("body") if loop is not None else None + if loop_body is not None: + intervals.append((loop_body.start_byte, loop_body.end_byte)) + receiver_scopes.add(scope_key(loop_body)) + + if ( + node.type == "receive_statement" + and any(child.type == ":=" for child in node.children) + and self._go_field_binds_name(node, "left", receiver_name) + and node.parent is not None + ): + intervals.append((node.end_byte, node.parent.end_byte)) + receiver_scopes.add(scope_key(node.parent)) + + stack.extend((child, scope) for child in reversed(node.named_children)) + + merged: list[tuple[int, int]] = [] + for start, end in sorted(intervals): + if start >= end: + continue + if merged and start <= merged[-1][1]: + merged[-1] = merged[-1][0], max(merged[-1][1], end) + else: + merged.append((start, end)) + return ( + receiver_name, + tuple(start for start, _ in merged), + tuple(end for _, end in merged), + ) + + @staticmethod + def _go_receiver_is_shadowed( + node, + bindings: tuple[str, tuple[int, ...], tuple[int, ...]], + ) -> bool: + """Return whether a pre-indexed Go binding shadows the receiver.""" + index = bisect_right(bindings[1], node.start_byte) - 1 + return index >= 0 and node.start_byte < bindings[2][index] + @staticmethod def _cpp_scope_join( outer: Optional[str], diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index a0e9613a..605772f1 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -38,6 +38,141 @@ func (r *InMemoryRepo) Save(user *User) error { return nil } +func (r *InMemoryRepo) SaveAndReturn(user *User) error { + return (*r).Save(user) +} + +type ShadowA struct{} +type ShadowB struct{} +type ShadowC int + +func (a *ShadowA) Save() bool { return true } +func (b *ShadowB) Save() bool { return true } +func (c ShadowC) Save() bool { return true } + +func (a *ShadowA) CallsShadowedReceiver() { + func(a *ShadowB) { a.Save() }(&ShadowB{}) +} + +func (a *ShadowA) CallsBlockShadowedReceiver() { + if true { + a := &ShadowB{} + a.Save() + } +} + +func (a *ShadowA) CallsVarShadowedReceiver() { + var a *ShadowB + a.Save() +} + +func (a *ShadowA) CallsRangeShadowedReceiver() { + for a := range []int{1} { + a.Save() + } +} + +func (a *ShadowA) CallsForClauseShadowedReceiver() { + for a := &ShadowB{}; a.Save(); a.Save() { + a.Save() + break + } +} + +func (a *ShadowA) CallsTypeSwitchShadowedReceiver(value any) { + switch a := value.(type) { + case *ShadowB: + a.Save() + } +} + +func (a *ShadowA) CallsExpressionCaseShadowedReceiver() { + switch 1 { + case 1: + var a *ShadowB + a.Save() + } +} + +func (a *ShadowA) CallsSelectCaseShadowedReceiver(ch <-chan struct{}) { + select { + case <-ch: + var a *ShadowB + a.Save() + default: + } +} + +func (a *ShadowA) CallsNamedResultShadowedReceiver() { + func() (a *ShadowB) { + a.Save() + return nil + }() +} + +func (a *ShadowA) CallsAfterShadowScope() { + { + var a *ShadowB + a.Save() + } + a.Save() +} + +func (a *ShadowA) CallsInitializerScope() { + if a := func() *ShadowB { + a.Save() + return nil + }(); a != nil { + a.Save() + } +} + +func (a *ShadowA) CallsSameScopeRedeclaration() { + a, n := a, 1 + _ = n + a.Save() +} + +func (a *ShadowA) CallsTypeSwitchInitShadowedReceiver(value any) { + switch a := func() *ShadowB { + a.Save() + return value.(*ShadowB) + }(); value := any(a).(type) { + case *ShadowB: + _ = value + a.Save() + } + a.Save() +} + +func (a *ShadowA) CallsSelectReceiveShadowedReceiver(ch <-chan *ShadowB) { + select { + case a := <-func() <-chan *ShadowB { + a.Save() + return ch + }(): + a.Save() + default: + } + a.Save() +} + +func (a *ShadowA) CallsConstShadowedReceiver() { + { + const a ShadowC = 0 + a.Save() + } + a.Save() +} + +func (a *ShadowA) CallsTypeShadowedReceiver() { + { + type a = ShadowC + a.Save(0) + } + a.Save() +} + func CreateUser(repo UserRepository, name string, email string) (*User, error) { user := &User{ID: 1, Name: name, Email: email} err := repo.Save(user) diff --git a/tests/test_multilang.py b/tests/test_multilang.py index 2c27f5ab..8ec7c7e7 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -53,14 +53,16 @@ def test_methods_attached_to_receiver(self): struct they belong to. """ funcs = [n for n in self.nodes if n.kind == "Function"] - by_name = {f.name: f for f in funcs} - assert "FindByID" in by_name - assert "Save" in by_name - assert by_name["FindByID"].parent_name == "InMemoryRepo" - assert by_name["Save"].parent_name == "InMemoryRepo" + find_by_id = next(f for f in funcs if f.name == "FindByID") + save = next( + f for f in funcs + if f.name == "Save" and f.parent_name == "InMemoryRepo" + ) + assert find_by_id.parent_name == "InMemoryRepo" + assert save.parent_name == "InMemoryRepo" # Free functions should still have no parent. - assert by_name["NewInMemoryRepo"].parent_name is None - assert by_name["CreateUser"].parent_name is None + assert next(f for f in funcs if f.name == "NewInMemoryRepo").parent_name is None + assert next(f for f in funcs if f.name == "CreateUser").parent_name is None contains = [(e.source, e.target) for e in self.edges if e.kind == "CONTAINS"] find_by_id_contains = [ @@ -82,6 +84,155 @@ def test_methods_attached_to_receiver(self): assert find_by_id_contains[0][0].endswith("::InMemoryRepo") assert save_contains[0][0].endswith("::InMemoryRepo") + def test_receiver_call_resolves_to_receiver_method(self): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith("::InMemoryRepo.SaveAndReturn") + ] + assert len(calls) == 1 + assert calls[0].target.endswith("::InMemoryRepo.Save") + assert calls[0].extra["receiver"] == "r" + + @pytest.mark.parametrize( + ("method_name", "call_count"), + [ + ("CallsShadowedReceiver", 1), + ("CallsBlockShadowedReceiver", 1), + ("CallsVarShadowedReceiver", 1), + ("CallsRangeShadowedReceiver", 1), + ("CallsForClauseShadowedReceiver", 3), + ("CallsTypeSwitchShadowedReceiver", 1), + ("CallsExpressionCaseShadowedReceiver", 1), + ("CallsSelectCaseShadowedReceiver", 1), + ("CallsNamedResultShadowedReceiver", 1), + ], + ) + def test_shadowed_receiver_call_stays_unresolved( + self, method_name, call_count, + ): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith(f"::ShadowA.{method_name}") + and edge.extra.get("receiver") == "a" + ] + assert len(calls) == call_count + assert all(edge.target == "Save" for edge in calls) + assert all("go_method_receiver" not in edge.extra for edge in calls) + + def test_receiver_call_resolves_after_shadowing_scope(self): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith("::ShadowA.CallsAfterShadowScope") + and edge.extra.get("receiver") == "a" + ] + assert [edge.target.endswith("::ShadowA.Save") for edge in calls] == [ + False, True, + ] + + def test_initializer_uses_receiver_before_shadowing(self): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith("::ShadowA.CallsInitializerScope") + and edge.extra.get("receiver") == "a" + ] + assert [edge.target.endswith("::ShadowA.Save") for edge in calls] == [ + True, False, + ] + + def test_same_scope_redeclaration_keeps_receiver_resolution(self): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith("::ShadowA.CallsSameScopeRedeclaration") + and edge.extra.get("receiver") == "a" + ] + assert len(calls) == 1 + assert calls[0].target.endswith("::ShadowA.Save") + assert calls[0].extra["go_method_receiver"] is True + + @pytest.mark.parametrize( + ("method_name", "receiver_resolution"), + [ + ("CallsTypeSwitchInitShadowedReceiver", [True, False, True]), + ("CallsSelectReceiveShadowedReceiver", [True, False, True]), + ("CallsConstShadowedReceiver", [False, True]), + ("CallsTypeShadowedReceiver", [False, True]), + ], + ) + def test_receiver_bindings_follow_lexical_lifetime( + self, method_name, receiver_resolution, + ): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith(f"::ShadowA.{method_name}") + and edge.extra.get("receiver") == "a" + ] + assert [ + edge.target.endswith("::ShadowA.Save") for edge in calls + ] == receiver_resolution + + def test_deep_receiver_scope_walk_does_not_overflow(self): + source = ( + "package auth\n" + "type ShadowA struct{}\n" + "func (a *ShadowA) Save() {}\n" + "func (a *ShadowA) Deep() {\n" + + "(" * 1200 + + "a.Save()" + + ")" * 1200 + + "\n}\n" + ).encode() + root = self.parser._get_parser("go").parse(source).root_node + stack = [root] + method = None + call = None + while stack: + current = stack.pop() + if ( + current.type == "method_declaration" + and current.child_by_field_name("name").text == b"Deep" + ): + method = current + if current.type == "call_expression": + call = current + stack.extend(current.children) + assert method is not None + assert call is not None + bindings = self.parser._go_receiver_binding_index(method, "a") + assert not self.parser._go_receiver_is_shadowed(call, bindings) + + def test_receiver_binding_prepass_runs_once_per_method(self, monkeypatch): + builds = [] + original = CodeParser._go_receiver_binding_index + + def counted(parser, method, receiver_name): + name = method.child_by_field_name("name").text.decode() + if name == "ManyCalls": + builds.append(name) + return original(parser, method, receiver_name) + + monkeypatch.setattr(CodeParser, "_go_receiver_binding_index", counted) + source = ( + "package auth\n" + "type ShadowA struct{}\n" + "func (a *ShadowA) Save() {}\n" + "func (a *ShadowA) ManyCalls() {\n" + + "\n".join("a.Save()" for _ in range(200)) + + "\n}\n" + ).encode() + _, edges = CodeParser().parse_bytes(Path("many_calls.go"), source) + calls = [ + edge for edge in edges + if edge.kind == "CALLS" and edge.source.endswith("::ShadowA.ManyCalls") + ] + assert len(calls) == 200 + assert builds == ["ManyCalls"] + class TestRustParsing: def setup_method(self):