Skip to content

Commit dac31d2

Browse files
authored
Merge pull request #815 from DeusData/distill/699-receiver-recursion
fix(extract): receiver-aware self-recursion detection
2 parents 2e7c64b + 39a0917 commit dac31d2

2 files changed

Lines changed: 208 additions & 2 deletions

File tree

internal/cbm/cbm.c

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,77 @@ static bool is_alloc_name(const char *n) {
439439
return name_in_set(n, set);
440440
}
441441

442+
// Extract the receiver identifier from a def's receiver text — Go's
443+
// "(s *Store)" / "(s Store)" → "s". Stores the identifier start in *out and
444+
// returns its length; returns 0 for unnamed receivers ("(*Store)", "(Store)"),
445+
// where no second token follows the identifier (a lone token is the TYPE, not
446+
// a name — such methods have no receiver variable to call through anyway).
447+
static size_t receiver_ident(const char *recv_text, const char **out) {
448+
const char *p = recv_text;
449+
if (*p == '(') {
450+
p++;
451+
}
452+
while (*p == ' ' || *p == '\t') {
453+
p++;
454+
}
455+
const char *start = p;
456+
while ((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || (*p >= '0' && *p <= '9') ||
457+
*p == '_') {
458+
p++;
459+
}
460+
size_t len = (size_t)(p - start);
461+
if (len == 0) {
462+
return 0; // "(*Store)": leading '*', no identifier
463+
}
464+
while (*p == ' ' || *p == '\t') {
465+
p++;
466+
}
467+
if (*p == ')' || *p == '\0') {
468+
return 0; // "(Store)": single token is the type, receiver unnamed
469+
}
470+
*out = start;
471+
return len;
472+
}
473+
474+
// Whether a callee expression targets the same instance/class as the enclosing
475+
// def, i.e. counts as genuine self-recursion rather than a same-named call on a
476+
// different receiver. callee_name may be bare ("recur") or qualified
477+
// ("self.recur", "this.recur", "super().save", "axios.get", "self.obj.recur").
478+
//
479+
// Bare names have no receiver → assume self-call (free function calling itself
480+
// by bare name; preserves prior behavior). Qualified names: the receiver chain
481+
// is everything before the LAST '.', and the WHOLE chain must name the same
482+
// object — self/this/cls/@self, or the enclosing def's own receiver identifier
483+
// (Go: `s` in `func (s *Store) save()`, from CBMDefinition.receiver). Matching
484+
// the whole chain (not its first segment) keeps self.obj.recur() out: it
485+
// targets self's FIELD obj, a different object. super() is the parent class and
486+
// any other receiver (axios, console, ...) a different target. See #599.
487+
static bool is_self_receiver(const char *callee_name, const char *def_receiver) {
488+
if (!callee_name || !callee_name[0]) {
489+
return false;
490+
}
491+
const char *dot = strrchr(callee_name, '.');
492+
if (!dot) {
493+
return true; // bare name → self-recursion candidate
494+
}
495+
size_t rlen = (size_t)(dot - callee_name);
496+
static const char *const self_receivers[] = {"self", "this", "cls", "@self", NULL};
497+
for (int i = 0; self_receivers[i]; i++) {
498+
size_t sl = strlen(self_receivers[i]);
499+
if (rlen == sl && strncmp(callee_name, self_receivers[i], sl) == 0) {
500+
return true;
501+
}
502+
}
503+
if (def_receiver) {
504+
const char *rid = NULL;
505+
size_t ril = receiver_ident(def_receiver, &rid);
506+
if (ril > 0 && ril == rlen && strncmp(callee_name, rid, ril) == 0) {
507+
return true; // call through the enclosing method's own receiver
508+
}
509+
}
510+
return false; // super() / axios / console / self.obj / any other receiver
511+
}
512+
442513
// Count parameters from a signature string like "(int a, Foo* b, cb (*)(int,int))".
443514
// Fallback for languages where param_names isn't populated (e.g. C keeps only the
444515
// 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
772843
continue;
773844
}
774845
CBMDefinition *d = &result->defs.items[best];
775-
// callee_name may be bare ("recur") or qualified ("pkg.recur", "self.recur")
846+
// callee_name may be bare ("recur") or qualified ("self.recur",
847+
// "super().save", "axios.get"). A short-name match alone is not
848+
// self-recursion: the callee must also target the same object
849+
// (is_self_receiver), or super().save() inside save and axios.get
850+
// inside get are false positives (#599).
776851
const char *dot = strrchr(c->callee_name, '.');
777852
const char *callee_short = dot ? dot + 1 : c->callee_name;
778853
bool in_loop = c->loop_depth > 0;
779854

780-
if (strcmp(callee_short, d->name) == 0) {
855+
if (strcmp(callee_short, d->name) == 0 && is_self_receiver(c->callee_name, d->receiver)) {
781856
// Direct self-recursion. The call graph omits self-edges (pass_calls
782857
// skips source==target), so detect it here; seeds "recursive".
783858
d->is_recursive = true;

tests/test_extraction.c

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2987,6 +2987,132 @@ TEST(complexity_guarded_recursion) {
29872987
PASS();
29882988
}
29892989

2990+
/* #599: super().save() inside a method named save is a parent-class call —
2991+
* the receiver is super(), never self — so it must NOT flag self-recursion.
2992+
* super()-ONLY fixture: no self.save() alongside, so the assertion cannot pass
2993+
* vacuously off a genuine self-call. */
2994+
TEST(complexity_super_only_not_recursive) {
2995+
CBMFileResult *r = extract("class B(A):\n"
2996+
" def save(self):\n"
2997+
" super().save()\n",
2998+
CBM_LANG_PYTHON, "t", "super_only.py");
2999+
ASSERT_NOT_NULL(r);
3000+
ASSERT_FALSE(r->has_error);
3001+
const CBMDefinition *d = find_def(r, "save");
3002+
ASSERT_NOT_NULL(d);
3003+
ASSERT_FALSE(d->is_recursive); /* parent-class call, not self-recursion */
3004+
ASSERT_FALSE(d->unguarded_recursion);
3005+
cbm_free_result(r);
3006+
PASS();
3007+
}
3008+
3009+
/* #599: a same-named call on an unrelated receiver (axios.get inside a
3010+
* function also named get) is delegation, not self-recursion. */
3011+
TEST(complexity_same_name_other_receiver_not_recursive) {
3012+
CBMFileResult *r = extract("function get(url) {\n"
3013+
" return axios.get(url);\n"
3014+
"}\n",
3015+
CBM_LANG_JAVASCRIPT, "t", "axios_get.js");
3016+
ASSERT_NOT_NULL(r);
3017+
ASSERT_FALSE(r->has_error);
3018+
const CBMDefinition *d = find_def(r, "get");
3019+
ASSERT_NOT_NULL(d);
3020+
ASSERT_FALSE(d->is_recursive); /* axios.get targets axios, not this fn */
3021+
ASSERT_FALSE(d->unguarded_recursion);
3022+
cbm_free_result(r);
3023+
PASS();
3024+
}
3025+
3026+
/* Guard: genuine self-recursion through self/this receivers still trips the
3027+
* detector after the receiver-aware narrowing (#599). */
3028+
TEST(complexity_self_receiver_still_recursive) {
3029+
/* Python: self.recur() — same object. */
3030+
CBMFileResult *r = extract("class C:\n"
3031+
" def recur(self, n):\n"
3032+
" if n > 0:\n"
3033+
" self.recur(n - 1)\n",
3034+
CBM_LANG_PYTHON, "t", "self_recur.py");
3035+
ASSERT_NOT_NULL(r);
3036+
ASSERT_FALSE(r->has_error);
3037+
const CBMDefinition *d = find_def(r, "recur");
3038+
ASSERT_NOT_NULL(d);
3039+
ASSERT_TRUE(d->is_recursive);
3040+
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if n > 0` */
3041+
cbm_free_result(r);
3042+
3043+
/* JS: this.step() — same object. */
3044+
r = extract("class C {\n"
3045+
" step(n) {\n"
3046+
" if (n > 0) { this.step(n - 1); }\n"
3047+
" }\n"
3048+
"}\n",
3049+
CBM_LANG_JAVASCRIPT, "t", "this_step.js");
3050+
ASSERT_NOT_NULL(r);
3051+
ASSERT_FALSE(r->has_error);
3052+
d = find_def(r, "step");
3053+
ASSERT_NOT_NULL(d);
3054+
ASSERT_TRUE(d->is_recursive);
3055+
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if (n > 0)` */
3056+
cbm_free_result(r);
3057+
PASS();
3058+
}
3059+
3060+
/* #599: chained receiver — self.obj.recur() inside recur targets self's FIELD
3061+
* obj, a different object. The whole receiver chain ("self.obj") must be
3062+
* compared, not just its first segment ("self"). */
3063+
TEST(complexity_chained_receiver_not_self) {
3064+
CBMFileResult *r = extract("class C:\n"
3065+
" def recur(self, n):\n"
3066+
" self.obj.recur(n)\n",
3067+
CBM_LANG_PYTHON, "t", "chained_recur.py");
3068+
ASSERT_NOT_NULL(r);
3069+
ASSERT_FALSE(r->has_error);
3070+
const CBMDefinition *d = find_def(r, "recur");
3071+
ASSERT_NOT_NULL(d);
3072+
ASSERT_FALSE(d->is_recursive); /* self.obj is not self */
3073+
ASSERT_FALSE(d->unguarded_recursion);
3074+
cbm_free_result(r);
3075+
PASS();
3076+
}
3077+
3078+
/* #599: Go method receiver — the enclosing def's own receiver identifier
3079+
* (`s` in `func (s *Store) save()`) is whitelisted dynamically from
3080+
* CBMDefinition.receiver, so s.save() still counts as self-recursion while
3081+
* s.backup.save() (a field's same-named method) does not. */
3082+
TEST(complexity_go_method_receiver_self_recursion) {
3083+
CBMFileResult *r = extract("package p\n"
3084+
"type Store struct{}\n"
3085+
"func (s *Store) save(n int) {\n"
3086+
" if n > 0 {\n"
3087+
" s.save(n - 1)\n"
3088+
" }\n"
3089+
"}\n",
3090+
CBM_LANG_GO, "t", "store.go");
3091+
ASSERT_NOT_NULL(r);
3092+
ASSERT_FALSE(r->has_error);
3093+
const CBMDefinition *d = find_def(r, "save");
3094+
ASSERT_NOT_NULL(d);
3095+
ASSERT_TRUE(d->is_recursive); /* s.save() == receiver s → self */
3096+
ASSERT_FALSE(d->unguarded_recursion); /* guarded by `if n > 0` */
3097+
cbm_free_result(r);
3098+
3099+
/* Same-named method on a field of the receiver: NOT self-recursion. */
3100+
r = extract("package p\n"
3101+
"type Store struct{ backup *Store }\n"
3102+
"func (s *Store) save(n int) {\n"
3103+
" s.backup.save(n)\n"
3104+
"}\n",
3105+
CBM_LANG_GO, "t", "store_backup.go");
3106+
ASSERT_NOT_NULL(r);
3107+
ASSERT_FALSE(r->has_error);
3108+
d = find_def(r, "save");
3109+
ASSERT_NOT_NULL(d);
3110+
ASSERT_FALSE(d->is_recursive); /* s.backup is not s */
3111+
ASSERT_FALSE(d->unguarded_recursion);
3112+
cbm_free_result(r);
3113+
PASS();
3114+
}
3115+
29903116
/* Deep chained member access + parameter count structure smells. */
29913117
TEST(complexity_access_depth_and_params) {
29923118
CBMFileResult *r = extract("package p\n"
@@ -3380,6 +3506,11 @@ SUITE(extraction) {
33803506
RUN_TEST(complexity_linear_scan_in_loop);
33813507
RUN_TEST(complexity_recursion_in_loop_unguarded);
33823508
RUN_TEST(complexity_guarded_recursion);
3509+
RUN_TEST(complexity_super_only_not_recursive);
3510+
RUN_TEST(complexity_same_name_other_receiver_not_recursive);
3511+
RUN_TEST(complexity_self_receiver_still_recursive);
3512+
RUN_TEST(complexity_chained_receiver_not_self);
3513+
RUN_TEST(complexity_go_method_receiver_self_recursion);
33833514
RUN_TEST(complexity_access_depth_and_params);
33843515
RUN_TEST(walk_defs_no_truncation_over_4096_issue668);
33853516

0 commit comments

Comments
 (0)