mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 07:29:33 +00:00
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 <noreply@anthropic.com>
(cherry picked from commit 879dfd330d)
This commit is contained in:
1 parent
447d41dd0d
commit
cb973c0351
2 files changed
+153
-1
No files matched your search
@@ -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:
|
||||
|
||||
@@ -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))
|
||||
Reference in new issue
Block a user