From 2caf51a4c6556a9d60d330758b1397ef7b804d95 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 31 Oct 2026 21:31:23 +0100 Subject: [PATCH] fix(elixir): a folded multi-clause function carries the metrics of every clause Since #2371 the clauses of a multi-clause function fold into one Function node whose span covers all of them, but only the first clause was measured: the folded node kept the complexity, cognitive, loop and access metrics and the line count of that clause while its span grew. fold_elixir_clause now receives the metrics of each clause it folds and adds them to the node: cyclomatic, cognitive and loop_count add up, loop_depth and max_access_depth take the deepest clause, every clause past the first adds one dispatch decision to cyclomatic, and lines is recomputed from the folded span. A clause whose walk reports CBM_WALK_METRIC_UNAVAILABLE makes the folded value unavailable rather than a partial sum. This is the treatment proposed in the PR description and agreed in the review thread. Indexed elixir-lang/elixir (669f8481, 6703 Elixir Function nodes) against the merge parent: the 2212 folded functions change, the 4491 single-clause functions keep identical metrics, Functions with complexity above 0 go from 3412 to 4785, and every node now has lines equal to its span (4491 did before). Signed-off-by: Martin Vogel --- internal/cbm/extract_defs.c | 39 +++++++++++++++++++++++++--- tests/test_extraction.c | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 1acd9d93b..8f759e035 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -9714,6 +9714,24 @@ static TSNode elixir_call_args(TSNode node) { return args; } +/* Combine one folded clause's metric with the function's value so far. A clause + * whose walk could not finish reports CBM_WALK_METRIC_UNAVAILABLE, and so does + * the folded function: a sum that skipped that clause would read low on exactly + * the largest functions. */ +static int elixir_fold_metric_sum(int acc, int clause) { + if (acc < 0 || clause < 0) { + return CBM_WALK_METRIC_UNAVAILABLE; + } + return acc + clause; +} + +static int elixir_fold_metric_max(int acc, int clause) { + if (acc < 0 || clause < 0) { + return CBM_WALK_METRIC_UNAVAILABLE; + } + return clause > acc ? clause : acc; +} + // Fold one more clause of an Elixir function into the def already pushed for // the previous clause. Every clause is its own `def` call, and an Elixir QN // carries neither module nor arity, so all of them compute one qualified name @@ -9792,9 +9810,10 @@ static TSNode elixir_call_args(TSNode node) { // name, where the pre-patch last-clause-wins record reported a publicly // callable name as unexported. Verified against the source in every one: the // new value is the correct one. +// `cx` is this clause's own complexity; a folded clause adds it to the function. // Returns true when the clause folded. static bool fold_elixir_clause(CBMExtractCtx *ctx, const char *qn, TSNode scope, TSNode node, - uint32_t end_line, bool is_exported) { + uint32_t end_line, bool is_exported, const cbm_complexity_t *cx) { if (!qn || ctx->result->defs.count <= 0 || ts_node_is_null(scope)) { return false; } @@ -9819,6 +9838,18 @@ static bool fold_elixir_clause(CBMExtractCtx *ctx, const char *qn, TSNode scope, prev->end_line = end_line; } prev->is_exported = prev->is_exported || is_exported; + /* One node now stands for every clause, so its metrics cover every clause + * rather than the first: branch and loop counts add up, the nesting depths + * take the deepest clause, and each clause past the first is one more + * dispatch decision. A multi-clause function with no branch in any body is + * still a decision tree over its heads. `lines` follows the widened span. */ + int cyclomatic = elixir_fold_metric_sum(prev->complexity, cx->cyclomatic); + prev->complexity = cyclomatic < 0 ? cyclomatic : cyclomatic + 1; + prev->cognitive = elixir_fold_metric_sum(prev->cognitive, cx->cognitive); + prev->loop_count = elixir_fold_metric_sum(prev->loop_count, cx->loop_count); + prev->loop_depth = elixir_fold_metric_max(prev->loop_depth, cx->loop_depth); + prev->max_access_depth = elixir_fold_metric_max(prev->max_access_depth, cx->max_access_depth); + prev->lines = (int)(prev->end_line - prev->start_line + TS_LINE_OFFSET); return true; } @@ -10010,7 +10041,9 @@ static void extract_elixir_func_def(CBMExtractCtx *ctx, TSNode node, const char uint32_t start_line = ts_node_start_point(node).row + TS_LINE_OFFSET; uint32_t end_line = ts_node_end_point(node).row + TS_LINE_OFFSET; bool is_exported = (strcmp(macro, "def") == 0 || strcmp(macro, "defmacro") == 0); - bool folded = fold_elixir_clause(ctx, qn, *scope, node, end_line, is_exported); + cbm_complexity_t cx; + elixir_compute_complexity(node, ctx->source, &cx); + bool folded = fold_elixir_clause(ctx, qn, *scope, node, end_line, is_exported, &cx); *scope = ts_node_parent(node); if (folded) { return; @@ -10026,8 +10059,6 @@ static void extract_elixir_func_def(CBMExtractCtx *ctx, TSNode node, const char def.end_line = end_line; def.is_exported = is_exported; def.lines = (int)(def.end_line - def.start_line + TS_LINE_OFFSET); - cbm_complexity_t cx; - elixir_compute_complexity(node, ctx->source, &cx); def.complexity = cx.cyclomatic; def.cognitive = cx.cognitive; def.loop_count = cx.loop_count; diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 8e7231ccd..29fd1cc3c 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -2810,6 +2810,57 @@ TEST(elixir_complexity_and_fingerprint) { PASS(); } +/* The clauses of a multi-clause function fold into one Function node, so that + * node's metrics must cover every clause, not just the first: branch, cognitive + * and loop counts add up, loop_depth and max_access_depth take the deepest + * clause, every clause past the first is one more dispatch decision, and + * `lines` is the folded span. `single` repeats the second clause on its own as + * the per-clause reference: it is not folded, so it carries no dispatch charge. */ +TEST(elixir_folded_clause_metrics_cover_every_clause) { + CBMFileResult *r = extract("defmodule Route do\n" /* 1 */ + " def pick(0), do: Map.new()\n" /* 2 */ + " def pick(n) when n > 0 do\n" /* 3 */ + " for x <- [n], do: Map.new([{x, x}])\n" /* 4 */ + " end\n" /* 5 */ + " def pick(rows) do\n" /* 6 */ + " for row <- rows, do: (for x <- row, do: x)\n" /* 7 */ + " end\n" /* 8 */ + " def single(n) when n > 0 do\n" /* 9 */ + " for x <- [n], do: Map.new([{x, x}])\n" /* 10 */ + " end\n" /* 11 */ + "end\n", /* 12 */ + CBM_LANG_ELIXIR, "t", "route.ex"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + ASSERT_EQ(1, count_defs_named(r, "Function", "pick")); + const CBMDefinition *one = elixir_first_def(r, "single"); + ASSERT_NOT_NULL(one); + /* `when` + `for`, each at nesting 0; one comprehension; Map.new is one dot */ + ASSERT_EQ(2, one->complexity); + ASSERT_EQ(2, one->cognitive); + ASSERT_EQ(1, one->loop_count); + ASSERT_EQ(1, one->loop_depth); + ASSERT_EQ(1, one->max_access_depth); + ASSERT_EQ(3, one->lines); + + const CBMDefinition *pick = elixir_first_def(r, "pick"); + ASSERT_NOT_NULL(pick); + ASSERT_EQ(2, (int)pick->start_line); + ASSERT_EQ(8, (int)pick->end_line); + /* clauses 0 + 2 + 2, plus 2 for the two clauses past the first */ + ASSERT_EQ(6, pick->complexity); + /* 0 + 2 + (1 + 2): the inner comprehension sits one level deep */ + ASSERT_EQ(5, pick->cognitive); + ASSERT_EQ(3, pick->loop_count); + /* the deepest clause, not the sum of the clauses (that would be 3) */ + ASSERT_EQ(2, pick->loop_depth); + /* two clauses with one dot each: the deepest is 1, the sum would be 2 */ + ASSERT_EQ(1, pick->max_access_depth); + ASSERT_EQ(7, pick->lines); + cbm_free_result(r); + PASS(); +} + /* The Elixir complexity walk keeps its pending nodes on the growable stack * cbm_compute_complexity uses. A fixed 4096-frame stack dropped the FIRST * children of any wider body (they are pushed last-to-first), so the largest @@ -14649,6 +14700,7 @@ SUITE(extraction) { RUN_TEST(extract_elixir_guarded_def_head_guard_usages); RUN_TEST(elixir_channels_pubsub_and_handle_in); RUN_TEST(elixir_complexity_and_fingerprint); + RUN_TEST(elixir_folded_clause_metrics_cover_every_clause); RUN_TEST(elixir_complexity_wide_body_exceeds_old_stack_cap); RUN_TEST(haskell_function); RUN_TEST(haskell_infix_definition_name);