From 1d6a140f233d9938cf3b16d0d4d758b708d69aaa Mon Sep 17 00:00:00 2001 From: Tony Thayer-Osborne Date: Mon, 3 Aug 2026 10:48:24 -0700 Subject: [PATCH] feat(pkl): extract calls, branches, throws and decorators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pkl was wired end-to-end but shallow: on a real-world 172-file Pkl corpus the graph had 386 IMPORTS edges and 0 CALLS edges, because pkl_call_types was empty_types. Pkl has no dedicated call node. `unqualifiedAccessExpr` and `qualifiedAccessExpr` are the same node whether they are a call (`helper(a)`) or a bare property read (`host`) — the only discriminator is an `argumentList` child. extract_pkl_callee gates on that child, and is dispatched from extract_callee_name with an unconditional return: falling through to field-based or generic first-identifier resolution would mint a CALLS edge for every property read in every Pkl file, since a bare access expr's first child is an identifier. Callee resolution: helper(a) -> "helper" utils.fallback(a) -> "utils.fallback" (module-qualified; cbm.c shortens to the last dotted segment) s.trim().toLowerCase() -> "toLowerCase" (a receiver that is itself a call has parens in its text and is not prefixed) new Server { ... } -> "Server" (links to the class def) Also fills the other empty slots in the Pkl spec, all confirmed against real parse trees rather than guessed: typeAlias (class), importGlobClause / importExpr (import), ifExpr / whenGenerator / forGenerator (branch), throwExpr (throw), annotation (decorator). forGenerator is additionally registered in cbm_is_loop_node_type so `for (x in xs)` counts toward loop-nesting depth; the name is Pkl-unique so no other grammar collides. The Pkl repro test moves from the structural battery (dims 1-5) to the full callable battery (dims 1-8) and adds an inline negative assertion that bare property reads do NOT become CALLS edges — the regression the argumentList gate exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Tony Thayer-Osborne --- internal/cbm/extract_calls.c | 68 ++++++++++++++++++++++++++ internal/cbm/helpers.c | 31 +++--------- internal/cbm/lang_specs.c | 22 ++++++--- tests/repro/repro_grammar_config.c | 78 ++++++++++++++++++++++-------- 4 files changed, 150 insertions(+), 49 deletions(-) diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index abfe21140..c1b3d0464 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -1065,6 +1065,65 @@ static char *extract_nickel_callee(CBMArena *a, TSNode node, const char *source, return NULL; } +// Pkl: `unqualifiedAccessExpr` / `qualifiedAccessExpr` are the same node whether +// they are a call (`helper(a)`) or a bare property read (`host`) — the only +// discriminator is an `argumentList` child, so both are gated on it. For a +// qualified call the method name is the `identifier` child that is not the +// `receiver`; the receiver is prefixed only when it is itself a plain name +// (`utils.fallback(a)` -> "utils.fallback", module-qualified, which cbm.c +// shortens to the last dotted segment when resolving). A receiver that is itself +// a call must NOT be prefixed: `s.trim().toLowerCase()` -> "toLowerCase", since +// the receiver's text carries parens and would never resolve. +// `newExpr` resolves to its `declaredType` so `new Server {}` links to the class. +static char *extract_pkl_callee(CBMArena *a, TSNode node, const char *source, const char *nk) { + if (strcmp(nk, "newExpr") == 0) { + // `new { ... }` with an inferred type has no declaredType child. + TSNode dt = cbm_find_child_by_kind(node, "declaredType"); + return ts_node_is_null(dt) ? NULL : cbm_node_text(a, dt, source); + } + + bool qualified = strcmp(nk, "qualifiedAccessExpr") == 0; + if (!qualified && strcmp(nk, "unqualifiedAccessExpr") != 0) { + return NULL; + } + // No argument list -> property read, not a call. + if (ts_node_is_null(cbm_find_child_by_kind(node, "argumentList"))) { + return NULL; + } + + TSNode recv = ts_node_child_by_field_name(node, TS_FIELD("receiver")); + TSNode name = (TSNode){0}; + uint32_t nc = ts_node_named_child_count(node); + for (uint32_t i = 0; i < nc; i++) { + TSNode child = ts_node_named_child(node, i); + if (!ts_node_is_null(recv) && ts_node_eq(child, recv)) { + continue; + } + if (strcmp(ts_node_type(child), "identifier") == 0) { + name = child; + break; + } + } + if (ts_node_is_null(name)) { + return NULL; + } + char *mn = cbm_node_text(a, name, source); + if (!mn || !mn[0]) { + return NULL; + } + if (!qualified || ts_node_is_null(recv)) { + return mn; + } + if (strcmp(ts_node_type(recv), "unqualifiedAccessExpr") == 0 && + ts_node_is_null(cbm_find_child_by_kind(recv, "argumentList"))) { + char *rt = cbm_node_text(a, recv, source); + if (rt && rt[0]) { + return cbm_arena_sprintf(a, "%s.%s", rt, mn); + } + } + return mn; +} + // Typst: a `call` node's callee is its `item` field (an ident), matching the // def-side resolution of `#let greet(name) = ...`. static char *extract_typst_callee(CBMArena *a, TSNode node, const char *source, const char *nk) { @@ -1692,6 +1751,15 @@ static char *extract_callee_name(CBMArena *a, TSNode node, const char *source, C } } + /* Pkl: resolve here and return unconditionally — the access-expr call node + * types double as plain property reads, so falling through to field-based or + * generic first-identifier resolution would mint a CALLS edge for every + * property read (a bare `host` has an `identifier` first child, which the + * generic fallback would happily emit). NULL here means "not a call". */ + if (lang == CBM_LANG_PKL) { + return extract_pkl_callee(a, node, source, ts_node_type(node)); + } + // Helm / Go templates: resolve `include "x"` / `template "x"` to the // referenced named template so it links to the define'd Function (#338). if (lang == CBM_LANG_GOTEMPLATE) { diff --git a/internal/cbm/helpers.c b/internal/cbm/helpers.c index 46d3fda6a..d7fbf82a4 100644 --- a/internal/cbm/helpers.c +++ b/internal/cbm/helpers.c @@ -580,29 +580,14 @@ int cbm_count_branching(TSNode node, const char **branching_types) { // Loop node-type names across tree-sitter grammars, for loop-nesting depth. bool cbm_is_loop_node_type(const char *kind) { - static const char *const loops[] = {"for_statement", - "while_statement", - "do_statement", - "do_while_statement", - "for_in_statement", - "for_of_statement", - "for_each_statement", - "foreach_statement", - "enhanced_for_statement", - "for_range_loop", - "c_style_for_statement", - "for_expression", - "while_expression", - "loop_expression", - "while_let_expression", - "repeat_statement", - "repeat_while_statement", - "until", - "while_modifier", - "until_modifier", - "for", - "while", - NULL}; + static const char *const loops[] = { + "for_statement", "while_statement", "do_statement", "do_while_statement", + "for_in_statement", "for_of_statement", "for_each_statement", "foreach_statement", + "enhanced_for_statement", "for_range_loop", "c_style_for_statement", "for_expression", + "while_expression", "loop_expression", "while_let_expression", "repeat_statement", + "repeat_while_statement", + // Pkl: `for (x in xs) { ... }` inside an object body. + "forGenerator", "until", "while_modifier", "until_modifier", "for", "while", NULL}; for (const char *const *l = loops; *l; l++) { if (strcmp(kind, *l) == 0) { return true; diff --git a/internal/cbm/lang_specs.c b/internal/cbm/lang_specs.c index 040700702..89a355347 100644 --- a/internal/cbm/lang_specs.c +++ b/internal/cbm/lang_specs.c @@ -1586,11 +1586,21 @@ static const char *tlaplus_branch_types[] = {"if_then_else", "case", NULL}; static const char *tlaplus_var_types[] = {"variable_declaration", NULL}; static const char *tlaplus_module_types[] = {"source_file", NULL}; static const char *pkl_func_types[] = {"classMethod", "objectMethod", NULL}; -static const char *pkl_class_types[] = {"clazz", NULL}; -static const char *pkl_import_types[] = {"importClause", "extendsOrAmendsClause", "extends", - "import", NULL}; +static const char *pkl_class_types[] = {"clazz", "typeAlias", NULL}; +static const char *pkl_import_types[] = { + "importClause", "importGlobClause", "importExpr", "extendsOrAmendsClause", + "extends", "import", NULL}; static const char *pkl_var_types[] = {"classProperty", "objectProperty", NULL}; static const char *pkl_module_types[] = {"module", NULL}; +/* Both access exprs double as plain property reads; extract_pkl_callee keeps + * only the ones carrying an argumentList. `newExpr` resolves to its type. */ +static const char *pkl_call_types[] = {"unqualifiedAccessExpr", "qualifiedAccessExpr", "newExpr", + NULL}; +/* Control-flow only, matching every other spec (short-circuit operators are + * deliberately excluded). `forGenerator` is also a loop — see helpers.c. */ +static const char *pkl_branch_types[] = {"ifExpr", "whenGenerator", "forGenerator", NULL}; +static const char *pkl_throw_types[] = {"throwExpr", NULL}; +static const char *pkl_decorator_types[] = {"annotation", NULL}; static const char *gomod_var_types[] = {"require_directive", "replace_directive", NULL}; static const char *gomod_import_types[] = {"require", NULL}; static const char *gomod_module_types[] = {"source_file", NULL}; @@ -2616,9 +2626,9 @@ static const CBMLangSpec lang_specs[CBM_LANG_COUNT] = { // CBM_LANG_PKL [CBM_LANG_PKL] = {CBM_LANG_PKL, pkl_func_types, pkl_class_types, empty_types, pkl_module_types, - empty_types, pkl_import_types, empty_types, empty_types, pkl_var_types, - empty_types, empty_types, NULL, empty_types, NULL, NULL, tree_sitter_pkl, - NULL}, + pkl_call_types, pkl_import_types, empty_types, pkl_branch_types, + pkl_var_types, empty_types, pkl_throw_types, NULL, pkl_decorator_types, NULL, + NULL, tree_sitter_pkl, NULL}, // CBM_LANG_GOMOD [CBM_LANG_GOMOD] = {CBM_LANG_GOMOD, empty_types, empty_types, empty_types, gomod_module_types, diff --git a/tests/repro/repro_grammar_config.c b/tests/repro/repro_grammar_config.c index 9b143cfe3..c76f811b1 100644 --- a/tests/repro/repro_grammar_config.c +++ b/tests/repro/repro_grammar_config.c @@ -45,7 +45,8 @@ * 6. calls-extracted : inv_has_call(r, callee) == 1. * Only asserted for languages that have non-empty * call_types: HCL (function_call), NICKEL (infix_expr), - * JSONNET (functioncall), STARLARK (call). + * JSONNET (functioncall), STARLARK (call), + * PKL (unqualified/qualifiedAccessExpr, newExpr). * * FULL-PIPELINE (rh_index_files -> cbm_store_t*, via inv_count_* store helpers): * 7. callable-sourcing : inv_count_calls_by_source(store,project,&mod,&call). @@ -85,11 +86,15 @@ * Dims 1-5 ("Class"). No calls. * XML -- class_types = element -> "Class". Dims 1-5 ("Class"). No calls. * PROPERTIES -- var_types = property -> "Variable". Dims 1-5 ("Variable"). No calls. - * PKL -- func_types = classMethod/objectMethod -> "Function"; - * class_types = clazz -> "Class"; var_types = classProperty/objectProperty. - * call_types = empty_types. Dims 1-5 ("Function", "Class"). No call dim. * * LANGUAGES WITH CALLABLES (dims 1-6 + R, and pipeline dims 7-8 where applicable): + * PKL -- func_types = classMethod/objectMethod -> "Function"; + * class_types = clazz/typeAlias -> "Class"; + * var_types = classProperty/objectProperty; + * call_types = unqualifiedAccessExpr/qualifiedAccessExpr/newExpr. + * Dims 1-8. The access-expr call types double as property reads, + * so extract_pkl_callee gates them on an `argumentList` child; + * the test adds an inline negative assertion for that gate. * HCL -- class_types = block -> "Class"; var_types = attribute; * call_types = function_call. Dims 1-6. No func_types so no pipeline * dim 7 (calls would be module-sourced with no Function anchor). @@ -766,39 +771,72 @@ TEST(repro_grammar_config_ron) { /* ── PKL ────────────────────────────────────────────────────────────────────── * Idiomatic PKL (Apple Pkl) module with a class definition - * (pkl_class_types = {"clazz"} -> "Class"), a method inside it + * (pkl_class_types = {"clazz", "typeAlias"} -> "Class"), methods inside it * (pkl_func_types = {"classMethod", "objectMethod"} -> "Function"), and * class properties (pkl_var_types = {"classProperty", "objectProperty"}). - * pkl_call_types = empty_types so no call extraction occurs. + * pkl_call_types = {"unqualifiedAccessExpr", "qualifiedAccessExpr", "newExpr"}. * - * Dims asserted: 1-5 + R ("Class" for the class def, "Function" for the method). - * Dims 6-8 SKIPPED: call_types = empty_types in spec. - * Expected GREEN: dims 1-5. Dim 5 RED would indicate clazz->Class or - * classMethod->Function mapping is broken in the PKL grammar walker. + * Dims asserted: 1-8 (full battery) + R. + * Dim 6 GREEN: `makeUrl(host, port)` inside url() extracts callee "makeUrl". + * Dim 7 GREEN: every call site in the fixture is inside a classMethod body, so + * no CALLS edge is Module-sourced. (Real-world Pkl does call at module level; + * the fixture deliberately avoids it because dim 7 treats Module-sourced + * in-body calls as the enclosing-func gap.) + * Dim 8 GREEN: makeUrl and Server are both defined in-file, so neither the + * unqualified call nor the newExpr constructor edge dangles. + * + * PKL-SPECIFIC REGRESSION (asserted inline below): `unqualifiedAccessExpr` and + * `qualifiedAccessExpr` are the same node for a call and for a bare property + * read, so the interpolated `host` / `port` reads inside makeUrl must NOT be + * emitted as CALLS. extract_pkl_callee gates on an `argumentList` child; without + * that gate every property read in every Pkl file becomes a call edge. */ TEST(repro_grammar_config_pkl) { static const char src[] = "module cbm.Config\n" "\n" - "function makeUrl(host: String, port: Int): String = \"http://\\(host):\\(port)\"\n" + "typealias Port = Int\n" + "\n" + "function makeUrl(host: String, port: Port): String = \"http://\\(host):\\(port)\"\n" "\n" "class Server {\n" " host: String = \"localhost\"\n" - " port: Int = 8080\n" + " port: Port = 8080\n" " tls: Boolean = false\n" "\n" - " function url(): String = \"http://\\(host):\\(port)\"\n" - "}\n" + " function url(): String = makeUrl(host, port)\n" "\n" - "server = new Server {\n" - " host = \"0.0.0.0\"\n" - " port = 9000\n" + " function clone(): Server = new Server { host = host }\n" "}\n"; static const char bad[] = "module cbm.Config\nclass Server {\n host:"; - if (config_struct_battery("PKL", src, CBM_LANG_PKL, "config.pkl", - "Class", "Function") != 0) + if (config_callable_battery("PKL", src, CBM_LANG_PKL, "config.pkl", + "Function", "makeUrl") != 0) return 1; - return config_robustness("PKL", bad, CBM_LANG_PKL, "config.pkl"); + + /* Bare property reads must not be calls (see PKL-SPECIFIC REGRESSION above). */ + CBMFileResult *pr = inv_rx(src, CBM_LANG_PKL, "config.pkl"); + if (!pr) { + printf(" %sFAIL%s [PKL] inv_rx returned NULL\n", tf_red(), tf_reset()); + return 1; + } + int bogus = 0; + for (int i = 0; i < pr->calls.count; i++) { + const char *cn = pr->calls.items[i].callee_name; + if (cn && (strcmp(cn, "host") == 0 || strcmp(cn, "port") == 0 || + strcmp(cn, "tls") == 0)) { + bogus++; + } + } + cbm_free_result(pr); + if (bogus != 0) { + printf(" %sFAIL%s [PKL] property-read-not-call: %d bare property read(s) " + "emitted as a CALLS edge\n", tf_red(), tf_reset(), bogus); + return 1; + } + + if (config_robustness("PKL", bad, CBM_LANG_PKL, "config.pkl") != 0) + return 1; + return config_pipeline_battery("PKL", "config.pkl", src); } /* ── NICKEL ───────────────────────────────────────────────────────────────────