Skip to content

Commit cac0d1a

Browse files
Support inheritance detection in Python Extractor (#175)
* test(python): cover canonical inheritance targets Co-authored-by: Cursor <cursoragent@cursor.com> * fix(python): resolve inheritance targets canonically This is the resolution of the bug that inheritance in Python was not resolved to an "implements" edge. The fix adds a post-processing step to the Python extractor: + Collect all parsed symbols and index them by short class name. + Resolve unambiguous inheritance targets to fully qualified symbols. + Preserve ambiguous targets unchanged to avoid incorrect edges. * fix(python): use imports to resolve inheritance bases Resolving ambiguous symbol names during inheritance detection by taking the import context into account. * fix(python): preserve inheritance fact compatibility Fixing regression in the golden test. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 776b84b commit cac0d1a

4 files changed

Lines changed: 182 additions & 0 deletions

File tree

internal/extractors/pythonextractor/python.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ func (e *PythonExtractor) Extract(ctx context.Context, repoPath string, files []
176176
fileModules[strings.TrimSuffix(f, ".py")] = true
177177
}
178178
resolveCallTargets(allFacts, fileModules, pkgDirs)
179+
resolveImplementsTargets(allFacts, fileModules, pkgDirs)
179180

180181
// Fold FastAPI include_router mount prefixes onto the bare decorator paths, so
181182
// a route reads as the path it actually serves ("/api/v1/cognify") rather than

internal/extractors/pythonextractor/python_ast.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ func (w *pyWalker) handleFromImport(node *sitter.Node) {
477477
// flag them as orphans. Kept only here to avoid bloating every from-import.
478478
isInit := w.relFile == "__init__.py" || strings.HasSuffix(w.relFile, "/__init__.py")
479479
var reexported []string
480+
bindings := make(map[string]string)
480481

481482
// Map each imported name to a resolvable target or "" (external).
482483
for i := uint(0); i < uint(node.ChildCount()); i++ {
@@ -501,6 +502,9 @@ func (w *pyWalker) handleFromImport(node *sitter.Node) {
501502
importedName = pyText(c, w.src)
502503
localName = importedName
503504
}
505+
if localName != "" && importedName != "" && importedName != "*" {
506+
bindings[localName] = importedName
507+
}
504508

505509
if isInit && importedName != "" && importedName != "*" {
506510
reexported = append(reexported, importedName)
@@ -522,6 +526,9 @@ func (w *pyWalker) handleFromImport(node *sitter.Node) {
522526
}
523527
}
524528

529+
if len(bindings) > 0 {
530+
depProps["bindings"] = bindings
531+
}
525532
if len(reexported) > 0 {
526533
depProps["reexports"] = reexported
527534
}

internal/extractors/pythonextractor/resolve.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,86 @@ func resolveCallTargets(allFacts []facts.Fact, fileModules map[string]bool, pkgD
113113
}
114114
}
115115

116+
func resolveImplementsTargets(allFacts []facts.Fact, fileModules map[string]bool, pkgDirs map[string]bool) {
117+
fileIdx := buildSuffixIndex(fileModules, pkgDirs)
118+
topPkgs := importableRoots(fileModules, pkgDirs)
119+
reexports := buildReexportIndex(allFacts, pkgDirs)
120+
symbols := make(map[string]bool)
121+
byShortName := make(map[string][]string)
122+
importedByFile := make(map[string]map[string]string)
123+
for i := range allFacts {
124+
if allFacts[i].Kind == facts.KindDependency && allFacts[i].Props["from"] == true {
125+
var module string
126+
for _, rel := range allFacts[i].Relations {
127+
if rel.Kind == facts.RelImports {
128+
module = rel.Target
129+
break
130+
}
131+
}
132+
if module != "" {
133+
if importedByFile[allFacts[i].File] == nil {
134+
importedByFile[allFacts[i].File] = make(map[string]string)
135+
}
136+
for local, imported := range stringMapProp(allFacts[i].Props, "bindings") {
137+
target := module + "." + imported
138+
if previous, exists := importedByFile[allFacts[i].File][local]; exists && previous != target {
139+
importedByFile[allFacts[i].File][local] = ""
140+
} else {
141+
importedByFile[allFacts[i].File][local] = target
142+
}
143+
}
144+
}
145+
}
146+
if allFacts[i].Kind != facts.KindSymbol {
147+
continue
148+
}
149+
name := allFacts[i].Name
150+
symbols[name] = true
151+
short := name
152+
if dot := strings.LastIndexByte(name, '.'); dot >= 0 {
153+
short = name[dot+1:]
154+
}
155+
byShortName[short] = append(byShortName[short], name)
156+
}
157+
for i := range allFacts {
158+
if allFacts[i].Kind == facts.KindDependency && allFacts[i].Props != nil {
159+
delete(allFacts[i].Props, "bindings")
160+
}
161+
}
162+
163+
for i := range allFacts {
164+
f := &allFacts[i]
165+
if f.Kind != facts.KindSymbol {
166+
continue
167+
}
168+
for j := range f.Relations {
169+
rel := &f.Relations[j]
170+
if rel.Kind != facts.RelImplements {
171+
continue
172+
}
173+
if symbols[rel.Target] {
174+
continue
175+
}
176+
if strings.ContainsRune(rel.Target, '.') {
177+
if resolved, keep := resolveDottedTarget(rel.Target, fileIdx, topPkgs, fileDir(f.File), reexports, symbols); keep && symbols[resolved] {
178+
rel.Target = resolved
179+
}
180+
continue
181+
}
182+
if imported := importedByFile[f.File][rel.Target]; imported != "" {
183+
if resolved, keep := resolveDottedTarget(imported, fileIdx, topPkgs, fileDir(f.File), reexports, symbols); keep && symbols[resolved] {
184+
rel.Target = resolved
185+
}
186+
continue
187+
}
188+
candidates := byShortName[rel.Target]
189+
if len(candidates) == 1 {
190+
rel.Target = candidates[0]
191+
}
192+
}
193+
}
194+
}
195+
116196
// isDottedCallTarget reports whether a call target is an unresolved dotted path
117197
// (e.g. "a.b.c") rather than an already-resolved slash symbol name
118198
// ("dir/mod.sym") or a bare short name ("Foo").
@@ -242,6 +322,23 @@ func stringSliceProp(props map[string]any, key string) []string {
242322
return nil
243323
}
244324

325+
func stringMapProp(props map[string]any, key string) map[string]string {
326+
out := make(map[string]string)
327+
switch v := props[key].(type) {
328+
case map[string]string:
329+
for k, value := range v {
330+
out[k] = value
331+
}
332+
case map[string]any:
333+
for k, value := range v {
334+
if s, ok := value.(string); ok {
335+
out[k] = s
336+
}
337+
}
338+
}
339+
return out
340+
}
341+
245342
// resolveDottedTarget maps a dotted call target ("a.b.c.sym") to a canonical slash
246343
// symbol name when its module prefix resolves to an internal file or to a package
247344
// that re-exports the symbol, keeps it dotted when the prefix is internal but

internal/extractors/pythonextractor/resolve_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,83 @@ func TestResolveCallTargets_AbsoluteInternal_RewritesToSlashSymbol(t *testing.T)
283283
}
284284
}
285285

286+
func TestExtract_PythonResolvesInheritanceTarget(t *testing.T) {
287+
dir := t.TempDir()
288+
files := map[string]string{
289+
"pkg/__init__.py": "",
290+
"pkg/wrongparent.py": "class Parent:\n pass\n",
291+
"pkg/parent.py": "class Parent:\n pass\n",
292+
"pkg/child.py": "from pkg.parent import Parent\n\nclass Child(Parent):\n pass\n",
293+
}
294+
var rel []string
295+
for name, content := range files {
296+
full := filepath.Join(dir, name)
297+
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
298+
t.Fatal(err)
299+
}
300+
if err := os.WriteFile(full, []byte(content), 0o644); err != nil {
301+
t.Fatal(err)
302+
}
303+
rel = append(rel, name)
304+
}
305+
306+
all, err := New().Extract(context.Background(), dir, rel)
307+
if err != nil {
308+
t.Fatalf("Extract: %v", err)
309+
}
310+
311+
for _, f := range all {
312+
if f.Kind != facts.KindSymbol || f.Name != "pkg/child.Child" {
313+
continue
314+
}
315+
if hasRel(f, facts.RelImplements, "pkg/parent.Parent") {
316+
return
317+
}
318+
t.Fatalf("Child inheritance target = %v, want pkg/parent.Parent", f.Relations)
319+
}
320+
t.Fatal("missing pkg/child.Child symbol")
321+
}
322+
323+
func TestExtract_PythonResolvesMultipleInheritanceTargets(t *testing.T) {
324+
dir := t.TempDir()
325+
files := map[string]string{
326+
"pkg/__init__.py": "",
327+
"pkg/father.py": "class Father:\n pass\n",
328+
"pkg/mother.py": "class Mother:\n pass\n",
329+
"pkg/child.py": "from pkg.father import Father\nfrom pkg.mother import Mother\n\nclass Child(Father, Mother):\n pass\n",
330+
}
331+
var rel []string
332+
for name, content := range files {
333+
full := filepath.Join(dir, name)
334+
if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
335+
t.Fatal(err)
336+
}
337+
if err := os.WriteFile(full, []byte(content), 0o644); err != nil {
338+
t.Fatal(err)
339+
}
340+
rel = append(rel, name)
341+
}
342+
343+
all, err := New().Extract(context.Background(), dir, rel)
344+
if err != nil {
345+
t.Fatalf("Extract: %v", err)
346+
}
347+
348+
for _, f := range all {
349+
if f.Kind != facts.KindSymbol || f.Name != "pkg/child.Child" {
350+
continue
351+
}
352+
if !hasRel(f, facts.RelImplements, "pkg/father.Father") {
353+
t.Errorf("Child missing Father inheritance target: %v", f.Relations)
354+
}
355+
if !hasRel(f, facts.RelImplements, "pkg/mother.Mother") {
356+
t.Errorf("Child missing Mother inheritance target: %v", f.Relations)
357+
}
358+
return
359+
}
360+
t.Fatal("missing pkg/child.Child symbol")
361+
}
362+
286363
func TestResolveCallTargets_External_DropsEdge(t *testing.T) {
287364
fileModules := modSet("airflow-core/src/airflow/models/dag")
288365
ff := []facts.Fact{

0 commit comments

Comments
 (0)