Skip to content

Commit b04ec12

Browse files
committed
refactor(go): pin the type-kind parse blocker beside the other model tests
The alias fix stays bundled with this branch rather than shipping separately. Prompted by prototyping the standalone version, which surfaced two things worth keeping: The two kind tests assert a property of the model over -a 1 output, with nothing to do with the L3/L4 overlays, so they move from test_go_dataflow.py to test_go_analysis.py beside the other model tests. The second now pins ValidationError specifically rather than bare Exception, and says why the fix listed the missing kind instead of widening the field to str -- widening would have turned a loud failure into silence for every future kind. GoType's docstring now names the analyzer file rather than a line number that will drift, and states the consequence that makes this severe: an unlisted kind fails the whole document, not just the one type. Verified the regression test fails without the one-line fix and passes with it, so it measures the fix rather than merely accompanying it. Refs #436
1 parent 7f1bca3 commit b04ec12

3 files changed

Lines changed: 53 additions & 26 deletions

File tree

‎cldk/models/go/models.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,11 +281,15 @@ class GoType(_NullSafeBase):
281281
discriminated by ``kind`` rather than a ``Union`` of subclasses. ``base_types`` are embedded
282282
types; ``fields`` are the struct's data fields (empty for every other kind).
283283
284-
The four kinds the analyzer emits (``symbol_table.go:432``): ``struct``, ``interface``,
285-
``defined`` (``type Celsius float64``) and ``alias`` (``type Alias = int``). This is a
286-
``Literal`` rather than an open ``str`` so a *new* kind is a loud failure and not a silent
287-
mis-read — ``extra="ignore"`` does not cover a ``Literal`` mismatch, which is exactly why the
288-
narrower spelling refused to parse real ``-a 1`` output (python-sdk#436).
284+
The four kinds the analyzer emits (``internal/syntactic_analysis/symbol_table.go``):
285+
``struct``, ``interface``, ``defined`` (``type Celsius float64``) and ``alias``
286+
(``type Alias = int``).
287+
288+
This stays a ``Literal`` rather than an open ``str`` so a *new* kind is a loud failure and not
289+
a silent mis-read — but note what that costs: ``extra="ignore"`` does not cover a ``Literal``
290+
mismatch, so an unlisted kind fails the **whole document**, not just the one type. That is why
291+
omitting ``alias`` refused real ``-a 1`` output from any project declaring a type alias
292+
(python-sdk#436), independently of the analysis level or the dataflow overlays.
289293
"""
290294

291295
id: str

‎tests/analysis/go/test_go_analysis.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,3 +270,44 @@ def test_real_corpus_call_graph_dangling_free(litestream_facade):
270270
assert g.number_of_edges() == 3082
271271
for src, dst in g.edges():
272272
assert "id" in g.nodes[src] and "id" in g.nodes[dst]
273+
274+
275+
# -----[ GoType.kind accepts every kind the analyzer emits ]-----
276+
#
277+
# ``kind`` is a ``Literal``, and ``extra="ignore"`` does not cover a ``Literal`` mismatch: an
278+
# unlisted kind fails the WHOLE document rather than dropping one field. So the listed set has to
279+
# be exactly the analyzer's, and a gap there is not a cosmetic omission — it is a project that does
280+
# not load at all. These live here rather than with the dataflow tests because the fact is about
281+
# ``-a 1`` output and the model that mirrors it, not about the L3/L4 overlays.
282+
FOUR_KINDS = Path(__file__).resolve().parents[2] / "resources" / "go" / "dataflow" / "four_type_kinds.json"
283+
284+
285+
def test_all_four_type_kinds_parse():
286+
"""``type Alias = int`` and ``type Celsius float64`` are ordinary Go, and cango emits them as
287+
``alias`` / ``defined`` (``internal/syntactic_analysis/symbol_table.go``). Listing only
288+
``struct``/``interface``/``defined`` made ``GoAnalysis`` refuse real ``-a 1`` output from any
289+
project declaring an alias — nothing to do with the analysis level or the dataflow overlays.
290+
"""
291+
from cldk.models.go import GoAnalysis
292+
293+
analysis = GoAnalysis.model_validate_json(FOUR_KINDS.read_text(encoding="utf-8"))
294+
kinds = {ty.kind for module in analysis.application.symbol_table.values() for ty in module.types.values()}
295+
assert kinds == {"struct", "interface", "defined", "alias"}
296+
297+
298+
def test_an_unknown_fifth_kind_still_fails_loudly():
299+
"""The ``Literal`` is a deliberate choice, not an oversight, so this pins the cost side of it:
300+
a kind the analyzer adds must be a *visible* failure rather than a value silently coerced or
301+
ignored, because every accessor keyed on ``kind`` would otherwise be quietly wrong.
302+
303+
Which is also why the fix was to list the missing kind rather than to widen the field to
304+
``str``: widening would have turned this failure into silence for every future kind too.
305+
"""
306+
from cldk.models.go import GoAnalysis
307+
from pydantic import ValidationError
308+
309+
doc = json.loads(FOUR_KINDS.read_text(encoding="utf-8"))
310+
module = next(iter(doc["application"]["symbol_table"].values()))
311+
next(iter(module["types"].values()))["kind"] = "type_parameter"
312+
with pytest.raises(ValidationError, match="kind"):
313+
GoAnalysis(**doc)

‎tests/analysis/go/test_go_dataflow.py‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,30 +73,12 @@ def backend(httpsql):
7373

7474

7575
# ======================================================================================
76-
# The parse blocker (#436's first commit): GoType.kind is a Literal, so a kind the SDK
77-
# does not list fails the WHOLE document rather than dropping a field.
76+
# The L3/L4 overlays survive validation. The *parse blocker* that shipped alongside them
77+
# (GoType.kind omitting "alias") is a model-level fact about -a 1 output with nothing to
78+
# do with dataflow, so it is pinned in test_go_analysis.py beside the other model tests.
7879
# ======================================================================================
7980

8081

81-
def test_all_four_type_kinds_parse():
82-
"""``type Alias = int`` made ``GoAnalysis(**doc)`` raise, which blocked ``-a 1`` output from any
83-
project declaring an alias — nothing to do with dataflow. All four kinds must load."""
84-
analysis = GoAnalysis.model_validate_json(FOUR_KINDS.read_text(encoding="utf-8"))
85-
kinds = {ty.kind for module in analysis.application.symbol_table.values() for ty in module.types.values()}
86-
assert kinds == {"struct", "interface", "defined", "alias"}
87-
88-
89-
def test_a_fifth_kind_is_refused_loudly_rather_than_dropped():
90-
"""The ``Literal`` is deliberate, not an oversight: a kind the analyzer adds must be a visible
91-
failure here and not a value silently coerced or ignored, because every accessor keyed on
92-
``kind`` would then be quietly wrong. ``extra="ignore"`` does not cover a ``Literal``."""
93-
doc = json.loads(FOUR_KINDS.read_text(encoding="utf-8"))
94-
module = next(iter(doc["application"]["symbol_table"].values()))
95-
next(iter(module["types"].values()))["kind"] = "type_parameter"
96-
with pytest.raises(Exception, match="kind"):
97-
GoAnalysis(**doc)
98-
99-
10082
def test_the_level_four_fixture_round_trips_losslessly():
10183
"""Every overlay the dataflow surface reads must survive validation — the regression that
10284
matters is the *silent* one, where a level-4 document parses and the graph is simply gone."""

0 commit comments

Comments
 (0)