From cb973c03518d6a357f080f3c1d974fd9320504e8 Mon Sep 17 00:00:00 2001 From: Faisal01011 Date: Sat, 3 Oct 2026 20:08:26 +0530 Subject: [PATCH] fix(extract): walk Scala type_definition right-hand sides (#2049) A type alias (`type Alias = List[Int]`), an `opaque type`, and a `match type` all declare a dependency on the types they name, but the type-reference dispatch only fired for `val_definition`/`var_definition`, so these produced zero `references` edges in any context -- in contrast to a `val`/`var`, whose annotation is walked. `match_type` needed a second, independent change: the type-reference walker's recursion whitelist did not know `match_type`/`type_case_clause`, so even a correctly dispatched call on a match type's right-hand side would have resolved nothing. Both are now handled, recursing over named children exactly as `compound_type` already is. Verified field names against tree-sitter-scala 0.26.0 before writing the fix: `type_definition` carries its RHS under the `type` field for all three forms (`generic_type`, `type_identifier`, `match_type`), so the existing `child_by_field_name("type")` walk applies verbatim and no new handler is needed. File-scope aliases inherit the #2054 anchor fallback. Scope: only the edges are added. The alias still gets no node of its own, which is a separate already-accepted tradeoff shared with `val`/`given` bindings. Verified: - uv run pytest tests/test_scala_type_definition.py -q (10 passed) - 8 of the 10 new tests fail on the pre-fix tree - uv run pytest tests/ -q -> 28 failed, 6154 passed, 101 skipped; the 28 failures are byte-identical to the clean-tree baseline and none touch Scala - uv run ruff check . -> clean - uv run pyright -> 635 errors, identical to the clean-tree baseline Co-Authored-By: Claude (cherry picked from commit 879dfd330d1ecd03e83add31e50a98c47cce1a4d) --- graphify/extractors/engine.py | 19 +++- tests/test_scala_type_definition.py | 135 ++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 tests/test_scala_type_definition.py diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index 6b1c2b1ba..18c260085 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -1299,6 +1299,15 @@ def _scala_collect_type_refs(node, source: bytes, generic: bool, out: list[tuple for c in node.children: if c.is_named: _scala_collect_type_refs(c, source, generic, out) + # A `match type` (`type Elem[X] = X match { case String => Char }`) names a + # real type in every case clause -- both the type it discriminates on and + # the type it yields. Without these two the walker returned nothing for a + # match type even when correctly dispatched (#2049). Recursing over named + # children matches how `compound_type` is already handled above. + if t in ("match_type", "type_case_clause"): + for c in node.children: + if c.is_named: + _scala_collect_type_refs(c, source, generic, out) def _python_collect_param_refs(params_node, source: bytes) -> list[tuple[str, str]]: """Collect type refs from each typed parameter under a `parameters` node.""" @@ -5329,7 +5338,7 @@ def _extract_generic( return if (config.ts_module == "tree_sitter_scala" - and t in ("val_definition", "var_definition")): + and t in ("val_definition", "var_definition", "type_definition")): # Scala 3 drops the boilerplate wrapper: `val`/`var` can be written # directly at file scope with no enclosing object/class/trait. Gating # on `parent_class_nid` therefore dropped every type reference a @@ -5341,6 +5350,14 @@ def _extract_generic( # position that legitimately has no container must fall back to the # file node rather than be skipped. Reuses the owner idiom from the # property branch above. + # + # `type_definition` joins the tuple for the same reason (#2049): a + # type alias, an `opaque type` and a `match type` all declare a + # dependency on the types they name, and all three carry the RHS + # under the same `type` field, so the existing walk applies verbatim. + # Only the *edges* are in scope here -- the alias still gets no node + # of its own, which is a separate accepted tradeoff shared with + # `val`/`given` bindings. owner_nid = parent_class_nid or file_nid type_node = node.child_by_field_name("type") if type_node is not None: diff --git a/tests/test_scala_type_definition.py b/tests/test_scala_type_definition.py new file mode 100644 index 000000000..2ca20e73b --- /dev/null +++ b/tests/test_scala_type_definition.py @@ -0,0 +1,135 @@ +"""Scala `type_definition` right-hand sides are never scanned for type references. + +A type alias (`type Alias = List[Int]`), an `opaque type`, and a `match type` +all declare a dependency on the types they name, but the extractor's +type-reference dispatch only fired for `val_definition`/`var_definition`, so +these produced zero `references` edges of any kind in any context -- in +contrast to a `val`/`var`, whose annotation *is* walked. The `match_type` case +has a second, independent gap: the type-reference walker's own recursion +whitelist does not know `match_type`/`type_case_clause`, so even a correctly +dispatched call on a match type's right-hand side resolves nothing (#2049). + +Scope note: the alias node itself (`type Alias = ...`) still gets no node of its +own. That is a separate, already-accepted tradeoff shared with `val`/`given` +bindings and is deliberately untouched here -- this change is only about the +edges the alias's right-hand side implies. + +These tests pin every form (plain alias, `opaque type`, `match type`, a +multi-argument generic, and the file-scope position), that each bound type +resolves, that the matched type in a `match type` case is walked too, and that +the `val`/`var` context split is preserved rather than flattened. +""" +from pathlib import Path + +from graphify.extract import extract_scala + +SRC = '''\ +class Marker +class Payload + +trait Repo: + type Alias = List[Payload] + opaque type Id = Long + type Elem[X] = X match + case String => Char + +type TopLevel = Map[String, Int] +opaque type OpaqueTop = Marker + +class Holder { + val plain: Payload = null +} +''' + + +def _build(tmp_path): + (tmp_path / "Aliases.scala").write_text(SRC) + r = extract_scala(tmp_path / "Aliases.scala") + nid = {n["label"]: n["id"] for n in r["nodes"]} + return r, nid + + +def _refs_from(r, src_label, nid): + return {(e["target"], e.get("context")) for e in r["edges"] + if e["relation"] == "references" and e["source"] == nid[src_label]} + + +def test_trait_type_alias_emits_reference_to_base_type(tmp_path): + r, nid = _build(tmp_path) + refs = _refs_from(r, "Repo", nid) + assert (nid["List"], "field") in refs + + +def test_trait_type_alias_emits_reference_to_type_argument(tmp_path): + r, nid = _build(tmp_path) + refs = _refs_from(r, "Repo", nid) + assert (nid["Payload"], "generic_arg") in refs + + +def test_opaque_type_emits_reference_to_underlying_type(tmp_path): + r, nid = _build(tmp_path) + refs = _refs_from(r, "Repo", nid) + assert (nid["Long"], "field") in refs + + +def test_match_type_emits_reference_to_case_result(tmp_path): + # `match_type` was the case that needed a walker change as well as a + # dispatch change: without `match_type`/`type_case_clause` in the recursion + # whitelist the dispatch would fire and still resolve nothing. + r, nid = _build(tmp_path) + refs = _refs_from(r, "Repo", nid) + assert (nid["Char"], "field") in refs + + +def test_match_type_emits_reference_to_matched_type(tmp_path): + # `case String =>` names a real type the alias discriminates on. The walk is + # a plain recursion over named children, exactly as `compound_type` already + # does, so the matched type is collected alongside the result. + r, nid = _build(tmp_path) + refs = _refs_from(r, "Repo", nid) + assert any(t == nid["String"] for t, _c in refs) + + +def test_file_scope_type_alias_is_owned_by_the_file_node(tmp_path): + # File-scope aliases need the same anchor fallback as file-scope `val` + # (#2054); without it this edge is dropped by the same missing-container + # defect. + r, nid = _build(tmp_path) + refs = _refs_from(r, "Aliases.scala", nid) + assert (nid["Map"], "field") in refs + assert (nid["String"], "generic_arg") in refs + assert (nid["Int"], "generic_arg") in refs + + +def test_file_scope_opaque_type_is_owned_by_the_file_node(tmp_path): + r, nid = _build(tmp_path) + refs = _refs_from(r, "Aliases.scala", nid) + assert (nid["Marker"], "field") in refs + + +def test_val_annotation_context_is_unchanged(tmp_path): + # The `val`/`var` path and the `type_definition` path share this dispatch, so + # pin that widening the tuple did not change the existing context split. + r, nid = _build(tmp_path) + refs = _refs_from(r, "Holder", nid) + assert refs == {(nid["Payload"], "field")} + + +def test_type_definition_edges_are_tagged_extracted(tmp_path): + r, nid = _build(tmp_path) + edges = [e for e in r["edges"] + if e["relation"] == "references" + and e["source"] in (nid["Repo"], nid["Aliases.scala"])] + assert edges + for e in edges: + assert e.get("confidence") == "EXTRACTED", e + loc = e.get("source_location") + assert isinstance(loc, str) and loc.startswith("L"), e + + +def test_alias_reference_never_points_at_the_owning_container(tmp_path): + # The self-reference guard follows the anchor, so an alias whose right-hand + # side names the enclosing type's own stem must not mint Repo -> Repo. + r, nid = _build(tmp_path) + for owner in ("Repo", "Aliases.scala"): + assert not any(t == nid[owner] for t, _c in _refs_from(r, owner, nid))