mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 23:49:14 +00:00
fix(extract): resolve Scala context bounds to references edges (#2046)
`def sort[A: Ordering]` desugars to an implicit `Ordering[A]`, so a context
bound is a real dependency the function has on that typeclass -- and the
idiomatic form is more common in modern Scala than the desugared
`(implicit ...)` it compiles to. `context_bound` was not referenced anywhere
in the extractor, so the dependency was invisible wherever it appeared.
Two grammar shapes drive the implementation, both verified against
tree-sitter-scala 0.26.0 before writing the fix:
- `context_bound` is a direct child of `type_parameters`, one per bound, so
the walk belongs in the existing per-function Scala block and needs no new
handler.
- A higher-kinded parameter nests a further `type_parameters` for its own arity
brackets (`def program[F[_]: Async: Temporal]`), so the walk recurses rather
than reading direct children only -- otherwise both bounds on `program` are
missed.
`upper_bound` (`[T <: Comparable[T]]`) is a distinct node type and is
deliberately not matched: a subtype constraint is not a dependency, so an edge
there would assert something the source does not say. Bounds also get their
own `type_bound` context rather than reusing `parameter_type`, so a consumer
can tell a typeclass constraint from a parameter annotation.
One trap worth recording, because the neighbouring code invites it:
`type_parameters` and `parameters` carry the *same* field name on
`function_definition`, so `child_by_field_name("parameters")` returns whichever
comes first. The bounds are therefore located by node type, never by field
name -- a field lookup would read the function's real parameters as bounds. A
regression test pins that `xs: List[A]` still resolves under `parameter_type`
and never under `type_bound`.
Verified:
- uv run pytest tests/test_scala_context_bounds.py -q (10 passed)
- 6 of the 10 new tests fail on the pre-fix tree
- uv run pytest tests/ -q -> 28 failed, 6164 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>
This commit is contained in:
1 parent
879dfd330d
commit
e293b64f6e
2 files changed
+191
No files matched your search
@@ -1309,6 +1309,38 @@ def _scala_collect_type_refs(node, source: bytes, generic: bool, out: list[tuple
|
||||
if c.is_named:
|
||||
_scala_collect_type_refs(c, source, generic, out)
|
||||
|
||||
|
||||
def _scala_collect_context_bounds(
|
||||
tp_node, source: bytes, out: list[tuple[str, str]]
|
||||
) -> None:
|
||||
"""Collect the types named by a ``type_parameters`` node's context bounds.
|
||||
|
||||
``def sort[A: Ordering]`` desugars to an implicit ``Ordering[A]``, so the
|
||||
bound is a real dependency the function has on that typeclass -- but
|
||||
``context_bound`` was not referenced anywhere in the extractor, so the
|
||||
dependency was invisible wherever it appeared (#2046).
|
||||
|
||||
A ``context_bound`` is a direct child of ``type_parameters``, one per bound.
|
||||
A higher-kinded parameter nests a further ``type_parameters`` for its own
|
||||
arity brackets (``def program[F[_]: Async: Temporal]``), so this recurses
|
||||
rather than reading direct children only -- otherwise both bounds on
|
||||
``program`` are missed.
|
||||
|
||||
``upper_bound`` (``[T <: Comparable[T]]``) is a distinct node type and is
|
||||
deliberately not matched here: a subtype constraint is not a dependency, so
|
||||
emitting an edge for it would assert something the source does not say.
|
||||
"""
|
||||
if tp_node is None:
|
||||
return
|
||||
for c in tp_node.children:
|
||||
if c.type == "type_parameters":
|
||||
_scala_collect_context_bounds(c, source, out)
|
||||
elif c.type == "context_bound":
|
||||
bound_type = c.child_by_field_name("type")
|
||||
if bound_type is not None:
|
||||
_scala_collect_type_refs(bound_type, source, False, 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."""
|
||||
out: list[tuple[str, str]] = []
|
||||
@@ -5849,6 +5881,26 @@ def _extract_generic(
|
||||
if target_nid != func_nid:
|
||||
add_edge(func_nid, target_nid, "references",
|
||||
line, context=ctx)
|
||||
# Context bounds (`def sort[A: Ordering]`) are the sugar for
|
||||
# typeclass-parameter wiring and name a real dependency, so they
|
||||
# get their own `type_bound` context rather than being folded
|
||||
# into `parameter_type` (#2046). Found by node type, never by
|
||||
# field name: `type_parameters` and `parameters` share the field
|
||||
# name "parameters" on function_definition, so a field lookup
|
||||
# here would return the type parameters and read the function's
|
||||
# real parameters as if they were bounds.
|
||||
for c in node.children:
|
||||
if c.type != "type_parameters":
|
||||
continue
|
||||
bound_refs: list[tuple[str, str]] = []
|
||||
_scala_collect_context_bounds(c, source, bound_refs)
|
||||
for ref_name, role in bound_refs:
|
||||
ctx = ("generic_arg" if role == "generic_arg"
|
||||
else "type_bound")
|
||||
target_nid = ensure_named_node(ref_name, line)
|
||||
if target_nid != func_nid:
|
||||
add_edge(func_nid, target_nid, "references",
|
||||
line, context=ctx)
|
||||
|
||||
body = _find_body(node, config)
|
||||
# JS/TS: capture callable members assigned directly in a function
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
"""Scala context bounds (`[A: Ordering]`) never produce a `references` edge.
|
||||
|
||||
Context bounds are the standard sugar for typeclass-parameter wiring in
|
||||
idiomatic Scala, and are more common in modern code than the desugared
|
||||
`(implicit ...)` form they compile to. The `context_bound` node type was not
|
||||
referenced anywhere in the extractor, so a type parameter's constraint was
|
||||
invisible no matter where in the file it appeared (#2046).
|
||||
|
||||
The grammar shapes this precisely, and two of them drive the fix:
|
||||
|
||||
- `context_bound` is a direct child of `type_parameters`, one per bound, sitting
|
||||
alongside the type parameter's own `identifier`.
|
||||
- For a higher-kinded parameter the nested arity brackets are a *nested*
|
||||
`type_parameters` (`F[_]`), so the walk has to recurse rather than take direct
|
||||
children only, or `[F[_]: Async: Temporal]` yields nothing.
|
||||
|
||||
Two deliberate non-emissions are pinned as well. An `upper_bound` (`[T <: C]`)
|
||||
is a separate node type and is a subtype constraint, not a dependency, so it
|
||||
must not emit an edge. And `type_parameters` and `parameters` share the *same*
|
||||
field name on `function_definition`, so the sibling walk must match on node type
|
||||
-- looking the node up by field name would return the type parameters and read
|
||||
the function's real parameters as if they were bounds.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
from graphify.extract import extract_scala
|
||||
|
||||
SRC = '''\
|
||||
trait Ordering
|
||||
trait Async
|
||||
trait Temporal
|
||||
trait Comparable
|
||||
|
||||
def sort[A: Ordering](xs: List[A]): List[A] = xs
|
||||
|
||||
def program[F[_]: Async: Temporal](x: F[Int]): F[Int] = x
|
||||
|
||||
def plain[T](t: T): T = t
|
||||
|
||||
def bounded[T <: Comparable[T]](t: T): T = t
|
||||
|
||||
class Container {
|
||||
def member[K: Ordering](k: K): K = k
|
||||
}
|
||||
'''
|
||||
|
||||
|
||||
def _build(tmp_path):
|
||||
(tmp_path / "ContextBounds.scala").write_text(SRC)
|
||||
r = extract_scala(tmp_path / "ContextBounds.scala")
|
||||
nid = {n["label"]: n["id"] for n in r["nodes"]}
|
||||
return r, nid
|
||||
|
||||
|
||||
def _bound_targets(r, func_label, nid):
|
||||
"""References from `func_label` whose context is `type_bound`."""
|
||||
return {e["target"] for e in r["edges"]
|
||||
if e["relation"] == "references"
|
||||
and e["source"] == nid[func_label]
|
||||
and e.get("context") == "type_bound"}
|
||||
|
||||
|
||||
def test_single_context_bound_emits_reference(tmp_path):
|
||||
r, nid = _build(tmp_path)
|
||||
assert nid["Ordering"] in _bound_targets(r, "sort()", nid)
|
||||
|
||||
|
||||
def test_several_context_bounds_each_emit_a_reference(tmp_path):
|
||||
r, nid = _build(tmp_path)
|
||||
targets = _bound_targets(r, "program()", nid)
|
||||
assert nid["Async"] in targets
|
||||
assert nid["Temporal"] in targets
|
||||
|
||||
|
||||
def test_nested_type_parameters_do_not_hide_the_bounds(tmp_path):
|
||||
# `F[_]` nests a type_parameters inside type_parameters. A direct-children
|
||||
# walk would miss both bounds on `program` entirely.
|
||||
r, nid = _build(tmp_path)
|
||||
targets = _bound_targets(r, "program()", nid)
|
||||
assert len(targets) >= 2, targets
|
||||
|
||||
|
||||
def test_context_bound_uses_its_own_context_not_parameter_type(tmp_path):
|
||||
# A typeclass constraint is not a parameter type. Reusing `parameter_type`
|
||||
# would make the two indistinguishable to a consumer.
|
||||
r, nid = _build(tmp_path)
|
||||
edges = [e for e in r["edges"]
|
||||
if e["relation"] == "references" and e["source"] == nid["sort()"]]
|
||||
bound = [e for e in edges if e.get("context") == "type_bound"]
|
||||
assert bound, "expected a type_bound context"
|
||||
assert all(e.get("context") != "parameter_type" for e in bound)
|
||||
|
||||
|
||||
def test_unbounded_type_parameter_emits_no_bound_edge(tmp_path):
|
||||
r, nid = _build(tmp_path)
|
||||
assert not _bound_targets(r, "plain()", nid)
|
||||
|
||||
|
||||
def test_upper_bound_emits_no_bound_edge(tmp_path):
|
||||
# `[T <: Comparable[T]]` is an `upper_bound`, a distinct node type. It states
|
||||
# a subtype constraint, not a dependency, so it must not become an edge.
|
||||
r, nid = _build(tmp_path)
|
||||
assert not _bound_targets(r, "bounded()", nid)
|
||||
|
||||
|
||||
def test_context_bound_does_not_swallow_the_functions_own_parameters(tmp_path):
|
||||
# `type_parameters` and `parameters` share the field name "parameters" on
|
||||
# `function_definition`. A field-name lookup would pick the type parameters
|
||||
# and read `xs: List[A]` as a bound, so this pins that the real parameter
|
||||
# type still resolves under the `parameter_type` context.
|
||||
r, nid = _build(tmp_path)
|
||||
refs = {(e["target"], e.get("context")) for e in r["edges"]
|
||||
if e["relation"] == "references" and e["source"] == nid["sort()"]}
|
||||
assert (nid["List"], "parameter_type") in refs
|
||||
assert (nid["List"], "type_bound") not in refs
|
||||
|
||||
|
||||
def test_member_function_bound_is_attributed_to_the_member(tmp_path):
|
||||
# Class members carry a dotted label (`.member()`); top-level defs do not.
|
||||
r, nid = _build(tmp_path)
|
||||
assert nid["Ordering"] in _bound_targets(r, ".member()", nid)
|
||||
|
||||
|
||||
def test_bound_edge_is_tagged_extracted_with_a_location(tmp_path):
|
||||
r, nid = _build(tmp_path)
|
||||
edges = [e for e in r["edges"]
|
||||
if e["relation"] == "references"
|
||||
and e.get("context") == "type_bound"]
|
||||
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_bound_never_points_at_the_function_itself(tmp_path):
|
||||
r, nid = _build(tmp_path)
|
||||
for label in ("sort()", "program()", ".member()"):
|
||||
assert nid[label] not in _bound_targets(r, label, nid)
|
||||
Reference in new issue
Block a user