diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index f681263a..c139edc2 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -591,6 +591,69 @@ def _is_test_file(path: str) -> bool: return any(p.search(path) for p in _TEST_FILE_PATTERNS) +def _extract_csharp_namespaces(root_node) -> list[str]: + """Return the list of namespaces declared in a C# compilation unit. + + C# supports two shapes: + + * Block scoped: ``namespace Foo.Bar { ... }`` + * File scoped: ``namespace Foo.Bar;`` (C# 10+) + + A single file can declare multiple namespaces, so this returns a list + in source order. Duplicates are preserved so the caller can reason + about multi-namespace files explicitly. See #310. + """ + namespaces: list[str] = [] + + def _walk(node) -> None: + # tree-sitter-c-sharp uses separate node types for the two shapes: + # * ``namespace_declaration`` (block form) + # * ``file_scoped_namespace_declaration`` (C# 10+ single-semicolon form) + if node.type in ( + "namespace_declaration", "file_scoped_namespace_declaration", + ): + for child in node.children: + if child.type in ("qualified_name", "identifier"): + text = child.text.decode("utf-8", errors="replace").strip() + if text: + namespaces.append(text) + break + for child in node.children: + _walk(child) + + _walk(root_node) + return namespaces + + +def _extract_annotation_list(node) -> list[str]: + """Return the list of decorator / annotation names on *node*. + + Walks: + * Java/Kotlin/C#: ``modifiers > annotation | marker_annotation`` child. + * Python: parent ``decorated_definition`` with ``decorator`` siblings. + + Each entry has its leading ``@`` stripped. Order follows source order so + the first annotation in the source is first in the returned list. This + function replaces the inlined logic that previously extracted decorators + only for test detection and then discarded them (see #295). + """ + deco_list: list[str] = [] + for sub in node.children: + # Java/Kotlin/C#: annotations inside a ``modifiers`` child. + if sub.type == "modifiers": + for mod in sub.children: + if mod.type in ("annotation", "marker_annotation"): + text = mod.text.decode("utf-8", errors="replace") + deco_list.append(text.lstrip("@").strip()) + # Python: check parent ``decorated_definition`` for decorator siblings. + if node.parent and node.parent.type == "decorated_definition": + for sib in node.parent.children: + if sib.type == "decorator": + text = sib.text.decode("utf-8", errors="replace") + deco_list.append(text.lstrip("@").strip()) + return deco_list + + def _is_test_function( name: str, file_path: str, decorators: tuple[str, ...] = (), ) -> bool: @@ -693,6 +756,14 @@ def parse_bytes(self, path: Path, source: bytes) -> tuple[list[NodeInfo], list[E # File node test_file = _is_test_file(file_path_str) + file_extra: dict = {} + # C#: capture every namespace this file declares so query-time + # fallbacks (importers_of, get_impact_radius) can resolve + # namespace-form IMPORTS_FROM targets back to file paths. See #310. + if language == "csharp": + ns_list = _extract_csharp_namespaces(tree.root_node) + if ns_list: + file_extra["csharp_namespaces"] = ns_list nodes.append(NodeInfo( kind="File", name=file_path_str, @@ -701,6 +772,7 @@ def parse_bytes(self, path: Path, source: bytes) -> tuple[list[NodeInfo], list[E line_end=source.count(b"\n") + 1, language=language, is_test=test_file, + extra=file_extra, )) # Pre-scan for import mappings and defined names @@ -2741,11 +2813,22 @@ def _extract_classes( if not name: return False + # Extract class-level annotations (Kotlin/Java/C#/Python). Without + # this, Kotlin `@HiltViewModel`, `@AndroidEntryPoint`, `@Composable` + # classes, Java `@Entity`/`@Service` classes, etc. would lose their + # annotation metadata. See #295. + class_decorators = _extract_annotation_list(child) + class_modifiers: Optional[str] = ( + ",".join(class_decorators) if class_decorators else None + ) + # Swift: detect the actual type keyword (class/struct/enum/actor/extension) # and store it in extra["swift_kind"] for richer downstream analysis. # Tree-sitter maps struct/enum/actor/extension all to class_declaration; # protocol uses its own protocol_declaration node type. extra: dict = {} + if class_decorators: + extra["decorators"] = list(class_decorators) if language == "swift": if child.type == "class_declaration": _swift_keywords = {"class", "struct", "enum", "actor", "extension"} @@ -2765,6 +2848,7 @@ def _extract_classes( line_end=child.end_point[0] + 1, language=language, parent_name=enclosing_class, + modifiers=class_modifiers, extra=extra, ) nodes.append(node) @@ -2829,22 +2913,16 @@ def _extract_functions( if receiver_type: enclosing_class = receiver_type - # Extract annotations/decorators for test detection + # Extract annotations/decorators for test detection AND persistence. + # Previously ``deco_list`` was used only for test detection and + # discarded — see #295 for Kotlin (``@Composable``, ``@HiltViewModel``, + # ``@Inject``) and the same pattern affects Java (``@Test``, + # ``@Autowired``), C# (``[HttpGet]``), and Python (``@app.get``). Now + # persisted into ``node.modifiers`` (comma-separated string) and + # ``node.extra["decorators"]`` (list) so consumers can filter by + # annotation (e.g. "show me all @Composable functions"). decorators: tuple[str, ...] = () - deco_list: list[str] = [] - for sub in child.children: - # Java/Kotlin/C#: annotations inside a modifiers child - if sub.type == "modifiers": - for mod in sub.children: - if mod.type in ("annotation", "marker_annotation"): - text = mod.text.decode("utf-8", errors="replace") - deco_list.append(text.lstrip("@").strip()) - # Python: check parent decorated_definition for decorator siblings - if child.parent and child.parent.type == "decorated_definition": - for sib in child.parent.children: - if sib.type == "decorator": - text = sib.text.decode("utf-8", errors="replace") - deco_list.append(text.lstrip("@").strip()) + deco_list: list[str] = _extract_annotation_list(child) if deco_list: decorators = tuple(deco_list) @@ -2854,6 +2932,13 @@ def _extract_functions( params = self._get_params(child, language, source) ret_type = self._get_return_type(child, language, source) + modifiers_str: Optional[str] = ( + ",".join(deco_list) if deco_list else None + ) + extra: dict = {} + if deco_list: + extra["decorators"] = list(deco_list) + node = NodeInfo( kind=kind, name=name, @@ -2864,7 +2949,9 @@ def _extract_functions( parent_name=enclosing_class, params=params, return_type=ret_type, + modifiers=modifiers_str, is_test=is_test, + extra=extra, ) nodes.append(node) diff --git a/code_review_graph/tools/query.py b/code_review_graph/tools/query.py index e3d8c3e1..ec6569cd 100644 --- a/code_review_graph/tools/query.py +++ b/code_review_graph/tools/query.py @@ -261,6 +261,33 @@ def query_graph( "file": e.file_path, }) edges_out.append(edge_to_dict(e)) + # C# fallback: IMPORTS_FROM edges for `using X.Y.Z;` directives + # carry the namespace string as their target, not a file path. + # For .cs files, look up every namespace the target file + # declares and re-search edges by namespace. See #310. + if node is not None and node.language == "csharp": + declared_ns: list[str] = [] + # The File node is the one whose kind == "File" in the + # same file. It carries the csharp_namespaces tag. + for n in store.get_nodes_by_file(node.file_path): + if n.kind == "File": + declared_ns = list( + n.extra.get("csharp_namespaces", []) or [] + ) + break + seen_sources = {r.get("importer") for r in results} + for ns in declared_ns: + for e in store.get_edges_by_target(ns): + if e.kind != "IMPORTS_FROM": + continue + if e.source_qualified in seen_sources: + continue + results.append({ + "importer": e.source_qualified, + "file": e.file_path, + }) + edges_out.append(edge_to_dict(e)) + seen_sources.add(e.source_qualified) elif pattern == "children_of": for e in store.get_edges_by_source(qn): diff --git a/tests/test_multilang.py b/tests/test_multilang.py index 9d45f434..dfaa3cfd 100644 --- a/tests/test_multilang.py +++ b/tests/test_multilang.py @@ -346,6 +346,121 @@ def test_finds_methods(self): assert "FindById" in names or "Save" in names +@pytest.mark.skipif( + not _has_csharp_parser(), + reason="csharp tree-sitter grammar not installed", +) +class TestCSharpNamespaceResolution: + """Regression tests for #310: C# ``using`` directives carry a namespace + string as their ``IMPORTS_FROM.target`` (e.g. ``ACME.Core``), not a + resolved file path. ``importers_of`` previously returned ``[]`` for + every .cs file; this suite locks in the namespace-fallback path. + """ + + def _write(self, path: Path, source: str) -> None: + path.write_text(source, encoding="utf-8") + + def test_file_scoped_namespace_tagged_on_file_node(self, tmp_path): + """File-scoped (C# 10+) ``namespace Foo.Bar;`` is captured.""" + src = "namespace ACME.Core;\n\npublic class TaskBoard {}\n" + f = tmp_path / "Core.cs" + self._write(f, src) + parser = CodeParser() + nodes, _ = parser.parse_file(f) + file_node = next(n for n in nodes if n.kind == "File") + assert file_node.extra.get("csharp_namespaces") == ["ACME.Core"] + + def test_block_namespace_tagged_on_file_node(self, tmp_path): + """Block-scoped ``namespace Foo.Bar { ... }`` is captured too.""" + src = ( + "namespace ACME.Core {\n" + " public class TaskBoard {}\n" + "}\n" + ) + f = tmp_path / "Core.cs" + self._write(f, src) + parser = CodeParser() + nodes, _ = parser.parse_file(f) + file_node = next(n for n in nodes if n.kind == "File") + assert file_node.extra.get("csharp_namespaces") == ["ACME.Core"] + + def test_multiple_namespaces_in_one_file(self, tmp_path): + """A single file can declare multiple namespaces; all are kept.""" + src = ( + "namespace ACME.Core {\n" + " public class A {}\n" + "}\n" + "namespace ACME.Utils {\n" + " public class B {}\n" + "}\n" + ) + f = tmp_path / "Multi.cs" + self._write(f, src) + parser = CodeParser() + nodes, _ = parser.parse_file(f) + file_node = next(n for n in nodes if n.kind == "File") + ns_list = file_node.extra.get("csharp_namespaces", []) + assert "ACME.Core" in ns_list + assert "ACME.Utils" in ns_list + + def test_non_csharp_file_has_no_namespace_tag(self, tmp_path): + """Non-C# files never carry a csharp_namespaces extra.""" + f = tmp_path / "mod.py" + self._write(f, "def foo():\n pass\n") + parser = CodeParser() + nodes, _ = parser.parse_file(f) + file_node = next(n for n in nodes if n.kind == "File") + assert "csharp_namespaces" not in file_node.extra + + def test_importers_of_resolves_namespace_to_file(self, tmp_path): + """Regression for the bug: ``importers_of `` must find + every .cs file that has ``using ;`` matching a + namespace declared by the target file.""" + (tmp_path / ".git").mkdir() + data_dir = tmp_path / ".code-review-graph" + data_dir.mkdir() + + core = tmp_path / "Core.cs" + self._write(core, ( + "namespace ACME.Core;\n" + "public class TaskBoard {}\n" + )) + app = tmp_path / "App.cs" + self._write(app, ( + "using ACME.Core;\n" + "namespace ACME.App;\n" + "public class App {}\n" + )) + unrelated = tmp_path / "Unrelated.cs" + self._write(unrelated, ( + "using System.Linq;\n" + "namespace ACME.Other;\n" + "public class Other {}\n" + )) + + from code_review_graph.graph import GraphStore + from code_review_graph.tools.query import query_graph + + store = GraphStore(data_dir / "graph.db") + parser = CodeParser() + for path in (core, app, unrelated): + nodes, edges = parser.parse_file(path) + for n in nodes: + store.upsert_node(n) + for e in edges: + store.upsert_edge(e) + store.commit() + store.close() + + result = query_graph("importers_of", str(core), repo_root=str(tmp_path)) + assert result.get("status") == "ok" + importers = {r["file"] for r in result.get("results", [])} + # App.cs imports ACME.Core, so it must be found. + assert str(app) in importers + # Unrelated.cs does NOT import ACME.Core. + assert str(unrelated) not in importers + + class TestRubyParsing: def setup_method(self): self.parser = CodeParser() @@ -438,6 +553,101 @@ def test_finds_calls(self): assert any("save" in t for t in targets) +class TestKotlinAnnotations: + """Regression tests for #295: Kotlin nodes must preserve annotation + metadata in both ``modifiers`` and ``extra['decorators']`` so consumers + can filter queries like "show me all @Composable functions". + """ + + def _parse(self, source: str, tmp_path): + p = tmp_path / "x.kt" + p.write_text(source, encoding="utf-8") + parser = CodeParser() + return parser.parse_file(p) + + def test_hilt_viewmodel_annotation_on_class(self, tmp_path): + src = ( + "package com.example\n" + "@HiltViewModel\n" + "class MyViewModel {\n" + " fun noop() {}\n" + "}\n" + ) + nodes, _ = self._parse(src, tmp_path) + classes = {n.name: n for n in nodes if n.kind == "Class"} + assert "MyViewModel" in classes + vm = classes["MyViewModel"] + assert vm.modifiers == "HiltViewModel" + assert vm.extra.get("decorators") == ["HiltViewModel"] + + def test_composable_annotation_on_function(self, tmp_path): + src = ( + "package com.example\n" + "@Composable\n" + "fun Greeting(name: String) {\n" + " println(name)\n" + "}\n" + ) + nodes, _ = self._parse(src, tmp_path) + funcs = {n.name: n for n in nodes if n.kind == "Function"} + assert "Greeting" in funcs + greet = funcs["Greeting"] + assert greet.modifiers == "Composable" + assert greet.extra.get("decorators") == ["Composable"] + + def test_multiple_annotations_on_function(self, tmp_path): + src = ( + "package com.example\n" + "class Container {\n" + " @Inject\n" + " @field:JvmField\n" + " fun install() {}\n" + "}\n" + ) + nodes, _ = self._parse(src, tmp_path) + funcs = {n.name: n for n in nodes if n.kind == "Function"} + assert "install" in funcs + install = funcs["install"] + assert install.extra.get("decorators") is not None + # Both annotations preserved. + decs = install.extra["decorators"] + assert any("Inject" in d for d in decs) + assert any("JvmField" in d for d in decs) + + def test_unannotated_function_has_none_modifiers(self, tmp_path): + """Regression guard: adding annotation support must not leak a + default empty string — unannotated nodes must still have + ``modifiers=None`` and no ``decorators`` key.""" + src = ( + "package com.example\n" + "fun bare() { println(1) }\n" + ) + nodes, _ = self._parse(src, tmp_path) + funcs = {n.name: n for n in nodes if n.kind == "Function"} + assert "bare" in funcs + bare = funcs["bare"] + assert bare.modifiers is None + assert "decorators" not in bare.extra + + def test_test_annotation_still_triggers_test_kind(self, tmp_path): + """Adding annotation persistence must not break the pre-existing + behavior of ``@Test`` promoting the node to kind='Test'.""" + src = ( + "package com.example\n" + "class MyTests {\n" + " @Test\n" + " fun testSomething() { println(42) }\n" + "}\n" + ) + nodes, _ = self._parse(src, tmp_path) + tests = [n for n in nodes if n.kind == "Test"] + names = {t.name for t in tests} + assert "testSomething" in names + # And the annotation metadata is still preserved. + t = next(n for n in tests if n.name == "testSomething") + assert t.extra.get("decorators") == ["Test"] + + class TestSwiftParsing: def setup_method(self): self.parser = CodeParser()