fix(extract): receiver-aware self-recursion detection

The recursion detector flagged any call whose short name matched the
enclosing def as self-recursion, so super().save() inside save,
axios.get() inside get, and console.error() inside error were all
false positives feeding is_recursive, recursion_in_loop and
unguarded_recursion.

Add is_self_receiver() and AND it into the short-name comparison: a
qualified callee only counts when its whole receiver chain (everything
before the last '.') names the same object — self/this/cls/@self, or
the enclosing def's own receiver identifier parsed from
CBMDefinition.receiver (Go: s in 'func (s *Store) save()'). Bare names
keep the prior behavior. Matching the full chain keeps self.obj.recur()
(a field's same-named method) out, and the dynamic receiver whitelist
keeps Go s.save() detected.

Distilled from PR #699 with two corrections: the receiver chain is
matched via the last dot instead of the first segment (self.obj.recur
was still a false positive), and the enclosing-receiver whitelist so
Go method self-recursion is not lost.

Closes #599

Co-authored-by: Gen Li <lg320531124@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-07-03 20:20:49 +02:00
parent 09148ab121
commit 87091edd24
2 changed files with 208 additions and 2 deletions
+77 -2
View File
@@ -439,6 +439,77 @@ static bool is_alloc_name(const char *n) {
return name_in_set(n, set);
}
// Extract the receiver identifier from a def's receiver text — Go's
// "(s *Store)" / "(s Store)" → "s". Stores the identifier start in *out and
// returns its length; returns 0 for unnamed receivers ("(*Store)", "(Store)"),
// where no second token follows the identifier (a lone token is the TYPE, not
// a name — such methods have no receiver variable to call through anyway).
static size_t receiver_ident(const char *recv_text, const char **out) {
const char *p = recv_text;
if (*p == '(') {
p++;
}
while (*p == ' ' || *p == '\t') {
p++;
}
const char *start = p;
while ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || (*p >= '0' && *p <= '9') ||
*p == '_') {
p++;
}
size_t len = (size_t)(p - start);
if (len == 0) {
return 0; // "(*Store)": leading '*', no identifier
}
while (*p == ' ' || *p == '\t') {
p++;
}
if (*p == ')' || *p == '\0') {
return 0; // "(Store)": single token is the type, receiver unnamed
}
*out = start;
return len;
}
// Whether a callee expression targets the same instance/class as the enclosing
// def, i.e. counts as genuine self-recursion rather than a same-named call on a
// different receiver. callee_name may be bare ("recur") or qualified
// ("self.recur", "this.recur", "super().save", "axios.get", "self.obj.recur").
//
// Bare names have no receiver → assume self-call (free function calling itself
// by bare name; preserves prior behavior). Qualified names: the receiver chain
// is everything before the LAST '.', and the WHOLE chain must name the same
// object — self/this/cls/@self, or the enclosing def's own receiver identifier
// (Go: `s` in `func (s *Store) save()`, from CBMDefinition.receiver). Matching
// the whole chain (not its first segment) keeps self.obj.recur() out: it
// targets self's FIELD obj, a different object. super() is the parent class and
// any other receiver (axios, console, ...) a different target. See #599.
static bool is_self_receiver(const char *callee_name, const char *def_receiver) {
if (!callee_name || !callee_name[0]) {
return false;
}
const char *dot = strrchr(callee_name, '.');
if (!dot) {
return true; // bare name → self-recursion candidate
}
size_t rlen = (size_t)(dot - callee_name);
static const char *const self_receivers[] = {"self", "this", "cls", "@self", NULL};
for (int i = 0; self_receivers[i]; i++) {
size_t sl = strlen(self_receivers[i]);
if (rlen == sl && strncmp(callee_name, self_receivers[i], sl) == 0) {
return true;
}
}
if (def_receiver) {
const char *rid = NULL;
size_t ril = receiver_ident(def_receiver, &rid);
if (ril > 0 && ril == rlen && strncmp(callee_name, rid, ril) == 0) {
return true; // call through the enclosing method's own receiver
}
}
return false; // super() / axios / console / self.obj / any other receiver
}
// Count parameters from a signature string like "(int a, Foo* b, cb (*)(int,int))".
// Fallback for languages where param_names isn't populated (e.g. C keeps only the
// signature text). Counts commas at the top paren level; treats "()"/"(void)" as 0.
@@ -772,12 +843,16 @@ CBMFileResult *cbm_extract_file(const char *source, int source_len, CBMLanguage
continue;
}
CBMDefinition *d = &result->defs.items[best];
// callee_name may be bare ("recur") or qualified ("pkg.recur", "self.recur")
// callee_name may be bare ("recur") or qualified ("self.recur",
// "super().save", "axios.get"). A short-name match alone is not
// self-recursion: the callee must also target the same object
// (is_self_receiver), or super().save() inside save and axios.get
// inside get are false positives (#599).
const char *dot = strrchr(c->callee_name, '.');
const char *callee_short = dot ? dot + 1 : c->callee_name;
bool in_loop = c->loop_depth > 0;
if (strcmp(callee_short, d->name) == 0) {
if (strcmp(callee_short, d->name) == 0 && is_self_receiver(c->callee_name, d->receiver)) {
// Direct self-recursion. The call graph omits self-edges (pass_calls
// skips source==target), so detect it here; seeds "recursive".
d->is_recursive = true;
+131
View File
@@ -2987,6 +2987,132 @@ TEST(complexity_guarded_recursion) {
PASS();
}
/* #599: super().save() inside a method named save is a parent-class call —
* the receiver is super(), never self — so it must NOT flag self-recursion.
* super()-ONLY fixture: no self.save() alongside, so the assertion cannot pass
* vacuously off a genuine self-call. */
TEST(complexity_super_only_not_recursive) {
CBMFileResult *r = extract("class B(A):\n"
" def save(self):\n"
" super().save()\n",
CBM_LANG_PYTHON, "t", "super_only.py");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
const CBMDefinition *d = find_def(r, "save");
ASSERT_NOT_NULL(d);
ASSERT_FALSE(d->is_recursive); /* parent-class call, not self-recursion */
ASSERT_FALSE(d->unguarded_recursion);
cbm_free_result(r);
PASS();
}
/* #599: a same-named call on an unrelated receiver (axios.get inside a
* function also named get) is delegation, not self-recursion. */
TEST(complexity_same_name_other_receiver_not_recursive) {
CBMFileResult *r = extract("function get(url) {\n"
" return axios.get(url);\n"
"}\n",
CBM_LANG_JAVASCRIPT, "t", "axios_get.js");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
const CBMDefinition *d = find_def(r, "get");
ASSERT_NOT_NULL(d);
ASSERT_FALSE(d->is_recursive); /* axios.get targets axios, not this fn */
ASSERT_FALSE(d->unguarded_recursion);
cbm_free_result(r);
PASS();
}
/* Guard: genuine self-recursion through self/this receivers still trips the
* detector after the receiver-aware narrowing (#599). */
TEST(complexity_self_receiver_still_recursive) {
/* Python: self.recur() — same object. */
CBMFileResult *r = extract("class C:\n"
" def recur(self, n):\n"
" if n > 0:\n"
" self.recur(n - 1)\n",
CBM_LANG_PYTHON, "t", "self_recur.py");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
const CBMDefinition *d = find_def(r, "recur");
ASSERT_NOT_NULL(d);
ASSERT_TRUE(d->is_recursive);
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if n > 0` */
cbm_free_result(r);
/* JS: this.step() — same object. */
r = extract("class C {\n"
" step(n) {\n"
" if (n > 0) { this.step(n - 1); }\n"
" }\n"
"}\n",
CBM_LANG_JAVASCRIPT, "t", "this_step.js");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
d = find_def(r, "step");
ASSERT_NOT_NULL(d);
ASSERT_TRUE(d->is_recursive);
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if (n > 0)` */
cbm_free_result(r);
PASS();
}
/* #599: chained receiver — self.obj.recur() inside recur targets self's FIELD
* obj, a different object. The whole receiver chain ("self.obj") must be
* compared, not just its first segment ("self"). */
TEST(complexity_chained_receiver_not_self) {
CBMFileResult *r = extract("class C:\n"
" def recur(self, n):\n"
" self.obj.recur(n)\n",
CBM_LANG_PYTHON, "t", "chained_recur.py");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
const CBMDefinition *d = find_def(r, "recur");
ASSERT_NOT_NULL(d);
ASSERT_FALSE(d->is_recursive); /* self.obj is not self */
ASSERT_FALSE(d->unguarded_recursion);
cbm_free_result(r);
PASS();
}
/* #599: Go method receiver — the enclosing def's own receiver identifier
* (`s` in `func (s *Store) save()`) is whitelisted dynamically from
* CBMDefinition.receiver, so s.save() still counts as self-recursion while
* s.backup.save() (a field's same-named method) does not. */
TEST(complexity_go_method_receiver_self_recursion) {
CBMFileResult *r = extract("package p\n"
"type Store struct{}\n"
"func (s *Store) save(n int) {\n"
" if n > 0 {\n"
" s.save(n - 1)\n"
" }\n"
"}\n",
CBM_LANG_GO, "t", "store.go");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
const CBMDefinition *d = find_def(r, "save");
ASSERT_NOT_NULL(d);
ASSERT_TRUE(d->is_recursive); /* s.save() == receiver s → self */
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if n > 0` */
cbm_free_result(r);
/* Same-named method on a field of the receiver: NOT self-recursion. */
r = extract("package p\n"
"type Store struct{ backup *Store }\n"
"func (s *Store) save(n int) {\n"
" s.backup.save(n)\n"
"}\n",
CBM_LANG_GO, "t", "store_backup.go");
ASSERT_NOT_NULL(r);
ASSERT_FALSE(r->has_error);
d = find_def(r, "save");
ASSERT_NOT_NULL(d);
ASSERT_FALSE(d->is_recursive); /* s.backup is not s */
ASSERT_FALSE(d->unguarded_recursion);
cbm_free_result(r);
PASS();
}
/* Deep chained member access + parameter count structure smells. */
TEST(complexity_access_depth_and_params) {
CBMFileResult *r = extract("package p\n"
@@ -3380,6 +3506,11 @@ SUITE(extraction) {
RUN_TEST(complexity_linear_scan_in_loop);
RUN_TEST(complexity_recursion_in_loop_unguarded);
RUN_TEST(complexity_guarded_recursion);
RUN_TEST(complexity_super_only_not_recursive);
RUN_TEST(complexity_same_name_other_receiver_not_recursive);
RUN_TEST(complexity_self_receiver_still_recursive);
RUN_TEST(complexity_chained_receiver_not_self);
RUN_TEST(complexity_go_method_receiver_self_recursion);
RUN_TEST(complexity_access_depth_and_params);
RUN_TEST(walk_defs_no_truncation_over_4096_issue668);