diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index bf09531..d75e657 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -2,7 +2,7 @@ > **Note:** This document is automatically generated and verified against the live test suite by `scripts/generate_requirements.py` and `tests/backend/test_requirements_sync.py`. -**Test Verification Baseline:** **1182 Automated Tests** (818 Pytest Backend + 314 Vitest Frontend + 50 Playwright E2E). +**Test Verification Baseline:** **1185 Automated Tests** (821 Pytest Backend + 314 Vitest Frontend + 50 Playwright E2E). --- @@ -959,6 +959,11 @@ code results are enriched with signature/kind/ast_symbol_id from SQLite._ - `test_variables_without_a_url_are_ignored` - `test_csharp_rules_do_not_apply_to_other_languages` +#### `tests/test_csharp_typed_call_targets.py` (3 tests) +- `test_receiver_types_come_from_fields_parameters_locals_and_properties` +- `test_callers_of_same_named_methods_with_concrete_injection` +- `test_callees_follow_the_field_type` + #### `tests/test_database_engine.py` (11 tests) - `test_schema_metadata_contains_all_tables` - `test_schema_api_keys_columns` diff --git a/app/services/chunking/csharp_receiver_types.py b/app/services/chunking/csharp_receiver_types.py new file mode 100644 index 0000000..67f95b3 --- /dev/null +++ b/app/services/chunking/csharp_receiver_types.py @@ -0,0 +1,170 @@ +""" +Declared types of C# call receivers, so that `_toolService.PublishInstance()` is stored as +`ToolService.PublishInstance` and not as the bare `PublishInstance`. + +A bare method name matches every class that has a method with that name: a controller, its +service, and the service's repository often all have `PublishInstance`. With the receiver's +declared type, trace_path and the topology graph connect the call to the correct class. + +The receiver must be a simple name (`x` or `this.x`). Its type comes from the nearest scope that +declares it: a parameter, local variable, `foreach` variable, or lambda parameter of an enclosing +method; or a field, property, or primary-constructor parameter of the enclosing type. `var x = new T()` +gives `T`. When no declaration is found (static calls, `var` without `new`, inherited members), +the call keeps the bare name. +""" + +import re +from typing import Dict, Optional, Tuple + +_TYPE_DECLARATIONS = frozenset({ + "class_declaration", "struct_declaration", "record_declaration", + "record_struct_declaration", "interface_declaration", +}) +_SCOPES = frozenset({ + "method_declaration", "constructor_declaration", "local_function_statement", + "lambda_expression", "anonymous_method_expression", "accessor_declaration", + "operator_declaration", "conversion_operator_declaration", "destructor_declaration", +}) +_SKIP_TYPES = frozenset({ + "var", "dynamic", "object", "string", "bool", "byte", "sbyte", "char", "decimal", "double", + "float", "int", "uint", "long", "ulong", "short", "ushort", "nint", "nuint", +}) +_SIMPLE_RECEIVER = re.compile(r"^(?:this\.)?([A-Za-z_]\w*)$") + +ScopeTypes = Dict[str, str] +TypeCache = Dict[Tuple[int, int, str], ScopeTypes] + + +def _text(node, source_bytes: bytes) -> str: + return source_bytes[node.start_byte:node.end_byte].decode("utf-8", errors="ignore").strip() + + +def _clean_type(node, source_bytes: bytes) -> Optional[str]: + """`Ns.ToolService`, `ILogger`, `ToolService?` -> the simple type name; None for `var`, arrays, built-ins.""" + if node is None or node.type in ("implicit_type", "predefined_type", "array_type", "tuple_type", "pointer_type"): + return None + raw = re.sub(r"<.*>", "", _text(node, source_bytes)).rstrip("?").strip() + name = raw.rsplit(".", 1)[-1].split("::")[-1] + if not re.fullmatch(r"[A-Za-z_]\w*", name) or name in _SKIP_TYPES: + return None + return name + + +def _declarator_name(declarator, source_bytes: bytes) -> Optional[str]: + name_node = declarator.child_by_field_name("name") + if name_node is None: + name_node = next((c for c in declarator.children if c.type == "identifier"), None) + return _text(name_node, source_bytes) if name_node is not None else None + + +def _created_type(declarator, source_bytes: bytes) -> Optional[str]: + """`var x = new T(...)` -> T (the creation must be the whole initializer, not a nested argument).""" + for child in declarator.children: + target = child + if target.type == "equals_value_clause": + target = next((c for c in target.children if c.is_named), None) + if target is not None and target.type == "object_creation_expression": + return _clean_type(target.child_by_field_name("type"), source_bytes) + return None + + +def _add_variable_declaration(decl, source_bytes: bytes, out: ScopeTypes) -> None: + declared = _clean_type(decl.child_by_field_name("type"), source_bytes) + for child in decl.children: + if child.type != "variable_declarator": + continue + name = _declarator_name(child, source_bytes) + var_type = declared or _created_type(child, source_bytes) + if name and var_type: + out.setdefault(name, var_type) + + +def _add_parameters(param_list, source_bytes: bytes, out: ScopeTypes) -> None: + for param in param_list.children: + if param.type != "parameter": + continue + name_node, type_node = param.child_by_field_name("name"), param.child_by_field_name("type") + p_type = _clean_type(type_node, source_bytes) + if name_node is not None and p_type: + out.setdefault(_text(name_node, source_bytes), p_type) + + +def _scope_types(scope, source_bytes: bytes) -> ScopeTypes: + """Parameters and locals of one method-like scope (nested methods and types are not entered).""" + out: ScopeTypes = {} + for child in scope.children: + if child.type == "parameter_list": + _add_parameters(child, source_bytes, out) + stack = [c for c in scope.children if c.type != "parameter_list"] + while stack: + node = stack.pop() + if node.type in _SCOPES or node.type in _TYPE_DECLARATIONS: + continue + if node.type == "variable_declaration" and node.parent is not None and node.parent.type != "field_declaration": + _add_variable_declaration(node, source_bytes, out) + elif node.type == "foreach_statement": + f_type = _clean_type(node.child_by_field_name("type"), source_bytes) + left = node.child_by_field_name("left") + if f_type and left is not None and left.type == "identifier": + out.setdefault(_text(left, source_bytes), f_type) + stack.extend(node.children) + return out + + +def _type_member_types(type_decl, source_bytes: bytes) -> ScopeTypes: + """Fields, properties, and primary-constructor parameters of one type declaration.""" + out: ScopeTypes = {} + for child in type_decl.children: + if child.type == "parameter_list": + _add_parameters(child, source_bytes, out) + elif child.type == "declaration_list": + for member in child.children: + if member.type in ("field_declaration", "event_field_declaration"): + for decl in member.children: + if decl.type == "variable_declaration": + _add_variable_declaration(decl, source_bytes, out) + elif member.type == "property_declaration": + p_type = _clean_type(member.child_by_field_name("type"), source_bytes) + name_node = member.child_by_field_name("name") + if p_type and name_node is not None: + out.setdefault(_text(name_node, source_bytes), p_type) + return out + + +def _cached(cache: TypeCache, node, source_bytes: bytes, builder) -> ScopeTypes: + key = (node.start_byte, node.end_byte, node.type) + if key not in cache: + cache[key] = builder(node, source_bytes) + return cache[key] + + +def receiver_type(call_node, source_bytes: bytes, cache: TypeCache) -> Optional[str]: + """Declared type of the receiver of `receiver.Method(...)`, or None when it is not known.""" + fn_node = call_node.child_by_field_name("function") + if fn_node is None or fn_node.type != "member_access_expression": + return None + receiver = fn_node.child_by_field_name("expression") + if receiver is None: + return None + m = _SIMPLE_RECEIVER.match(_text(receiver, source_bytes)) + if not m: + return None + name, via_this = m.group(1), receiver.type == "member_access_expression" + node = call_node.parent + while node is not None: + if node.type in _SCOPES and not via_this: + found = _cached(cache, node, source_bytes, _scope_types).get(name) + if found: + return found + elif node.type in _TYPE_DECLARATIONS: + return _cached(cache, node, source_bytes, _type_member_types).get(name) + node = node.parent + return None + + +def qualify_csharp_call(call_node, source_bytes: bytes, target: str, cache: TypeCache) -> str: + """`Type.target` when the receiver's declared type is known, else `target` unchanged.""" + if call_node.type != "invocation_expression" or not target or "." in target: + return target + r_type = receiver_type(call_node, source_bytes, cache) + return f"{r_type}.{target}" if r_type else target diff --git a/app/services/chunking/symbol_extractor.py b/app/services/chunking/symbol_extractor.py index 1ed9e92..795debe 100644 --- a/app/services/chunking/symbol_extractor.py +++ b/app/services/chunking/symbol_extractor.py @@ -19,6 +19,7 @@ extract_inheritance_relationships ) from app.services.chunking.cobol_extractor import extract_cobol_symbols_and_chunks +from app.services.chunking.csharp_receiver_types import qualify_csharp_call from app.services.chunking.api_route_extractor import ( normalize_path_pattern, match_route_and_call, @@ -90,6 +91,8 @@ def extract_symbols_and_chunks( target_types = CODE_CONTAINER_TYPES.get(language, set()) file_symbol = os.path.basename(filepath) + is_csharp = language in ("c_sharp", "csharp") + receiver_type_cache: Dict[Any, Dict[str, str]] = {} def traverse(node, parent_symbol: Optional[str] = None): line_num = node.start_point[0] + 1 @@ -216,6 +219,8 @@ def traverse(node, parent_symbol: Optional[str] = None): else extract_target_from_call_node(node, source_bytes) ) if target and target not in ("self", "this", "super", "new", "var"): + if is_csharp: + target = qualify_csharp_call(node, source_bytes, target, receiver_type_cache) active_src = parent_symbol if parent_symbol else file_symbol active_src_name = parent_symbol.split(".")[-1] if (parent_symbol and "." in parent_symbol) else active_src relationships.append({ diff --git a/app/services/diagnostics.py b/app/services/diagnostics.py index e085e2f..6a5752c 100644 --- a/app/services/diagnostics.py +++ b/app/services/diagnostics.py @@ -52,6 +52,10 @@ def _get_callers_and_callees( "FROM ast_relationships WHERE (target_symbol = ? OR target_symbol = ?)" ) c_params: List[Any] = [symbol_name, short_sym] + if "." not in symbol_name: + # C# call targets can be stored with the receiver type (`ToolService.PublishInstance`). + c_sql = c_sql[:-1] + " OR target_symbol LIKE ?)" + c_params.append(f"%.{symbol_name}") if effective_repo: c_sql += " AND repo = ?" c_params.append(effective_repo) diff --git a/app/services/graph_traversal.py b/app/services/graph_traversal.py index 11ca18b..789079c 100644 --- a/app/services/graph_traversal.py +++ b/app/services/graph_traversal.py @@ -190,6 +190,18 @@ def _resolve_symbol_candidates(sym_str: str) -> List[dict]: up = sym_str.upper() return syms_by_full_upper.get(up) or syms_by_name_upper.get(up) or [] + def _single_implementation(full: str) -> str: + """`IRepo.Save` -> `SqlRepo.Save` when exactly one class implements `IRepo` and has the member.""" + if "." not in full: + return full + owner, member = full.rsplit(".", 1) + impls = iface_to_concretes.get(owner.split(".")[-1], set()) + if len(impls) == 1: + impl_cands = syms_by_full.get(f"{next(iter(impls))}.{member}", []) + if impl_cands: + return impl_cands[0]["full_symbol"] + return full + def _resolve_di_callee(raw_target: str, caller_sym: str, candidates: List[dict]) -> str: if not candidates: if "." in raw_target: @@ -201,7 +213,7 @@ def _resolve_di_callee(raw_target: str, caller_sym: str, candidates: List[dict]) return impl_cands[0]["full_symbol"] return raw_target if len(candidates) == 1: - return candidates[0].get("full_symbol") or candidates[0].get("name") or raw_target + return _single_implementation(candidates[0].get("full_symbol") or candidates[0].get("name") or raw_target) non_iface = [ c for c in candidates if (c.get("full_symbol") or "").rsplit(".", 1)[0] not in interface_names and c.get("kind") != "interface_declaration" diff --git a/app/services/topology/graph_builder.py b/app/services/topology/graph_builder.py index 59b8ff8..59be48e 100644 --- a/app/services/topology/graph_builder.py +++ b/app/services/topology/graph_builder.py @@ -110,6 +110,7 @@ def get_topology_graph( # Symbol nodes symbol_id_by_db_id: Dict[int, str] = {} symbols_by_name: Dict[Tuple[str, str], List[str]] = {} + symbols_by_full: Dict[Tuple[str, str], List[str]] = {} symbols_by_file: Dict[Tuple[str, str], List[str]] = {} for r in symbol_rows: @@ -119,6 +120,8 @@ def get_topology_graph( key_name = (r["repo"], r["name"]) symbols_by_name.setdefault(key_name, []).append(s_id) + if r["full_symbol"]: + symbols_by_full.setdefault((r["repo"], r["full_symbol"]), []).append(s_id) key_file = (r["repo"], clean_fp) symbols_by_file.setdefault(key_file, []).append(s_id) @@ -296,6 +299,11 @@ def _methods_match(m1: Optional[str], m2: Optional[str]) -> bool: tgt_sym_name = rel["target_symbol"] tgt_s_ids = symbols_by_name.get((rel["repo"], tgt_sym_name), []) + if not tgt_s_ids and tgt_sym_name and "." in tgt_sym_name: + # Typed call target (`ToolService.PublishInstance`): match the full symbol, else the method name. + tgt_s_ids = symbols_by_full.get((rel["repo"], tgt_sym_name), []) + if not tgt_s_ids: + tgt_s_ids = symbols_by_name.get((rel["repo"], tgt_sym_name.rsplit(".", 1)[-1]), []) if not tgt_s_ids: # Try finding across all repos for (s_repo, s_name), s_ids in symbols_by_name.items(): diff --git a/docs/mcp-tools-and-resources.md b/docs/mcp-tools-and-resources.md index 08112ec..3a9427c 100644 --- a/docs/mcp-tools-and-resources.md +++ b/docs/mcp-tools-and-resources.md @@ -77,7 +77,7 @@ Paginated MCP resource templates (`files/{page}` and `routes/{page}`) use 1-inde - **`find_symbol`**: Queries `ast_symbols` directly by `name` (`exact=True` or substring `exact=False`) and optional `repo`. Clamps `limit` to `1..100` (default `30`) with `offset` pagination. Use this tool first when you know an identifier name. - **`get_file_outline`**: Returns deduplicated classes, methods, functions, signatures, and `[start_line, end_line]` spans for `filepath`. Clamps `limit` to `1..500` (default `150`) with `offset` pagination (`total_symbols`, `has_more`, `next_offset`). -- **`trace_path`**: Traverses AST `callers`, `callees`, or `both` via BFS (`depth` clamped `1..5`, default `2`) and resolves cross-repository HTTP route/caller boundaries (`external_callers`, `external_handlers`). Clamps `limit` to `1..200` (default `50`) with `offset`, returning `total_callers`, `total_callees`, `has_more`, and `next_offset`. Caller/callee nodes omit their Git permalinks unless `include_permalinks=True` (the target always keeps its permalink), which keeps deep traces compact. +- **`trace_path`**: Traverses AST `callers`, `callees`, or `both` via BFS (`depth` clamped `1..5`, default `2`) and resolves cross-repository HTTP route/caller boundaries (`external_callers`, `external_handlers`). Clamps `limit` to `1..200` (default `50`) with `offset`, returning `total_callers`, `total_callees`, `has_more`, and `next_offset`. Caller/callee nodes omit their Git permalinks unless `include_permalinks=True` (the target always keeps its permalink), which keeps deep traces compact. In C#, a call such as `_toolService.PublishInstance()` is indexed with the receiver's declared type (`ToolService.PublishInstance`, from a field, property, parameter, local, or primary-constructor parameter), so methods with the same name in different classes are not mixed up; re-index C# repositories after an upgrade to get these targets. ### HTTP Route and Client Discovery Tools diff --git a/tests/test_csharp_typed_call_targets.py b/tests/test_csharp_typed_call_targets.py new file mode 100644 index 0000000..ac90934 --- /dev/null +++ b/tests/test_csharp_typed_call_targets.py @@ -0,0 +1,163 @@ +"""C# call targets carry the receiver's declared type, so same-named methods of different classes stay apart.""" + +import json +from unittest.mock import patch + +import pytest + +from app.services.chunking import extract_symbols_and_chunks +from app.services.chunking.tree_sitter_loader import get_tree_sitter_parser +from app.services.search import trace_symbol_path +from tests.test_multilanguage_cs_ts_py import _conn_factory, _setup_sqlite_repo + +pytestmark = pytest.mark.skipif(get_tree_sitter_parser("c_sharp") is None, reason="needs the C# tree-sitter grammar") + +TYPES_CS = """\ +public class Worker(IClock clock) +{ + private readonly ToolService _toolService; + private readonly Ns.Data.ToolRepository? _repo; + public ILogger Logger { get; } + + public async Task Run(Bar bar, string name) + { + var made = new Baz(); + var unknown = Factory.Create(); + await _toolService.PublishInstance(1); + await this._repo.Save(2); + bar.Go(); + made.Spin(); + unknown.Spin(); + clock.Tick(); + Logger.LogInformation("x"); + name.Trim(); + StaticHelpers.Format(3); + foreach (Item item in bar.Items) { item.Touch(); } + LocalCall(); + } + + public void Shadow(ToolRepository _toolService) + { + _toolService.PublishInstance(4); + } +} +""" + + +def _call_targets(source: str): + res = extract_symbols_and_chunks(source, "Worker.cs", repo="r") + return {r.target_symbol for r in res.relationships if r.relationship_type == "CALLS"} + + +def test_receiver_types_come_from_fields_parameters_locals_and_properties(): + targets = _call_targets(TYPES_CS) + for expected in ( + "ToolService.PublishInstance", # field + "ToolRepository.Save", # this.field, namespace and nullable removed + "Bar.Go", # method parameter + "Baz.Spin", # var x = new T() + "IClock.Tick", # primary-constructor parameter + "ILogger.LogInformation", # property, generic arguments removed + "Item.Touch", # foreach variable + "ToolRepository.PublishInstance", # parameter hides the field of the same name + ): + assert expected in targets, expected + # Unknown receivers keep the bare name. + assert {"Spin", "Format", "LocalCall", "Trim"} <= targets + assert "string.Trim" not in targets and "String.Trim" not in targets + + +# The real layout that showed the problem: concrete classes injected into fields, no interfaces. +CONTROLLERS_CS = """\ +public class AssistantsController +{ + private readonly AssistantService _assistantService; + public AssistantsController(AssistantService assistantService) { _assistantService = assistantService; } + + [HttpPost("{id}/publish")] + public async Task PublishInstance(Guid id) + { + return Ok(await _assistantService.PublishInstance(id)); + } +} + +public class ToolsController +{ + private readonly ToolService _toolService; + private readonly AssistantService _assistantService; + public ToolsController(ToolService toolService, AssistantService assistantService) { _toolService = toolService; _assistantService = assistantService; } + + [HttpPost("{id}/publish")] + public async Task PublishInstance(Guid id) + { + return Ok(await _toolService.PublishInstance(id)); + } +} +""" +SERVICES_CS = """\ +public class AssistantService +{ + private readonly AssistantSuperRepository _assistantRepository; + public AssistantService(AssistantSuperRepository assistantRepository) { _assistantRepository = assistantRepository; } + + public async Task PublishInstance(Guid id) + { + return await _assistantRepository.PublishInstance(id); + } +} + +public class ToolService +{ + private readonly ToolSuperRepository _toolRepository; + public ToolService(ToolSuperRepository toolRepository) { _toolRepository = toolRepository; } + + public async Task PublishInstance(Guid id) + { + return await _toolRepository.PublishInstance(id); + } +} +""" +REPOSITORIES_CS = """\ +public class AssistantSuperRepository +{ + public Task PublishInstance(Guid id) => Task.FromResult(1); +} + +public class ToolSuperRepository +{ + public Task PublishInstance(Guid id) => Task.FromResult(2); +} +""" + + +@pytest.fixture +def hub_db(tmp_path): + db_path = str(tmp_path / "hub.db") + _setup_sqlite_repo(db_path, "hub", [ + ("Controllers/Controllers.cs", CONTROLLERS_CS), + ("Services/Services.cs", SERVICES_CS), + ("Data/Repositories.cs", REPOSITORIES_CS), + ]) + return _conn_factory(db_path) + + +def _trace(get_conn, symbol: str, direction: str): + with patch("app.services.search.get_db_connection", side_effect=get_conn): + return json.loads(trace_symbol_path(symbol, repo="hub", direction=direction, depth=1)) + + +@pytest.mark.parametrize("target, caller", [ + ("AssistantService.PublishInstance", "AssistantsController.PublishInstance"), + ("ToolService.PublishInstance", "ToolsController.PublishInstance"), + ("AssistantSuperRepository.PublishInstance", "AssistantService.PublishInstance"), + ("ToolSuperRepository.PublishInstance", "ToolService.PublishInstance"), +]) +def test_callers_of_same_named_methods_with_concrete_injection(hub_db, target, caller): + callers = [c["symbol"] for c in _trace(hub_db, target, "callers")["callers"]] + assert callers == [caller] + + +def test_callees_follow_the_field_type(hub_db): + callees = [c["symbol"] for c in _trace(hub_db, "ToolsController.PublishInstance", "callees")["callees"]] + assert "ToolService.PublishInstance" in callees + assert "AssistantService.PublishInstance" not in callees