diff --git a/code_review_graph/main.py b/code_review_graph/main.py index 3cc929c8..b6275236 100644 --- a/code_review_graph/main.py +++ b/code_review_graph/main.py @@ -944,6 +944,7 @@ def _apply_tool_filter(tools: str | None = None) -> None: # via env var CRG_TOOLS=query_graph_tool,semantic_search_nodes_tool """ + import asyncio import os raw = tools or os.environ.get("CRG_TOOLS") @@ -952,10 +953,31 @@ def _apply_tool_filter(tools: str | None = None) -> None: allowed = {t.strip() for t in raw.split(",") if t.strip()} if not allowed: return - registered = list(mcp._tool_manager._tools.keys()) - for name in registered: + # FastMCP >=3 exposes tool enumeration via the async ``list_tools`` + # method. ``_apply_tool_filter`` is typically called from + # ``main()`` before the MCP event loop starts, but tests may invoke + # it from within a running event loop — in that case ``asyncio.run`` + # raises ``RuntimeError``. Fall back to running the coroutine on a + # dedicated short-lived loop in a worker thread. Earlier code path + # relied on ``mcp._tool_manager._tools`` which is a private + # attribute that was removed in fastmcp>=3.0. + def _list_tool_names() -> list[str]: + coro_factory = mcp.list_tools + try: + asyncio.get_running_loop() + except RuntimeError: + return [t.name for t in asyncio.run(coro_factory())] + import concurrent.futures + + def _runner() -> list[str]: + return [t.name for t in asyncio.run(coro_factory())] + + with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: + return pool.submit(_runner).result() + + for name in _list_tool_names(): if name not in allowed: - mcp.remove_tool(name) + mcp.local_provider.remove_tool(name) diff --git a/code_review_graph/parser.py b/code_review_graph/parser.py index 7cba96d6..508f3620 100644 --- a/code_review_graph/parser.py +++ b/code_review_graph/parser.py @@ -128,6 +128,41 @@ class EdgeInfo: ".gd": "gdscript", } +# Shebang interpreter → language mapping for extension-less Unix scripts. +# Each key is the **basename** of the interpreter path as it appears after +# ``#!`` (or after ``#!/usr/bin/env``). Only languages already registered +# above are listed — this file strictly routes extension-less scripts, it +# does NOT introduce new languages on its own. See issue #237. +SHEBANG_INTERPRETER_TO_LANGUAGE: dict[str, str] = { + # POSIX / bash-compatible shells — all routed through tree-sitter-bash + "bash": "bash", + "sh": "bash", + "zsh": "bash", + "ksh": "bash", + "dash": "bash", + "ash": "bash", + # Python (every common variant) + "python": "python", + "python2": "python", + "python3": "python", + "pypy": "python", + "pypy3": "python", + # JavaScript via Node + "node": "javascript", + "nodejs": "javascript", + # Ruby / Perl / Lua / R / PHP + "ruby": "ruby", + "perl": "perl", + "lua": "lua", + "Rscript": "r", + "php": "php", +} + +# Maximum bytes to read from the head of a file when probing for a shebang. +# 256 is enough for any reasonable shebang line (``#!/usr/bin/env python3 -u\n`` +# is ~30 chars) while keeping the worst-case read tiny even on fat binaries. +_SHEBANG_PROBE_BYTES = 256 + # Tree-sitter node type mappings per language # Maps (language) -> dict of semantic role -> list of TS node types _CLASS_TYPES: dict[str, list[str]] = { @@ -172,7 +207,12 @@ class EdgeInfo: "elixir": [], "zig": ["container_declaration"], "powershell": ["class_statement"], - "julia": ["struct_definition", "abstract_definition"], + "julia": [ + "struct_definition", "abstract_definition", "module_definition", + ], + # GDScript: inner classes use ``class Name:`` (class_definition); the + # file-level ``class_name Name`` gives the script itself an identity. + "gdscript": ["class_definition", "class_name_statement"], } _FUNCTION_TYPES: dict[str, list[str]] = { @@ -219,10 +259,15 @@ class EdgeInfo: "elixir": [], "zig": ["fn_proto", "fn_decl"], "powershell": ["function_statement"], + # Julia: short-form functions `f(x) = expr` parse as `assignment` nodes + # (not a dedicated definition node) and are handled in + # _extract_julia_constructs. "julia": [ "function_definition", - "short_function_definition", + "macro_definition", ], + # GDScript: ``func name(args) -> ReturnType:`` — includes ``static func``. + "gdscript": ["function_definition"], } _IMPORT_TYPES: dict[str, list[str]] = { @@ -263,6 +308,11 @@ class EdgeInfo: "powershell": [], # Julia: import/using are import_statement nodes. "julia": ["import_statement", "using_statement"], + # GDScript has no ``import`` keyword. The closest analogue is + # ``extends OtherClass`` / ``extends "res://path.gd"``, which establishes + # a hard dependency on the parent script. preload()/load() calls remain + # as ordinary CALLS edges. + "gdscript": ["extends_statement"], } _CALL_TYPES: dict[str, list[str]] = { @@ -284,7 +334,12 @@ class EdgeInfo: ], "kotlin": ["call_expression"], "swift": ["call_expression"], - "php": ["function_call_expression", "member_call_expression"], + "php": [ + "function_call_expression", + "member_call_expression", + "scoped_call_expression", + "nullsafe_member_call_expression", + ], "scala": ["call_expression", "instance_expression", "generic_function"], "solidity": ["call_expression"], "lua": ["function_call"], @@ -300,7 +355,14 @@ class EdgeInfo: "elixir": [], "zig": ["call_expression", "builtin_call_expr"], "powershell": ["command_expression"], - "julia": ["call_expression"], + "julia": [ + "call_expression", + "broadcast_call_expression", + "macrocall_expression", + ], + # GDScript: bare calls produce ``call``; ``obj.method()`` is an + # ``attribute`` node whose right-hand side is an ``attribute_call``. + "gdscript": ["call", "attribute_call"], } # Patterns that indicate a test function @@ -327,6 +389,8 @@ class EdgeInfo: re.compile(r".*Test\.java$"), re.compile(r".*_test\.resi?$"), re.compile(r".*\.test\.resi?$"), + re.compile(r"test/runtests\.jl$"), + re.compile(r"test/.*\.jl$"), ] _TEST_RUNNER_NAMES = frozenset({ @@ -687,7 +751,88 @@ def _get_parser(self, language: str): # type: ignore[arg-type] return self._parsers[language] def detect_language(self, path: Path) -> Optional[str]: - return EXTENSION_TO_LANGUAGE.get(path.suffix.lower()) + """Map a file path to its language name. + + Extension-based lookup is tried first. For extension-less files + (typical for Unix scripts like ``bin/myapp`` or ``.git/hooks/pre-commit``) + we fall back to reading the first line for a shebang. Files that + already have a known extension are never re-read — shebang probing + only runs when the extension lookup returns ``None`` **and** the path + has no suffix at all. See issue #237. + """ + suffix = path.suffix.lower() + lang = EXTENSION_TO_LANGUAGE.get(suffix) + if lang is not None: + return lang + # Only probe shebang for files without any extension — "README", "LICENSE", + # and other extension-less text files also fall here, but the probe is a + # cheap 256-byte read that returns None when no shebang is found. + if suffix == "": + return self._detect_language_from_shebang(path) + return None + + @staticmethod + def _detect_language_from_shebang(path: Path) -> Optional[str]: + """Inspect the first line of ``path`` for a shebang interpreter. + + Returns the mapped language name or ``None`` if the file has no + shebang, is unreadable, or names an interpreter we don't map. + + Accepted shapes:: + + #!/bin/bash + #!/usr/bin/env python3 + #!/usr/bin/env -S node --experimental-vm-modules + #!/usr/bin/bash -e + + Only the basename of the interpreter is consulted. Trailing flags + after the interpreter are ignored. Windows-style ``\r\n`` line + endings are handled. Binary files read as garbage bytes simply + fail the ``#!`` prefix check and return ``None``. + """ + try: + with path.open("rb") as fh: + head = fh.read(_SHEBANG_PROBE_BYTES) + except (OSError, PermissionError): + return None + if not head.startswith(b"#!"): + return None + + # Take just the first line, stripped of leading "#!" and any + # surrounding whitespace. Split on NUL to defend against accidental + # binary content following a ``#!`` prefix. + first_line = head.split(b"\n", 1)[0].split(b"\0", 1)[0] + try: + line = first_line[2:].decode("utf-8", errors="strict").strip() + except UnicodeDecodeError: + return None + if not line: + return None + + tokens = line.split() + if not tokens: + return None + + first = tokens[0] + # `/usr/bin/env` indirection: the interpreter is the next token. + # `/usr/bin/env -S node --flag` is also valid — skip any leading + # ``-`` options after env. + if first.endswith("/env") or first == "env": + interpreter_token: Optional[str] = None + for tok in tokens[1:]: + if tok.startswith("-"): + # ``-S`` takes no argument in most envs; skip and continue. + continue + interpreter_token = tok + break + if interpreter_token is None: + return None + interpreter = interpreter_token.rsplit("/", 1)[-1] + else: + # Direct form: ``#!/bin/bash`` or ``#!/usr/local/bin/python3``. + interpreter = first.rsplit("/", 1)[-1] + + return SHEBANG_INTERPRETER_TO_LANGUAGE.get(interpreter) def parse_file(self, path: Path) -> tuple[list[NodeInfo], list[EdgeInfo]]: """Parse a single file and return extracted nodes and edges.""" @@ -719,11 +864,20 @@ def parse_bytes(self, path: Path, source: bytes) -> tuple[list[NodeInfo], list[E if language == "notebook": return self._parse_notebook(path, source) - # Databricks .py notebook exports - if language == "python" and source.startswith( - b"# Databricks notebook source\n", - ): - return self._parse_databricks_py_notebook(path, source) + # Databricks .py notebook exports. The header is ALWAYS the very + # first line, but the file may have CRLF line endings on Windows + # (git's core.autocrlf=true default). Match the first line robustly + # after stripping any trailing ``\r`` so the detection works on both + # platforms. See issue #239. + if language == "python": + first_newline = source.find(b"\n") + first_line = ( + source[:first_newline].rstrip(b"\r") + if first_newline != -1 + else source.rstrip(b"\r") + ) + if first_line == b"# Databricks notebook source": + return self._parse_databricks_py_notebook(path, source) # ReScript: regex-based parser (no tree-sitter grammar bundled). if language == "rescript": @@ -1844,6 +1998,19 @@ def _extract_from_tree( ): continue + # --- Julia-specific constructs --- + # Short-form functions (`f(x) = expr`) parse as ``assignment``, + # ``include("file.jl")`` as a call_expression, exports as + # ``export_statement``, and macrocalls (including ``@testset``) + # need recursion into bodies that may themselves contain + # function definitions (e.g. ``@inline function f ... end``). + if language == "julia" and self._extract_julia_constructs( + child, node_type, source, language, file_path, + nodes, edges, enclosing_class, enclosing_func, + import_map, defined_names, _depth, + ): + continue + # --- Dart call detection (see #87) --- # tree-sitter-dart does not wrap calls in a single # ``call_expression`` node; instead the pattern is @@ -1892,7 +2059,7 @@ def _extract_from_tree( if node_type in func_types and self._extract_functions( child, source, language, file_path, nodes, edges, enclosing_class, import_map, defined_names, - _depth, + _depth, enclosing_func, ): continue @@ -2142,26 +2309,27 @@ def _extract_elixir_constructs( return True # ---- Everything else = a regular function/method call ---------- - # Emit a CALLS edge when we're inside a function (same rule as - # the generic _extract_calls path). - if enclosing_func: - # For dotted calls like `IO.puts(msg)`, prefer the dotted - # identifier; for bare calls use the first identifier. - call_name = ident - caller = self._qualify( - enclosing_func, file_path, enclosing_class, - ) - target = self._resolve_call_target( - call_name, file_path, language, - import_map or {}, defined_names or set(), - ) - edges.append(EdgeInfo( - kind="CALLS", - source=caller, - target=target, - file_path=file_path, - line=node.start_point[0] + 1, - )) + # Module-scope calls attribute to the File node (same rule as the + # generic _extract_calls path). + # For dotted calls like `IO.puts(msg)`, prefer the dotted + # identifier; for bare calls use the first identifier. + call_name = ident + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) + target = self._resolve_call_target( + call_name, file_path, language, + import_map or {}, defined_names or set(), + ) + edges.append(EdgeInfo( + kind="CALLS", + source=caller, + target=target, + file_path=file_path, + line=node.start_point[0] + 1, + )) # Recurse into arguments + do_block so nested calls are caught. for sub in node.children: if sub.type in ("arguments", "do_block"): @@ -2329,6 +2497,346 @@ def _extract_r_constructs( return False + # ------------------------------------------------------------------ + # Julia-specific helpers + # ------------------------------------------------------------------ + + def _julia_short_func_name(self, call_expr) -> Optional[str]: + """Extract the name from a ``call_expression`` that is the LHS of + a short-form function ``f(x) = expr`` or ``Base.f(x) = expr`` or + ``Foo{T}(x) = expr``. + """ + for child in call_expr.children: + if child.type == "identifier": + return child.text.decode("utf-8", errors="replace") + if child.type == "field_expression": + for ident in reversed(child.children): + if ident.type == "identifier": + return ident.text.decode("utf-8", errors="replace") + return None + if child.type == "parametrized_type_expression": + for ident in child.children: + if ident.type == "identifier": + return ident.text.decode("utf-8", errors="replace") + return None + return None + + def _julia_string_arg(self, call_expr) -> Optional[str]: + """Return the first string literal argument of a call_expression.""" + for child in call_expr.children: + if child.type != "argument_list": + continue + for arg in child.children: + if arg.type == "string_literal": + for sub in arg.children: + if sub.type == "content": + return sub.text.decode("utf-8", errors="replace") + raw = arg.text.decode("utf-8", errors="replace") + return raw.strip('"').strip("'") + return None + + def _julia_call_first_identifier(self, call_expr) -> Optional[str]: + """First identifier of a ``call_expression`` (the function being + called). Used to detect ``include("...")``. + """ + for child in call_expr.children: + if child.type == "identifier": + return child.text.decode("utf-8", errors="replace") + return None + + def _extract_julia_constructs( + self, + child, + node_type: str, + source: bytes, + language: str, + file_path: str, + nodes: list[NodeInfo], + edges: list[EdgeInfo], + enclosing_class: Optional[str], + enclosing_func: Optional[str], + import_map: Optional[dict[str, str]], + defined_names: Optional[set[str]], + _depth: int, + ) -> bool: + """Handle Julia-specific constructs the type tables can't cover. + + Returns True if the child was fully handled and should be skipped + by the main dispatch loop. + """ + # --- Short-form function: assignment with call_expression LHS --- + # ``f(x) = expr`` or ``Base.f(x) = expr``. Anything else with an + # ``=`` (plain variable, const) is left to the generic path. + if node_type == "assignment": + lhs = child.children[0] if child.children else None + # Unwrap typed LHS: ``f(x)::RetT = expr`` parses as + # ``assignment > typed_expression > call_expression``. + if lhs is not None and lhs.type == "typed_expression": + for sub in lhs.children: + if sub.type == "call_expression": + lhs = sub + break + if lhs is not None and lhs.type == "call_expression": + name = self._julia_short_func_name(lhs) + if name: + is_test = _is_test_function(name, file_path, ()) + kind = "Test" if is_test else "Function" + qualified = self._qualify( + name, file_path, enclosing_class, + ) + nodes.append(NodeInfo( + kind=kind, + name=name, + file_path=file_path, + line_start=child.start_point[0] + 1, + line_end=child.end_point[0] + 1, + language=language, + parent_name=enclosing_class, + is_test=is_test, + )) + container = ( + self._qualify(enclosing_class, file_path, None) + if enclosing_class + else file_path + ) + edges.append(EdgeInfo( + kind="CONTAINS", + source=container, + target=qualified, + file_path=file_path, + line=child.start_point[0] + 1, + )) + # Recurse into the RHS only (children after the ``=`` + # operator) with this function as the enclosing scope + # so internal calls wire up correctly. Visiting the + # whole assignment would re-treat the LHS + # ``call_expression`` as a self-call. + seen_op = False + for sub in child.children: + if not seen_op: + if sub.type == "operator": + seen_op = True + continue + self._extract_from_tree( + sub, source, language, file_path, nodes, edges, + enclosing_class=enclosing_class, + enclosing_func=name, + import_map=import_map, + defined_names=defined_names, + _depth=_depth + 1, + ) + return True + + # --- Skip call_expression nodes that are actually function + # signatures (``function foo(x) ... end`` has a ``signature > + # call_expression`` that describes the definition, not a call). + if node_type == "call_expression": + parent = child.parent + if parent is not None and parent.type == "signature": + return True + + # --- include("file.jl") -> IMPORTS_FROM edge --- + if node_type == "call_expression": + if self._julia_call_first_identifier(child) == "include": + path_arg = self._julia_string_arg(child) + if path_arg: + resolved = self._resolve_module_to_file( + path_arg, file_path, language, + ) + edges.append(EdgeInfo( + kind="IMPORTS_FROM", + source=file_path, + target=resolved if resolved else path_arg, + file_path=file_path, + line=child.start_point[0] + 1, + )) + # Fall through - let generic call dispatch also record + # the CALLS edge and recurse for nested calls. + return False + + # --- export_statement / public_statement -> REFERENCES edges --- + # ``public`` (1.11+) is a softer variant of ``export`` — symbols + # are part of the public API but not brought into scope by + # ``using``. Track both so review tools can answer "what's the + # public surface of this module?". + if node_type in ("export_statement", "public_statement"): + source_qual = ( + self._qualify(enclosing_class, file_path, None) + if enclosing_class + else file_path + ) + marker = ( + "julia_export" + if node_type == "export_statement" + else "julia_public" + ) + for sub in child.children: + if sub.type == "identifier": + name = sub.text.decode("utf-8", errors="replace") + edges.append(EdgeInfo( + kind="REFERENCES", + source=source_qual, + target=name, + file_path=file_path, + line=child.start_point[0] + 1, + extra={marker: True}, + )) + return True + + # --- macrocall_expression --- + if node_type == "macrocall_expression": + macro_name = None + for sub in child.children: + if sub.type == "macro_identifier": + for ident in sub.children: + if ident.type == "identifier": + macro_name = ident.text.decode( + "utf-8", errors="replace", + ) + break + break + + if macro_name == "enum": + # @enum Color RED BLUE GREEN + # First argument is the enum type name; the rest are + # variant names. Model the type as a Class and each + # variant as a Function child, so callers referencing a + # variant resolve to something in the graph. + type_name: Optional[str] = None + variant_identifiers: list = [] + for sub in child.children: + if sub.type != "macro_argument_list": + continue + for arg in sub.children: + if arg.type != "identifier": + continue + if type_name is None: + type_name = arg.text.decode( + "utf-8", errors="replace", + ) + else: + variant_identifiers.append(arg) + break + if type_name: + line_start = child.start_point[0] + 1 + line_end = child.end_point[0] + 1 + qualified_type = self._qualify( + type_name, file_path, enclosing_class, + ) + nodes.append(NodeInfo( + kind="Class", + name=type_name, + file_path=file_path, + line_start=line_start, + line_end=line_end, + language=language, + parent_name=enclosing_class, + extra={"julia_kind": "enum"}, + )) + container = ( + self._qualify(enclosing_class, file_path, None) + if enclosing_class + else file_path + ) + edges.append(EdgeInfo( + kind="CONTAINS", + source=container, + target=qualified_type, + file_path=file_path, + line=line_start, + )) + for variant in variant_identifiers: + vname = variant.text.decode( + "utf-8", errors="replace", + ) + qualified_v = self._qualify( + vname, file_path, type_name, + ) + nodes.append(NodeInfo( + kind="Function", + name=vname, + file_path=file_path, + line_start=variant.start_point[0] + 1, + line_end=variant.end_point[0] + 1, + language=language, + parent_name=type_name, + extra={"julia_kind": "enum_variant"}, + )) + edges.append(EdgeInfo( + kind="CONTAINS", + source=qualified_type, + target=qualified_v, + file_path=file_path, + line=variant.start_point[0] + 1, + )) + return True + + if macro_name == "testset": + # @testset "desc" begin ... end + desc = None + body_parent = None + for sub in child.children: + if sub.type != "macro_argument_list": + continue + body_parent = sub + for arg in sub.children: + if arg.type == "string_literal": + for c in arg.children: + if c.type == "content": + desc = c.text.decode( + "utf-8", errors="replace", + ) + break + break + line_no = child.start_point[0] + 1 + synth_base = f"testset:{desc}" if desc else "testset" + synth_name = f"{synth_base}@L{line_no}" + qualified = self._qualify( + synth_name, file_path, enclosing_class, + ) + nodes.append(NodeInfo( + kind="Test", + name=synth_name, + file_path=file_path, + line_start=child.start_point[0] + 1, + line_end=child.end_point[0] + 1, + language=language, + parent_name=enclosing_class, + is_test=True, + )) + container = ( + self._qualify( + enclosing_func, file_path, enclosing_class, + ) + if enclosing_func + else file_path + ) + edges.append(EdgeInfo( + kind="CONTAINS", + source=container, + target=qualified, + file_path=file_path, + line=child.start_point[0] + 1, + )) + if body_parent is not None: + self._extract_from_tree( + body_parent, source, language, file_path, nodes, edges, + enclosing_class=enclosing_class, + enclosing_func=synth_name, + import_map=import_map, defined_names=defined_names, + _depth=_depth + 1, + ) + return True + + # Other macrocalls: let the generic CALLS path emit the edge, + # but also recurse into the macro_argument_list so that any + # function defs nested under @inline / @generated / etc. get + # captured. We return False so the generic dispatcher still + # runs for the CALLS edge. + return False + + return False + # ------------------------------------------------------------------ # Lua-specific helpers # ------------------------------------------------------------------ @@ -3267,6 +3775,7 @@ def _extract_functions( import_map: Optional[dict[str, str]], defined_names: Optional[set[str]], _depth: int, + enclosing_func: Optional[str] = None, ) -> bool: """Extract a function/method definition node. @@ -3305,7 +3814,17 @@ def _extract_functions( is_test = _is_test_function(name, file_path, decorators) kind = "Test" if is_test else "Function" - qualified = self._qualify(name, file_path, enclosing_class) + + # Julia: nested functions (``function inner`` inside another + # ``function outer``) should wire up to their enclosing function, + # not skip past it to the enclosing class/module. + parent_name = enclosing_class + container_scope = enclosing_class + if language == "julia" and enclosing_func: + parent_name = enclosing_func + container_scope = enclosing_func + + qualified = self._qualify(name, file_path, parent_name) params = self._get_params(child, language, source) ret_type = self._get_return_type(child, language, source) @@ -3330,7 +3849,7 @@ def _extract_functions( line_start=child.start_point[0] + 1, line_end=child.end_point[0] + 1, language=language, - parent_name=enclosing_class, + parent_name=parent_name, params=params, return_type=ret_type, is_test=is_test, @@ -3340,8 +3859,8 @@ def _extract_functions( # CONTAINS edge container = ( - self._qualify(enclosing_class, file_path, None) - if enclosing_class + self._qualify(container_scope, file_path, None) + if container_scope else file_path ) edges.append(EdgeInfo( @@ -3352,6 +3871,58 @@ def _extract_functions( line=child.start_point[0] + 1, )) + # Julia: ``function Base.show(io, x)`` extends a foreign module's + # method. Record a REFERENCES edge from the function to the + # qualifier module so cross-module links stay visible even though + # the function's local name is just the method name. + if language == "julia" and child.type == "function_definition": + for sub in child.children: + if sub.type != "signature": + continue + call_expr = None + scope = sub + # Peel where_expression / typed_expression wrappers so we + # land on the inner call_expression regardless of + # ``func(x) where T`` or ``func(x)::T`` sugar. + for _ in range(2): + found_wrapper = False + for inner in scope.children: + if inner.type in ( + "where_expression", "typed_expression", + ): + scope = inner + found_wrapper = True + break + if not found_wrapper: + break + for inner in scope.children: + if inner.type == "call_expression": + call_expr = inner + break + if call_expr is None: + break + if call_expr.children and call_expr.children[0].type == "field_expression": + field_expr = call_expr.children[0] + parts: list[str] = [] + for ident in field_expr.children: + if ident.type == "identifier": + parts.append( + ident.text.decode("utf-8", errors="replace"), + ) + # Module qualifier = everything except the final method + # name. + if len(parts) >= 2: + qualifier = ".".join(parts[:-1]) + edges.append(EdgeInfo( + kind="REFERENCES", + source=qualified, + target=qualifier, + file_path=file_path, + line=child.start_point[0] + 1, + extra={"julia_qualified_def": True}, + )) + break + # Solidity: modifier invocations on functions -> CALLS edges if language == "solidity": for sub in child.children: @@ -3491,9 +4062,17 @@ def _extract_calls( ) return True - if call_name and enclosing_func: - caller = self._qualify( - enclosing_func, file_path, enclosing_class, + if call_name: + # Module-scope calls (no enclosing function) are attributed to + # the File node. Matches the existing convention for CONTAINS + # edges and _extract_value_references. Without this fallback, + # any function called only from top-level script glue, CLI + # entrypoints, or Jupyter/Databricks notebook cells is flagged + # as dead by find_dead_code. + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path ) # Java method_invocation: extract actual method name and receiver @@ -3586,17 +4165,21 @@ def _extract_jsx_component_call( Treat uppercase component tags such as ```` as call-like edges so caller/impact queries can cross the JSX boundary. Intrinsic DOM tags (``
``) are ignored. - """ - if not enclosing_func: - return + Module-scope JSX (e.g. a top-level ```` render call) attributes + to the File node. + """ target = self._resolve_jsx_component_target( child, language, file_path, import_map or {}, defined_names or set(), ) if not target: return - caller = self._qualify(enclosing_func, file_path, enclosing_class) + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) edges.append(EdgeInfo( kind="CALLS", source=caller, @@ -3882,15 +4465,20 @@ def _extract_solidity_constructs( Returns True if the child was fully handled and should skip default recursion. """ - # Emit statements: emit EventName(...) -> CALLS edge - if node_type == "emit_statement" and enclosing_func: + # Emit statements: emit EventName(...) -> CALLS edge. + # Module-scope emits attribute to the File node. + if node_type == "emit_statement": for sub in child.children: if sub.type == "expression": for ident in sub.children: if ident.type == "identifier": - caller = self._qualify( - enclosing_func, file_path, - enclosing_class, + caller = ( + self._qualify( + enclosing_func, file_path, + enclosing_class, + ) + if enclosing_func + else file_path ) edges.append(EdgeInfo( kind="CALLS", @@ -4288,6 +4876,38 @@ def _do_resolve_module( # ``dart:core`` / ``dart:async`` etc. are SDK libraries we do # not track; fall through to return None. + elif language == "java": + # ``import com.example.pkg.ClassName;`` — convert dot-notation + # to a relative path and walk up from the caller's directory to + # find the source root. Wildcards (``import pkg.*``) and static + # member imports (``import static pkg.Class.member``) that don't + # resolve as-is are retried after dropping the last segment + # (the member name). + if module.endswith(".*"): + return None # wildcard import — can't resolve to one file + rel_path = module.replace(".", "/") + ".java" + current = caller_dir + while True: + target = current / rel_path + if target.is_file(): + return str(target.resolve()) + if current == current.parent: + break + current = current.parent + # Static import: ``pkg.Class.member`` — strip member, try again + dot = module.rfind(".") + if dot > 0: + class_module = module[:dot] + rel_path2 = class_module.replace(".", "/") + ".java" + current = caller_dir + while True: + target = current / rel_path2 + if target.is_file(): + return str(target.resolve()) + if current == current.parent: + break + current = current.parent + return None def _find_dart_pubspec_root( @@ -4531,6 +5151,45 @@ def _get_name(self, node, language: str, kind: str) -> Optional[str]: result = self._get_name(child, language, kind) if result: return result + # C++: inside function_declarator, the name appears as + # qualified_identifier (Class::method), destructor_name (~Class), + # operator_name (operator==), or field_identifier. The generic + # loop below only recognizes 'identifier'/'type_identifier', + # so scoped method definitions would otherwise fall through and + # match the outer return-type type_identifier as the function name. + # Nested scopes (Outer::Inner::method) produce nested + # qualified_identifier nodes — peel until we find the leaf name. + if language == "cpp" and node.type == "function_declarator": + def _leaf_name(qi): + # Walk right-to-left: the rightmost identifier/ + # destructor_name/operator_name is the method name. + # If the rightmost child is itself a qualified_identifier + # (nested scope), recurse into it. + for sub in reversed(qi.children): + if sub.type in ( + "identifier", + "destructor_name", + "operator_name", + ): + return sub.text.decode( + "utf-8", errors="replace") + if sub.type == "qualified_identifier": + inner = _leaf_name(sub) + if inner: + return inner + return None + for child in node.children: + if child.type == "qualified_identifier": + name = _leaf_name(child) + if name: + return name + if child.type in ( + "field_identifier", + "destructor_name", + "operator_name", + ): + return child.text.decode( + "utf-8", errors="replace") # Objective-C method_definition: the method name is the first # ``identifier`` child (first part of the selector). Multi-part @@ -4555,6 +5214,16 @@ def _get_name(self, node, language: str, kind: str) -> Optional[str]: for child in node.children: if child.type == "field_identifier": return child.text.decode("utf-8", errors="replace") + # Java methods: tree-sitter-java puts type_identifier or generic_type + # (return type) before identifier (method name). Must run before + # the generic loop, which would match the return type's + # type_identifier (e.g. "String", "ConfigBean"). + # Constructors are fine — they have no return type node. + # Kotlin is unaffected: its syntax places the name before the type. + if language == "java" and node.type == "method_declaration": + for child in node.children: + if child.type == "identifier": + return child.text.decode("utf-8", errors="replace") # Swift extensions: name is inside user_type > type_identifier # (e.g. `extension MyClass: Protocol { ... }`) if language == "swift" and node.type == "class_declaration": @@ -4563,6 +5232,89 @@ def _get_name(self, node, language: str, kind: str) -> Optional[str]: for sub in child.children: if sub.type == "type_identifier": return sub.text.decode("utf-8", errors="replace") + # Julia: functions / macros nest the name inside + # ``signature > call_expression > identifier``. Qualified names + # (``function Base.show``) store the method name as the last + # identifier of a ``field_expression``. ``where`` clauses wrap the + # call in a ``where_expression``. + # Structs and abstract types put the name inside ``type_head``, + # possibly wrapped in ``binary_expression`` (``<:``) or + # ``parametrized_type_expression`` (``{T}``). + if language == "julia": + if node.type in ("function_definition", "macro_definition"): + for child in node.children: + if child.type == "signature": + call = child + # Unwrap where_expression: signature > where_expression > call_expression + for sub in call.children: + if sub.type == "where_expression": + call = sub + break + # Unwrap typed_expression: signature > typed_expression > call_expression + # (``function foo(x)::ReturnType``) + for sub in call.children: + if sub.type == "typed_expression": + call = sub + break + for sub in call.children: + if sub.type == "call_expression": + for target in sub.children: + if target.type == "identifier": + return target.text.decode( + "utf-8", errors="replace", + ) + if target.type == "field_expression": + # Qualified: last identifier is method name + for ident in reversed(target.children): + if ident.type == "identifier": + return ident.text.decode( + "utf-8", errors="replace", + ) + if target.type == "parametrized_type_expression": + # Parametric constructor: Foo{T}(x) = ... + for p in target.children: + if p.type == "identifier": + return p.text.decode( + "utf-8", errors="replace", + ) + return None + return None + if node.type in ("struct_definition", "abstract_definition"): + for child in node.children: + if child.type == "type_head": + # Direct identifier: struct Foo ... end + for sub in child.children: + if sub.type == "identifier": + return sub.text.decode( + "utf-8", errors="replace", + ) + # Subtyped: type_head > binary_expression > identifier (first) + for sub in child.children: + if sub.type == "binary_expression": + for ident in sub.children: + if ident.type == "identifier": + return ident.text.decode( + "utf-8", errors="replace", + ) + if ident.type == "parametrized_type_expression": + for p in ident.children: + if p.type == "identifier": + return p.text.decode( + "utf-8", errors="replace", + ) + return None + return None + # Parametric (no <:): type_head > parametrized_type_expression + for sub in child.children: + if sub.type == "parametrized_type_expression": + for p in sub.children: + if p.type == "identifier": + return p.text.decode( + "utf-8", errors="replace", + ) + return None + return None + # Most languages use a 'name' child. # field_identifier covers C++ class member function names inside # function_declarator (e.g. virtual std::string get_name() = 0). @@ -4650,7 +5402,23 @@ def _get_bases(self, node, language: str, source: bytes) -> list[str]: for arg in child.children: if arg.type in ("identifier", "attribute"): bases.append(arg.text.decode("utf-8", errors="replace")) - elif language in ("java", "csharp", "kotlin"): + elif language == "java": + # Java: superclass and super_interfaces wrap the keyword + # (extends/implements) around type_identifier children. + # Taking .text would include the keyword (e.g. "implements Foo"). + # Drill into the children to extract bare type names. + for child in node.children: + if child.type == "superclass": + for sub in child.children: + if sub.type in ("type_identifier", "generic_type"): + bases.append(sub.text.decode("utf-8", errors="replace")) + elif child.type == "super_interfaces": + for sub in child.children: + if sub.type == "type_list": + for ident in sub.children: + if ident.type in ("type_identifier", "generic_type"): + bases.append(ident.text.decode("utf-8", errors="replace")) + elif language in ("csharp", "kotlin"): # Look for superclass/interfaces in extends/implements clauses for child in node.children: if child.type in ( @@ -4737,6 +5505,43 @@ def _get_bases(self, node, language: str, source: bytes) -> list[str]: ident.text.decode("utf-8", errors="replace") ) break + elif language == "julia": + # Julia: struct Foo <: Bar / abstract type Foo <: Bar end + # AST: type_head > binary_expression with operator "<:" and + # identifier children; the identifier AFTER the operator is the + # supertype. + if node.type in ("struct_definition", "abstract_definition"): + for child in node.children: + if child.type != "type_head": + continue + for sub in child.children: + if sub.type != "binary_expression": + continue + has_subtype_op = False + for op_child in sub.children: + if ( + op_child.type == "operator" + and op_child.text == b"<:" + ): + has_subtype_op = True + break + if not has_subtype_op: + continue + idents = [ + c for c in sub.children if c.type == "identifier" + ] + # First identifier is the type being defined; the + # second (if present) is the supertype. + if len(idents) >= 2: + bases.append( + idents[1].text.decode("utf-8", errors="replace"), + ) + elif len(idents) == 1: + # Could be `Parametric{T} <: Super` where the + # first side is parametrized_type_expression. + bases.append( + idents[0].text.decode("utf-8", errors="replace"), + ) return bases def _extract_import(self, node, language: str, source: bytes) -> list[str]: @@ -4850,6 +5655,70 @@ def _find_string_literal(n) -> Optional[str]: val = _find_string_literal(node) if val: imports.append(val) + elif language == "julia": + # using/import statements. Children can be: + # - identifier (simple: `using Foo`) + # - import_path (dotted: `using Foo.Bar`) + # - selected_import (`using Foo: bar, baz` — first child is the + # module as identifier/import_path, remaining identifiers after + # the ':' are imported names to record as ``Module.name``) + def _import_path_text(n) -> str: + parts: list[str] = [] + for sub in n.children: + if sub.type == "identifier": + parts.append(sub.text.decode("utf-8", errors="replace")) + return ".".join(parts) + + for child in node.children: + if child.type == "identifier": + imports.append( + child.text.decode("utf-8", errors="replace"), + ) + elif child.type == "import_path": + path = _import_path_text(child) + if path: + imports.append(path) + elif child.type == "selected_import": + module_name: Optional[str] = None + seen_colon = False + for sub in child.children: + if sub.type == ":": + seen_colon = True + continue + if not seen_colon: + if sub.type == "identifier": + module_name = sub.text.decode( + "utf-8", errors="replace", + ) + elif sub.type == "import_path": + path = _import_path_text(sub) + if path: + module_name = path + else: + if sub.type == "identifier" and module_name: + imported = sub.text.decode( + "utf-8", errors="replace", + ) + imports.append(f"{module_name}.{imported}") + elif language == "gdscript": + # ``extends Node`` → type > identifier("Node") + # ``extends "res://path.gd"`` → string literal + # ``extends SomeClass.Nested`` → type node (keep full text) + for child in node.children: + if child.type == "type": + txt = child.text.decode("utf-8", errors="replace").strip() + if txt: + imports.append(txt) + elif child.type == "string": + val = child.text.decode("utf-8", errors="replace").strip("'\"") + if val: + imports.append(val) + elif child.type == "identifier": + # Fallback: some grammar variants expose the parent type as + # a bare identifier next to the ``extends`` keyword. + txt = child.text.decode("utf-8", errors="replace") + if txt and txt != "extends": + imports.append(txt) else: # Fallback: just record the text imports.append(text) @@ -4863,6 +5732,55 @@ def _get_call_name(self, node, language: str, source: bytes) -> Optional[str]: first = node.children[0] + # Julia macrocall: ``@test expr`` — name is inside + # ``macro_identifier > identifier``. Prefix with ``@`` to distinguish + # from ordinary calls. + if language == "julia" and node.type == "macrocall_expression": + for child in node.children: + if child.type == "macro_identifier": + for sub in child.children: + if sub.type == "identifier": + raw = sub.text.decode("utf-8", errors="replace") + return f"@{raw}" + return None + return None + + # Julia broadcast call: ``sin.(x)`` — same structure as + # call_expression (first child is identifier or field_expression) + # so the generic paths below handle it. + if language == "php": + def _normalize_php_name(text: str) -> str: + # PHP global/function names can be prefixed with '\\'. + return text.lstrip("\\") + + if node.type == "function_call_expression": + for child in node.children: + if child.type in ("name", "qualified_name"): + raw = child.text.decode("utf-8", errors="replace") + return _normalize_php_name(raw) + return None + + if node.type in ( + "member_call_expression", + "nullsafe_member_call_expression", + ): + for child in reversed(node.children): + if child.type == "name": + return child.text.decode("utf-8", errors="replace") + return None + + if node.type == "scoped_call_expression": + parts = [] + for child in node.children: + if child.type in ("name", "qualified_name"): + raw = child.text.decode("utf-8", errors="replace") + parts.append(_normalize_php_name(raw)) + if len(parts) >= 2: + return f"{parts[0]}::{parts[-1]}" + if parts: + return parts[0] + return None + # Scala: instance_expression (new Foo(...)) – extract the type name if node.type == "instance_expression": for child in node.children: @@ -5199,21 +6117,25 @@ def _handle_r_call( import_map, defined_names, ) - if enclosing_func: - call_name = self._get_call_name(node, language, source) - if call_name: - caller = self._qualify(enclosing_func, file_path, enclosing_class) - target = self._resolve_call_target( - call_name, file_path, language, - import_map or {}, defined_names or set(), - ) - edges.append(EdgeInfo( - kind="CALLS", - source=caller, - target=target, - file_path=file_path, - line=node.start_point[0] + 1, - )) + # Module-scope R calls attribute to the File node. + call_name = self._get_call_name(node, language, source) + if call_name: + caller = ( + self._qualify(enclosing_func, file_path, enclosing_class) + if enclosing_func + else file_path + ) + target = self._resolve_call_target( + call_name, file_path, language, + import_map or {}, defined_names or set(), + ) + edges.append(EdgeInfo( + kind="CALLS", + source=caller, + target=target, + file_path=file_path, + line=node.start_point[0] + 1, + )) self._extract_from_tree( node, source, language, file_path, nodes, edges, diff --git a/tests/test_main.py b/tests/test_main.py index 61fbbf49..1a536c47 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -16,6 +16,17 @@ from code_review_graph import main as crg_main +@pytest.fixture(autouse=True) +def _isolate_crg_tools_env(monkeypatch): + """Always strip CRG_TOOLS so that any test invoking ``crg_main.main`` + does not accidentally permanently shrink the global tool registry + when the suite runs under a developer environment that exports + ``CRG_TOOLS``. Without this the snapshot/restore in + ``TestApplyToolFilter._restore_tools`` only sees the already-filtered + set and cannot restore the dropped tools.""" + monkeypatch.delenv("CRG_TOOLS", raising=False) + + class TestResolveRepoRoot: """Precedence rules for _resolve_repo_root (see #222 follow-up).""" @@ -59,7 +70,7 @@ def fake_run(**kwargs): monkeypatch.setattr(crg_main.mcp, "run", fake_run) crg_main.main(repo_root=None) - assert calls == [{"transport": "stdio"}] + assert calls == [{"transport": "stdio", "show_banner": False}] def test_http_calls_mcp_run_with_host_port(self, monkeypatch): calls: list[dict] = [] @@ -225,25 +236,37 @@ class TestApplyToolFilter: def _restore_tools(self): """Snapshot registered tools before test, restore after. - _apply_tool_filter calls ``mcp.remove_tool()`` which is - permanent. We restore by re-adding from the saved snapshot. + ``_apply_tool_filter`` calls ``mcp.remove_tool()`` which is + permanent. We snapshot the list of Tool objects via the public + ``list_tools()`` async API (FastMCP >=3) and re-register them + after the test body runs. """ - original = dict(crg_main.mcp._tool_manager._tools) + import asyncio + + original = asyncio.run(crg_main.mcp.list_tools()) yield - crg_main.mcp._tool_manager._tools.clear() - crg_main.mcp._tool_manager._tools.update(original) + current_names = { + t.name for t in asyncio.run(crg_main.mcp.list_tools()) + } + for tool in original: + if tool.name not in current_names: + crg_main.mcp.add_tool(tool) @pytest.fixture(autouse=True) def _clean_env(self, monkeypatch): """Ensure CRG_TOOLS is not set from the outer environment.""" monkeypatch.delenv("CRG_TOOLS", raising=False) + @staticmethod + async def _tool_names() -> set[str]: + return {t.name for t in await crg_main.mcp.list_tools()} + @pytest.mark.asyncio async def test_no_filter_keeps_all_tools(self): """When neither --tools nor CRG_TOOLS is set, all tools remain.""" - before = set((await crg_main.mcp.get_tools()).keys()) + before = await self._tool_names() crg_main._apply_tool_filter(None) - after = set((await crg_main.mcp.get_tools()).keys()) + after = await self._tool_names() assert before == after @pytest.mark.asyncio @@ -251,7 +274,7 @@ async def test_filter_via_argument(self): """The ``tools`` argument keeps only the listed tools.""" keep = "query_graph_tool,semantic_search_nodes_tool" crg_main._apply_tool_filter(keep) - remaining = set((await crg_main.mcp.get_tools()).keys()) + remaining = await self._tool_names() assert remaining == {"query_graph_tool", "semantic_search_nodes_tool"} @pytest.mark.asyncio @@ -259,7 +282,7 @@ async def test_filter_via_env_var(self, monkeypatch): """The ``CRG_TOOLS`` env var works as fallback.""" monkeypatch.setenv("CRG_TOOLS", "query_graph_tool") crg_main._apply_tool_filter(None) - remaining = set((await crg_main.mcp.get_tools()).keys()) + remaining = await self._tool_names() assert remaining == {"query_graph_tool"} @pytest.mark.asyncio @@ -267,21 +290,21 @@ async def test_argument_takes_precedence_over_env(self, monkeypatch): """CLI --tools wins over CRG_TOOLS env var.""" monkeypatch.setenv("CRG_TOOLS", "list_repos_tool") crg_main._apply_tool_filter("query_graph_tool") - remaining = set((await crg_main.mcp.get_tools()).keys()) + remaining = await self._tool_names() assert remaining == {"query_graph_tool"} @pytest.mark.asyncio async def test_empty_string_is_noop(self): """An empty string should not remove all tools.""" - before = set((await crg_main.mcp.get_tools()).keys()) + before = await self._tool_names() crg_main._apply_tool_filter("") - after = set((await crg_main.mcp.get_tools()).keys()) + after = await self._tool_names() assert before == after @pytest.mark.asyncio async def test_whitespace_handling(self): """Spaces around tool names are stripped.""" crg_main._apply_tool_filter(" query_graph_tool , semantic_search_nodes_tool ") - remaining = set((await crg_main.mcp.get_tools()).keys()) + remaining = await self._tool_names() assert remaining == {"query_graph_tool", "semantic_search_nodes_tool"} diff --git a/tests/test_parser.py b/tests/test_parser.py index 48bac552..841a62ac 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1258,6 +1258,48 @@ def test_elixir_top_level_dotted_call_attributes_to_file(self): ] assert len(top_level) == 1 + def test_cpp_scoped_method_names(self, tmp_path): + """C++ scoped method definitions must extract the leaf method name, + not the return-type identifier. + + Regression: previously ``Ret Class::method()`` indexed as ``Ret`` + (return type) and ``void Class::method()`` was silently dropped + because _get_name() fell through to the generic identifier loop, + which did not recognise qualified_identifier, destructor_name, or + operator_name nodes inside function_declarator. + """ + src = b""" +void PlaybackExtension::resetStateForPool() {} +quint64 PlaybackExtension::startTimestamp() const { return 0; } +PlaybackExtension::~PlaybackExtension() {} +~PlaybackExtension() {} +bool operator==(const A& a, const B& b) { return true; } +bool MyClass::operator<(const MyClass& o) const { return true; } +void foo() {} +int SnapshotController::getHandleIndex() { return 0; } +bool PlaybackWidget::AllocateResourceStrategy::allocateExtensionResource(int i) { return true; } +void A::B::C::deep() {} +ExtensionID PlaybackExtension::ID() const { return {}; } +""" + p = tmp_path / "x.cpp" + p.write_bytes(src) + nodes, _ = self.parser.parse_file(p) + names = [n.name for n in nodes if n.kind == "Function"] + assert names == [ + "resetStateForPool", + "startTimestamp", + "~PlaybackExtension", + "~PlaybackExtension", + "operator==", + "operator<", + "foo", + "getHandleIndex", + "allocateExtensionResource", + "deep", + "ID", + ] + + class TestCppScopedFunctionName: """Regression tests for C++ scoped function name extraction.