From 208e94a70978fc642229806dd3ad3585cf66d63b Mon Sep 17 00:00:00 2001 From: MerlinH Date: Fri, 7 Aug 2026 15:09:47 +0000 Subject: [PATCH 1/6] fix(go): resolve receiver method calls --- code_review_graph/parser.py | 67 ++++++++++++++++++++++++++++++++++++- tests/fixtures/sample_go.go | 4 +++ tests/test_multilang.py | 10 ++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 74db0b321..831849b06 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -4668,6 +4668,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 +4703,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 +4728,29 @@ def candidate_entries( continue receiver = edge.extra.get("receiver") has_receiver = bool(receiver) + if ( + is_go + and edge.kind == "CALLS" + 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``) @@ -10400,6 +10430,10 @@ def _extract_functions( # Java: detect Temporal method-level annotations and Kafka listeners method_extra: dict = {} + 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 if julia_qualifier: method_extra["julia_module_qualifier"] = julia_qualifier if language == "java" and deco_list: @@ -10726,7 +10760,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, @@ -10876,6 +10910,19 @@ 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 + 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 +14770,24 @@ 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 + @staticmethod def _cpp_scope_join( outer: Optional[str], diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index a0e9613a7..9f316ae0a 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -38,6 +38,10 @@ func (r *InMemoryRepo) Save(user *User) error { return nil } +func (r *InMemoryRepo) SaveAndReturn(user *User) error { + return r.Save(user) +} + 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 2c27f5aba..267e78129 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -82,6 +82,16 @@ 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" + class TestRustParsing: def setup_method(self): From b7f53540612ca8f7e48d9f2e4526041bb80d5b80 Mon Sep 17 00:00:00 2001 From: MerlinH Date: Fri, 7 Aug 2026 15:27:03 +0000 Subject: [PATCH 2/6] fix(go): avoid shadowed receiver calls --- code_review_graph/parser.py | 54 +++++++++++++++++++++++++++++++++++++ tests/fixtures/sample_go.go | 17 ++++++++++++ tests/test_multilang.py | 31 ++++++++++++++++----- 3 files changed, 95 insertions(+), 7 deletions(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 831849b06..782147c7c 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -4731,6 +4731,7 @@ def candidate_entries( if ( is_go and edge.kind == "CALLS" + and edge.extra.get("go_method_receiver") and receiver == go_receivers.get(edge.source) ): candidates = [ @@ -10769,6 +10770,10 @@ def _extract_calls( call_name = method_name if receiver: call_extra["receiver"] = receiver + if language == "go" and not self._go_receiver_is_shadowed( + child, receiver, + ): + call_extra["go_method_receiver"] = True if language == "java" and child.type == "method_reference": call_extra["call_syntax"] = "method_reference" @@ -14788,6 +14793,55 @@ def _get_go_receiver_name(node) -> Optional[str]: ) return name.text.decode("utf-8", errors="replace") if name else None + @staticmethod + def _go_receiver_is_shadowed(node, receiver_name: str) -> bool: + """Return whether a local Go binding shadows a method receiver.""" + cursor = node.parent + while cursor is not None: + if cursor.type == "func_literal": + params = next( + (child for child in cursor.children if child.type == "parameter_list"), + None, + ) + if params is not None and any( + part.type == "identifier" and part.text.decode( + "utf-8", errors="replace" + ) == receiver_name + for parameter in params.children + for part in parameter.children + ): + return True + if cursor.type == "block": + def shadows_receiver(child) -> bool: + if child.start_byte >= node.start_byte: + return False + if child.type == "short_var_declaration": + names = next( + ( + part for part in child.children + if part.type == "expression_list" + ), + None, + ) + if names is not None and any( + part.type == "identifier" and part.text.decode( + "utf-8", errors="replace" + ) == receiver_name + for part in names.children + ): + return True + return any( + shadows_receiver(part) + for part in child.children + if part.type not in ("block", "func_literal") + or (part.start_byte <= node.start_byte < part.end_byte) + ) + + if any(shadows_receiver(child) for child in cursor.children): + return True + cursor = cursor.parent + return False + @staticmethod def _cpp_scope_join( outer: Optional[str], diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index 9f316ae0a..1cfb26ca4 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -42,6 +42,23 @@ func (r *InMemoryRepo) SaveAndReturn(user *User) error { return r.Save(user) } +type ShadowA struct{} +type ShadowB struct{} + +func (a *ShadowA) Save() {} +func (b *ShadowB) Save() {} + +func (a *ShadowA) CallsShadowedReceiver() { + func(a *ShadowB) { a.Save() }(&ShadowB{}) +} + +func (a *ShadowA) CallsBlockShadowedReceiver() { + if true { + a := &ShadowB{} + 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 267e78129..5d04c5ef9 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 = [ @@ -92,6 +94,21 @@ def test_receiver_call_resolves_to_receiver_method(self): assert calls[0].target.endswith("::InMemoryRepo.Save") assert calls[0].extra["receiver"] == "r" + @pytest.mark.parametrize( + "method_name", + ["CallsShadowedReceiver", "CallsBlockShadowedReceiver"], + ) + def test_shadowed_receiver_call_stays_unresolved(self, method_name): + calls = [ + edge for edge in self.edges + if edge.kind == "CALLS" + and edge.source.endswith(f"::ShadowA.{method_name}") + and edge.target == "Save" + ] + assert len(calls) == 1 + assert calls[0].extra["receiver"] == "a" + assert "go_method_receiver" not in calls[0].extra + class TestRustParsing: def setup_method(self): From c72ada49a0e7bd0c1de8c9224041d043585f67bb Mon Sep 17 00:00:00 2001 From: MerlinH Date: Fri, 7 Aug 2026 15:51:20 +0000 Subject: [PATCH 3/6] fix(go): respect lexical receiver shadowing --- code_review_graph/parser.py | 168 +++++++++++++++++++++++++++++------- tests/fixtures/sample_go.go | 42 +++++++++ tests/test_multilang.py | 52 ++++++++++- 3 files changed, 228 insertions(+), 34 deletions(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 782147c7c..26519ac2d 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -14795,53 +14795,155 @@ def _get_go_receiver_name(node) -> Optional[str]: @staticmethod def _go_receiver_is_shadowed(node, receiver_name: str) -> bool: - """Return whether a local Go binding shadows a method receiver.""" + """Return whether a lexical Go binding shadows a method receiver.""" + path = [] cursor = node.parent while cursor is not None: - if cursor.type == "func_literal": - params = next( - (child for child in cursor.children if child.type == "parameter_list"), + path.append(cursor) + cursor = cursor.parent + + for scope in path: + if scope.type in { + "func_literal", "function_declaration", "method_declaration", + } and CodeParser._go_function_binds_name(scope, receiver_name): + return True + if ( + scope.type == "block" + and CodeParser._go_block_binds_name(scope, node, receiver_name) + ): + return True + if scope.type in {"if_statement", "expression_switch_statement"}: + initializer = next( + ( + child for child in scope.children + if child.type == "short_var_declaration" + ), None, ) - if params is not None and any( - part.type == "identifier" and part.text.decode( - "utf-8", errors="replace" - ) == receiver_name - for parameter in params.children - for part in parameter.children + if ( + initializer is not None + and initializer.end_byte <= node.start_byte + and CodeParser._go_binding_binds_name( + initializer, receiver_name, + ) ): return True - if cursor.type == "block": - def shadows_receiver(child) -> bool: - if child.start_byte >= node.start_byte: - return False - if child.type == "short_var_declaration": - names = next( + if scope.type == "for_statement": + body = next( + (child for child in scope.children if child.type == "block"), + None, + ) + if body in path: + range_clause = next( + ( + child for child in scope.children + if child.type == "range_clause" + ), + None, + ) + for_clause = next( + ( + child for child in scope.children + if child.type == "for_clause" + ), + None, + ) + initializer = ( + next( ( - part for part in child.children - if part.type == "expression_list" + child for child in for_clause.children + if child.type == "short_var_declaration" ), None, ) - if names is not None and any( - part.type == "identifier" and part.text.decode( - "utf-8", errors="replace" - ) == receiver_name - for part in names.children - ): - return True - return any( - shadows_receiver(part) - for part in child.children - if part.type not in ("block", "func_literal") - or (part.start_byte <= node.start_byte < part.end_byte) + if for_clause is not None + else None ) + if ( + (range_clause is not None and CodeParser._go_binding_binds_name( + range_clause, receiver_name, + )) + or ( + initializer is not None + and CodeParser._go_binding_binds_name( + initializer, receiver_name, + ) + ) + ): + return True + if ( + scope.type == "type_switch_statement" + and any(part.type == "type_case" for part in path) + and CodeParser._go_binding_binds_name(scope, receiver_name) + ): + return True + return False - if any(shadows_receiver(child) for child in cursor.children): - return True - cursor = cursor.parent + @staticmethod + def _go_function_binds_name(node, receiver_name: str) -> bool: + """Return whether a Go function scope binds ``receiver_name``.""" + parameters = [ + child for child in node.children if child.type == "parameter_list" + ] + if node.type == "method_declaration": + parameters = parameters[1:] + return any( + child.type == "identifier" + and child.text.decode("utf-8", errors="replace") == receiver_name + for parameter_list in parameters + for parameter in parameter_list.children + for child in parameter.children + ) + + @staticmethod + def _go_block_binds_name(block, node, receiver_name: str) -> bool: + """Return whether an earlier declaration in ``block`` binds the name.""" + statements = next( + (child for child in block.children if child.type == "statement_list"), + None, + ) + if statements is None: + return False + for statement in statements.children: + if statement.type == "var_declaration": + for spec in statement.children: + if ( + spec.type == "var_spec" + and spec.end_byte <= node.start_byte + and CodeParser._go_binding_binds_name(spec, receiver_name) + ): + return True + elif ( + statement.type == "short_var_declaration" + and statement.end_byte <= node.start_byte + and CodeParser._go_binding_binds_name(statement, receiver_name) + ): + return True return False + @staticmethod + def _go_binding_binds_name(node, receiver_name: str) -> bool: + """Return whether a Go declaration's left side binds ``receiver_name``.""" + if node.type == "var_spec": + candidates = node.children + else: + try: + assignment = next( + index for index, child in enumerate(node.children) + if child.type == ":=" + ) + except StopIteration: + return False + candidates = node.children[:assignment] + return any( + child.type == "identifier" + and child.text.decode("utf-8", errors="replace") == receiver_name + for candidate in candidates + for child in ( + candidate.children if candidate.type == "expression_list" else (candidate,) + ) + ) + @staticmethod def _cpp_scope_join( outer: Optional[str], diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index 1cfb26ca4..ea97ee19f 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -59,6 +59,48 @@ func (a *ShadowA) CallsBlockShadowedReceiver() { } } +func (a *ShadowA) CallsVarShadowedReceiver() { + var a *ShadowB + a.Save() +} + +func (a *ShadowA) CallsRangeShadowedReceiver() { + for a := range []int{1} { + a.Save() + } +} + +func (a *ShadowA) CallsTypeSwitchShadowedReceiver(value any) { + switch a := value.(type) { + case *ShadowB: + a.Save() + } +} + +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 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 5d04c5ef9..5a23873dc 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -96,7 +96,14 @@ def test_receiver_call_resolves_to_receiver_method(self): @pytest.mark.parametrize( "method_name", - ["CallsShadowedReceiver", "CallsBlockShadowedReceiver"], + [ + "CallsShadowedReceiver", + "CallsBlockShadowedReceiver", + "CallsVarShadowedReceiver", + "CallsRangeShadowedReceiver", + "CallsTypeSwitchShadowedReceiver", + "CallsNamedResultShadowedReceiver", + ], ) def test_shadowed_receiver_call_stays_unresolved(self, method_name): calls = [ @@ -109,6 +116,49 @@ def test_shadowed_receiver_call_stays_unresolved(self, method_name): assert calls[0].extra["receiver"] == "a" assert "go_method_receiver" not in calls[0].extra + 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_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] + while stack: + current = stack.pop() + if current.type == "call_expression": + assert not self.parser._go_receiver_is_shadowed(current, "a") + return + stack.extend(current.children) + pytest.fail("deep Go fixture should contain a call expression") + class TestRustParsing: def setup_method(self): From 5f65f626f70a69050c22aee9a710fc9a85778bb4 Mon Sep 17 00:00:00 2001 From: MerlinH Date: Fri, 7 Aug 2026 16:17:17 +0000 Subject: [PATCH 4/6] fix(go): cover remaining receiver scopes --- code_review_graph/parser.py | 106 ++++++++++++++++++++++-------------- tests/fixtures/sample_go.go | 34 +++++++++++- tests/test_multilang.py | 40 ++++++++++---- 3 files changed, 124 insertions(+), 56 deletions(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 26519ac2d..deb3d2bc4 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -14807,11 +14807,19 @@ def _go_receiver_is_shadowed(node, receiver_name: str) -> bool: "func_literal", "function_declaration", "method_declaration", } and CodeParser._go_function_binds_name(scope, receiver_name): return True - if ( - scope.type == "block" - and CodeParser._go_block_binds_name(scope, node, receiver_name) - ): - return True + if scope.type in { + "block", "expression_case", "type_case", "communication_case", + }: + method = scope.parent if scope.type == "block" else None + if CodeParser._go_statement_scope_binds_name( + scope, + node, + receiver_name, + method is not None + and method.type == "method_declaration" + and CodeParser._get_go_receiver_name(method) == receiver_name, + ): + return True if scope.type in {"if_statement", "expression_switch_statement"}: initializer = next( ( @@ -14833,44 +14841,45 @@ def _go_receiver_is_shadowed(node, receiver_name: str) -> bool: (child for child in scope.children if child.type == "block"), None, ) - if body in path: - range_clause = next( + range_clause = next( + ( + child for child in scope.children + if child.type == "range_clause" + ), + None, + ) + for_clause = next( + ( + child for child in scope.children + if child.type == "for_clause" + ), + None, + ) + initializer = ( + next( ( - child for child in scope.children - if child.type == "range_clause" + child for child in for_clause.children + if child.type == "short_var_declaration" ), None, ) - for_clause = next( - ( - child for child in scope.children - if child.type == "for_clause" - ), - None, + if for_clause is not None + else None + ) + if ( + range_clause is not None + and body in path + and CodeParser._go_binding_binds_name( + range_clause, receiver_name, ) - initializer = ( - next( - ( - child for child in for_clause.children - if child.type == "short_var_declaration" - ), - None, - ) - if for_clause is not None - else None + ) or ( + initializer is not None + and initializer.end_byte <= node.start_byte + and CodeParser._go_binding_binds_name( + initializer, receiver_name, ) - if ( - (range_clause is not None and CodeParser._go_binding_binds_name( - range_clause, receiver_name, - )) - or ( - initializer is not None - and CodeParser._go_binding_binds_name( - initializer, receiver_name, - ) - ) - ): - return True + ): + return True if ( scope.type == "type_switch_statement" and any(part.type == "type_case" for part in path) @@ -14896,10 +14905,15 @@ def _go_function_binds_name(node, receiver_name: str) -> bool: ) @staticmethod - def _go_block_binds_name(block, node, receiver_name: str) -> bool: - """Return whether an earlier declaration in ``block`` binds the name.""" + def _go_statement_scope_binds_name( + scope, + node, + receiver_name: str, + receiver_already_declared: bool, + ) -> bool: + """Return whether an earlier declaration in ``scope`` binds the name.""" statements = next( - (child for child in block.children if child.type == "statement_list"), + (child for child in scope.children if child.type == "statement_list"), None, ) if statements is None: @@ -14916,17 +14930,25 @@ def _go_block_binds_name(block, node, receiver_name: str) -> bool: elif ( statement.type == "short_var_declaration" and statement.end_byte <= node.start_byte - and CodeParser._go_binding_binds_name(statement, receiver_name) + and CodeParser._go_binding_binds_name( + statement, receiver_name, receiver_already_declared, + ) ): return True return False @staticmethod - def _go_binding_binds_name(node, receiver_name: str) -> bool: + def _go_binding_binds_name( + node, + receiver_name: str, + receiver_already_declared: bool = False, + ) -> bool: """Return whether a Go declaration's left side binds ``receiver_name``.""" if node.type == "var_spec": candidates = node.children else: + if receiver_already_declared: + return False try: assignment = next( index for index, child in enumerate(node.children) diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index ea97ee19f..af2ed25dd 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -45,8 +45,8 @@ func (r *InMemoryRepo) SaveAndReturn(user *User) error { type ShadowA struct{} type ShadowB struct{} -func (a *ShadowA) Save() {} -func (b *ShadowB) Save() {} +func (a *ShadowA) Save() bool { return true } +func (b *ShadowB) Save() bool { return true } func (a *ShadowA) CallsShadowedReceiver() { func(a *ShadowB) { a.Save() }(&ShadowB{}) @@ -70,6 +70,13 @@ func (a *ShadowA) CallsRangeShadowedReceiver() { } } +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: @@ -77,6 +84,23 @@ func (a *ShadowA) CallsTypeSwitchShadowedReceiver(value any) { } } +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() @@ -101,6 +125,12 @@ func (a *ShadowA) CallsInitializerScope() { } } +func (a *ShadowA) CallsSameScopeRedeclaration() { + a, n := a, 1 + _ = n + 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 5a23873dc..97def36a8 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -95,26 +95,31 @@ def test_receiver_call_resolves_to_receiver_method(self): assert calls[0].extra["receiver"] == "r" @pytest.mark.parametrize( - "method_name", + ("method_name", "call_count"), [ - "CallsShadowedReceiver", - "CallsBlockShadowedReceiver", - "CallsVarShadowedReceiver", - "CallsRangeShadowedReceiver", - "CallsTypeSwitchShadowedReceiver", - "CallsNamedResultShadowedReceiver", + ("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): + 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.target == "Save" + and edge.extra.get("receiver") == "a" ] - assert len(calls) == 1 - assert calls[0].extra["receiver"] == "a" - assert "go_method_receiver" not in calls[0].extra + 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 = [ @@ -138,6 +143,17 @@ def test_initializer_uses_receiver_before_shadowing(self): 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 + def test_deep_receiver_scope_walk_does_not_overflow(self): source = ( "package auth\n" From 23379e2be0521db091b794dea7b9fcf17393d85c Mon Sep 17 00:00:00 2001 From: MerlinH Date: Fri, 7 Aug 2026 17:51:30 +0000 Subject: [PATCH 5/6] fix(go): index receiver lexical bindings --- code_review_graph/parser.py | 316 ++++++++++++++++++------------------ tests/fixtures/sample_go.go | 42 +++++ tests/test_multilang.py | 64 +++++++- 3 files changed, 265 insertions(+), 157 deletions(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index deb3d2bc4..0e0166adb 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 @@ -6256,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: @@ -6476,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 @@ -6520,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]: @@ -10431,10 +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: @@ -10593,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 @@ -10639,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. @@ -10770,8 +10783,13 @@ def _extract_calls( call_name = method_name if receiver: call_extra["receiver"] = receiver - if language == "go" and not self._go_receiver_is_shadowed( - child, 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": @@ -14793,178 +14811,168 @@ def _get_go_receiver_name(node) -> Optional[str]: ) 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_receiver_is_shadowed(node, receiver_name: str) -> bool: - """Return whether a lexical Go binding shadows a method receiver.""" - path = [] - cursor = node.parent - while cursor is not None: - path.append(cursor) - cursor = cursor.parent - - for scope in path: - if scope.type in { - "func_literal", "function_declaration", "method_declaration", - } and CodeParser._go_function_binds_name(scope, receiver_name): + 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 - if scope.type in { - "block", "expression_case", "type_case", "communication_case", - }: - method = scope.parent if scope.type == "block" else None - if CodeParser._go_statement_scope_binds_name( - scope, - node, - receiver_name, - method is not None - and method.type == "method_declaration" - and CodeParser._get_go_receiver_name(method) == receiver_name, + 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 - if scope.type in {"if_statement", "expression_switch_statement"}: - initializer = next( - ( - child for child in scope.children - if child.type == "short_var_declaration" - ), - None, - ) + 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 ( - initializer is not None - and initializer.end_byte <= node.start_byte - and CodeParser._go_binding_binds_name( - initializer, receiver_name, - ) + function_body is not None + and self._go_function_parameters_bind_name(node, receiver_name) ): - return True - if scope.type == "for_statement": - body = next( - (child for child in scope.children if child.type == "block"), - None, - ) - range_clause = next( - ( - child for child in scope.children - if child.type == "range_clause" - ), - None, - ) - for_clause = next( - ( - child for child in scope.children - if child.type == "for_clause" - ), - None, - ) - initializer = ( - next( + 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 for_clause.children - if child.type == "short_var_declaration" + child for child in case.named_children + if child.type == "statement_list" ), None, ) - if for_clause is not None - else 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 ( - range_clause is not None - and body in path - and CodeParser._go_binding_binds_name( - range_clause, receiver_name, - ) - ) or ( - initializer is not None - and initializer.end_byte <= node.start_byte - and CodeParser._go_binding_binds_name( - initializer, receiver_name, - ) + binding_scope is not None + and scope_key(binding_scope) not in receiver_scopes ): - return True + intervals.append((node.end_byte, binding_scope.end_byte)) + receiver_scopes.add(scope_key(binding_scope)) + if ( - scope.type == "type_switch_statement" - and any(part.type == "type_case" for part in path) - and CodeParser._go_binding_binds_name(scope, receiver_name) + node.type == "range_clause" + and any(child.type == ":=" for child in node.children) + and self._go_field_binds_name(node, "left", receiver_name) ): - return True - return False + 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)) - @staticmethod - def _go_function_binds_name(node, receiver_name: str) -> bool: - """Return whether a Go function scope binds ``receiver_name``.""" - parameters = [ - child for child in node.children if child.type == "parameter_list" - ] - if node.type == "method_declaration": - parameters = parameters[1:] - return any( - child.type == "identifier" - and child.text.decode("utf-8", errors="replace") == receiver_name - for parameter_list in parameters - for parameter in parameter_list.children - for child in parameter.children - ) + 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)) - @staticmethod - def _go_statement_scope_binds_name( - scope, - node, - receiver_name: str, - receiver_already_declared: bool, - ) -> bool: - """Return whether an earlier declaration in ``scope`` binds the name.""" - statements = next( - (child for child in scope.children if child.type == "statement_list"), - None, + 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), ) - if statements is None: - return False - for statement in statements.children: - if statement.type == "var_declaration": - for spec in statement.children: - if ( - spec.type == "var_spec" - and spec.end_byte <= node.start_byte - and CodeParser._go_binding_binds_name(spec, receiver_name) - ): - return True - elif ( - statement.type == "short_var_declaration" - and statement.end_byte <= node.start_byte - and CodeParser._go_binding_binds_name( - statement, receiver_name, receiver_already_declared, - ) - ): - return True - return False @staticmethod - def _go_binding_binds_name( + def _go_receiver_is_shadowed( node, - receiver_name: str, - receiver_already_declared: bool = False, + bindings: tuple[str, tuple[int, ...], tuple[int, ...]], ) -> bool: - """Return whether a Go declaration's left side binds ``receiver_name``.""" - if node.type == "var_spec": - candidates = node.children - else: - if receiver_already_declared: - return False - try: - assignment = next( - index for index, child in enumerate(node.children) - if child.type == ":=" - ) - except StopIteration: - return False - candidates = node.children[:assignment] - return any( - child.type == "identifier" - and child.text.decode("utf-8", errors="replace") == receiver_name - for candidate in candidates - for child in ( - candidate.children if candidate.type == "expression_list" else (candidate,) - ) - ) + """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( diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index af2ed25dd..9cfed7973 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -44,9 +44,11 @@ func (r *InMemoryRepo) SaveAndReturn(user *User) error { 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{}) @@ -131,6 +133,46 @@ func (a *ShadowA) CallsSameScopeRedeclaration() { 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 97def36a8..8ec7c7e76 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -154,6 +154,28 @@ def test_same_scope_redeclaration_keeps_receiver_resolution(self): 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" @@ -167,13 +189,49 @@ def test_deep_receiver_scope_walk_does_not_overflow(self): ).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": - assert not self.parser._go_receiver_is_shadowed(current, "a") - return + call = current stack.extend(current.children) - pytest.fail("deep Go fixture should contain a call expression") + 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: From e4a401daf9319106b269d78960d32d1ec5c38187 Mon Sep 17 00:00:00 2001 From: MerlinH Date: Sat, 8 Aug 2026 00:00:07 +0000 Subject: [PATCH 6/6] fix(go): normalize wrapped method receivers --- code_review_graph/parser.py | 10 ++++++++++ tests/fixtures/sample_go.go | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 0e0166adb..a8397b344 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -10941,6 +10941,16 @@ def _get_member_call_receiver_method( 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"), diff --git a/tests/fixtures/sample_go.go b/tests/fixtures/sample_go.go index 9cfed7973..605772f1f 100644 --- a/tests/fixtures/sample_go.go +++ b/tests/fixtures/sample_go.go @@ -39,7 +39,7 @@ func (r *InMemoryRepo) Save(user *User) error { } func (r *InMemoryRepo) SaveAndReturn(user *User) error { - return r.Save(user) + return (*r).Save(user) } type ShadowA struct{}