Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now do the same for a Python import under a name of its own, like `from .models import User as UserModel`, `from . import helpers as h` or `from app.pkg import sub as subpkg`: when the module or package is added after the importing file was indexed, comes back after being deleted, or gets the imported name in a later edit, the import and every use of the alias, such as a call, a base class or the class passed as a value, now link to it. Before, they stayed unlinked until the importing file changed or the project was re-indexed. Re-index Python projects after upgrading to pick up links an earlier version missed.

- `codegraph sync` and the file watcher now also link a Python import of a name a package re-exports, like `from fastapi import FastAPI` through `fastapi/__init__.py`, when that `__init__.py` is added after the importing files were indexed or gains the re-export in a later edit. Before, calls through the name stayed unlinked until each importing file changed or the project was re-indexed. Large projects gain the most: sync also used to skip, in any language, a module or package that more than 500 references were waiting for, and now restoring Django's `django/urls/__init__.py` relinks every file that imports `path` from it. Re-index Python projects after upgrading to pick up links an earlier version missed.
Comment thread
bompus marked this conversation as resolved.

- In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector<std::pair<int, FileMetaData*>>` or `Striped<CacheAlignedWrapper<port::Mutex>>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading.

- In C++, a class written with attribute macros the parser can't read now keeps its members. That covers several macros between `class` and the class name, like protocolbuffers/protobuf's generated `class PROTOBUF_EXPORT PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED Any final`, a macro with arguments there, like RocksDB's `struct ALIGN_AS(64U) HandleImpl`, or one before a template specialization, and macros on a member: in front of it (`PROTOBUF_FUTURE_ADD_EARLY_NODISCARD`), between a pointer and its name (`PROTOBUF_NONNULL`), after its parameters or its name (`ABSL_ATTRIBUTE_LIFETIME_BOUND`, google/leveldb's `LOCKS_EXCLUDED(mu_)` and `GUARDED_BY(mu_)`), or alone on a line above a comment ({fmt}'s `FMT_BEGIN_EXPORT`). Before, protobuf's generated message classes were missing and their members, like `GetDescriptor`, `GetReflection` and `unknown_fields`, were indexed as functions outside any class, and leveldb's annotated methods were indexed under the annotation's name, so calls to them linked to nothing. Re-index C++ projects after upgrading.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates.

## About this fork

This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`5171a6aa`](https://github.com/colbymchenry/codegraph/commit/5171a6aa) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests.
This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`a3fdb920`](https://github.com/colbymchenry/codegraph/commit/a3fdb920) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests.

The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below).

Expand Down
2 changes: 1 addition & 1 deletion __tests__/fixtures/golden/torture-multilang.dump
Original file line number Diff line number Diff line change
Expand Up @@ -4753,7 +4753,7 @@
{"from_node_id":"file:torture.py","reference_name":"sys","reference_kind":"imports","line":2,"col":11,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"sys"}
{"from_node_id":"file:torture.py","reference_name":"target_cb","reference_kind":"function_ref","line":48,"col":20,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"target_cb"}
{"from_node_id":"file:torture.py","reference_name":"target_cb","reference_kind":"imports","line":6,"col":27,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"target_cb"}
{"from_node_id":"file:torture.py","reference_name":"thing","reference_kind":"imports","line":5,"col":22,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"thing"}
{"from_node_id":"file:torture.py","reference_name":"thing","reference_kind":"imports","line":5,"col":22,"candidates":null,"file_path":"torture.py","language":"python","status":"failed","name_tail":"module:relative"}
{"from_node_id":"file:torture.rb","reference_name":"../shared/helper","reference_kind":"imports","line":4,"col":0,"candidates":null,"file_path":"torture.rb","language":"ruby","status":"failed","name_tail":"helper"}
{"from_node_id":"file:torture.rb","reference_name":"../shared/helper.rb","reference_kind":"imports","line":4,"col":0,"candidates":null,"file_path":"torture.rb","language":"ruby","status":"failed","name_tail":"helper"}
{"from_node_id":"file:torture.rb","reference_name":"interp/","reference_kind":"imports","line":8,"col":0,"candidates":null,"file_path":"torture.rb","language":"ruby","status":"failed","name_tail":"interp/"}
Expand Down
167 changes: 159 additions & 8 deletions __tests__/sync-python-import-binding-retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,20 @@
* importer changed or the project was indexed again. #2450 parked the ES-module
* bindings under their module; a Python import names its module by a dotted
* path, relative to the importing file's package when it starts with dots.
*
* An unaliased name has the same gap when a package re-exports it:
* `from fastapi import FastAPI` reaches `fastapi/applications.py` through
* `fastapi/__init__.py`, which declares no node named `FastAPI`. So a package
* `__init__.py` that appears after its importers, or gains the re-export,
* left them unlinked too.
*/
import { describe, it, expect, afterEach } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import CodeGraph from '../src/index';
import { DatabaseConnection, getDatabasePath } from '../src/db';
import { QueryBuilder } from '../src/db/queries';
import { moduleReferenceKeys } from '../src/db/reference-tail';

type Files = Record<string, string>;
Expand Down Expand Up @@ -104,6 +111,46 @@ const BY_NAME: Files = {
};
const BY_NAME_ADDED: Files = { 'app/schemas.py': 'class FormSchema:\n pass\n' };

/**
* Unaliased imports of what a package re-exports from the modules that declare
* it: absolutely (`from shop import Order`) and from the package above
* (`from .. import Order`). The package renames what it re-exports, so nothing
* named `Order` or `price` exists for a name lookup to find.
*/
const REEXPORT_IMPORTERS: Files = {
'shop/models.py': 'class _Order:\n pass\n',
'shop/services/__init__.py': '',
'shop/services/pricing.py': 'def base_price():\n return 1\n',
'shop/api/__init__.py': '',
'shop/api/views.py': 'from .. import Order\n\n\ndef view():\n return Order()\n',
'checkout.py': 'from shop import Order, price\n\n\ndef run():\n return Order(), price()\n',
};
const REEXPORT_PACKAGE: Files = {
'shop/__init__.py': 'from .models import _Order as Order\nfrom .services.pricing import base_price as price\n',
};
const REEXPORT_LINKS = [
'Order: imports file checkout.py -> class shop/models.py::_Order',
'Order: imports file shop/__init__.py -> class shop/models.py::_Order',
'Order: imports file shop/api/views.py -> class shop/models.py::_Order',
'Order: instantiates function checkout.py -> class shop/models.py::_Order',
'Order: instantiates function shop/api/views.py -> class shop/models.py::_Order',
'price: calls function checkout.py -> function shop/services/pricing.py::base_price',
'price: imports file checkout.py -> function shop/services/pricing.py::base_price',
'price: imports file shop/__init__.py -> function shop/services/pricing.py::base_price',
];

/** fastapi's shape: `from fastapi import FastAPI` through `from .applications import FastAPI as FastAPI`. */
const SAME_NAME: Files = {
'kit/applications.py': 'class Kit:\n pass\n',
'app.py': 'from kit import Kit\n\n\ndef make():\n return Kit()\n',
};
const SAME_NAME_PACKAGE: Files = { 'kit/__init__.py': 'from .applications import Kit as Kit\n' };
const SAME_NAME_LINKS = [
'Kit: imports file app.py -> class kit/applications.py::Kit',
'Kit: imports file kit/__init__.py -> class kit/applications.py::Kit',
'Kit: instantiates function app.py -> class kit/applications.py::Kit',
];

let roots: string[] = [];
let graphs: CodeGraph[] = [];

Expand Down Expand Up @@ -145,9 +192,9 @@ function edgesOf(graph: CodeGraph): string[] {
.sort();
}

/** The {@link LINKS} the graph has. */
function linksOf(graph: CodeGraph): string[] {
const names = new Set(LINKS.map((link) => link.slice(0, link.indexOf(':'))));
/** The links of `expected`'s reference names the graph has ({@link LINKS} by default). */
function linksOf(graph: CodeGraph, expected: string[] = LINKS): string[] {
const names = new Set(expected.map((link) => link.slice(0, link.indexOf(':'))));
const ids = graph.getFiles().flatMap((file) => graph.getNodesInFile(file.path)).map((n) => n.id);
const links: string[] = [];
for (const edge of graph.getOutgoingEdgesFrom(ids)) {
Expand All @@ -162,9 +209,13 @@ function linksOf(graph: CodeGraph): string[] {

/** The final tree, indexed from scratch in a folder of its own: what the sync has to match. */
async function fullIndexOf(files: Files): Promise<string[]> {
return edgesOf(await freshGraphOf(files));
}

async function freshGraphOf(files: Files): Promise<CodeGraph> {
const root = tempRoot();
write(root, files);
return edgesOf(await index(root));
return index(root);
}

describe('sync links references through a Python import alias to their module', () => {
Expand Down Expand Up @@ -229,21 +280,64 @@ describe('sync links references through a Python import alias to their module',
}, 120_000);
});

describe('sync links an unaliased Python import through the package that re-exports it', () => {
it('when the package is added after its importers', async () => {
const root = tempRoot();
write(root, REEXPORT_IMPORTERS);
const graph = await index(root);
expect(linksOf(graph, REEXPORT_LINKS)).toEqual([]);

write(root, REEXPORT_PACKAGE);
const result = await graph.sync();
expect(result.filesAdded).toBe(1);
expect(result.filesModified).toBe(0);
expect(linksOf(graph, REEXPORT_LINKS)).toEqual(REEXPORT_LINKS);
expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...REEXPORT_IMPORTERS, ...REEXPORT_PACKAGE }));
}, 120_000);

it('when an edit gives the package the re-export', async () => {
const root = tempRoot();
write(root, { ...REEXPORT_IMPORTERS, 'shop/__init__.py': '' });
const graph = await index(root);

write(root, REEXPORT_PACKAGE);
expect((await graph.sync()).filesModified).toBe(1);
expect(linksOf(graph, REEXPORT_LINKS)).toEqual(REEXPORT_LINKS);
expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...REEXPORT_IMPORTERS, ...REEXPORT_PACKAGE }));
}, 120_000);

it('when it re-exports the name its module declares', async () => {
// While the package is missing, the import itself is linked to `Kit` by
// name, which a full index links through the package: the same link.
const root = tempRoot();
write(root, SAME_NAME);
const graph = await index(root);

write(root, SAME_NAME_PACKAGE);
expect((await graph.sync()).filesAdded).toBe(1);
expect(linksOf(graph, SAME_NAME_LINKS)).toEqual(SAME_NAME_LINKS);
expect(linksOf(await freshGraphOf({ ...SAME_NAME, ...SAME_NAME_PACKAGE }), SAME_NAME_LINKS)).toEqual(SAME_NAME_LINKS);
}, 120_000);
});

describe('a failed reference through a Python import alias', () => {
it('waits for its module, and every other reference keeps its own tail', async () => {
const root = tempRoot();
write(root, {
...IMPORTERS,
...BY_NAME,
// An unaliased import, a standard-library alias and a member read
// through an alias wait for the name they use.
...REEXPORT_IMPORTERS,
// An unaliased import of an outside package, a standard-library alias
// and a member read through an alias wait for the name they use; an
// unaliased import of the project's own module waits for its module.
'app/other.py': [
'from .models import Group',
'from django.db.models import Q',
'from collections import OrderedDict as OD',
'from .models import User as UserModel',
'',
'def build():',
' return Group(), OD(), UserModel.create()',
' return Group(), Q(), OD(), UserModel.create()',
'',
].join('\n'),
});
Expand Down Expand Up @@ -273,7 +367,13 @@ describe('a failed reference through a Python import alias', () => {
// Still found by its own name too: sync's lookup by whole name.
expect(tailOf('app/form.py', 'FormSchema', 'extends')).toBe('module:lib');

expect(tailOf('app/other.py', 'Group', 'calls')).toBe('Group');
// An unaliased import of a module in the project's own folders.
expect(tailOf('app/other.py', 'Group', 'calls')).toBe('module:models');
expect(tailOf('checkout.py', 'Order', 'calls')).toBe('module:shop');
expect(tailOf('checkout.py', 'price', 'imports')).toBe('module:shop');
expect(tailOf('shop/api/views.py', 'Order', 'calls')).toBe('module:shop');

expect(tailOf('app/other.py', 'Q', 'calls')).toBe('Q');
expect(tailOf('app/other.py', 'OD', 'calls')).toBe('OD');
expect(tailOf('app/other.py', 'UserModel.create', 'calls')).toBe('create');

Expand All @@ -287,3 +387,54 @@ describe('a failed reference through a Python import alias', () => {
}
}, 60_000);
});

describe('the retry ceiling', () => {
let db: DatabaseConnection | undefined;

afterEach(() => {
db?.close();
db = undefined;
});

/** 600 failed calls of `FastAPI` parked under 'module:fastapi', and 600 of `get` under their name. */
function fixture(): QueryBuilder {
db = DatabaseConnection.initialize(path.join(tempRoot(), 'test.db'));
const queries = new QueryBuilder(db.getDb());
queries.insertNode({ id: 'f', kind: 'file', name: 'app.py', qualifiedName: 'app.py', filePath: 'app.py',
language: 'python', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 });
const refs = Array.from({ length: 1200 }, (_, i) => ({
fromNodeId: 'f', referenceName: i < 600 ? 'FastAPI' : 'get', referenceKind: 'calls' as const,
line: i + 1, column: 0, filePath: 'app.py', language: 'python' as const,
}));
queries.insertUnresolvedRefsBatch(refs);
db.getDb().exec("UPDATE unresolved_refs SET status = 'failed', name_tail = CASE reference_name WHEN 'FastAPI' THEN 'module:fastapi' ELSE 'get' END");
return queries;
}

it('lets every importer of a package that appeared retry, and still skips a common name', () => {
const queries = fixture();
// Every importer of the package waits for it: a large population is not noise.
expect(queries.getRetryableFailedReferences(['module:fastapi'])).toHaveLength(600);
// A name shared by 600 failed calls is, as before; so is the by-name lookup of parked rows.
expect(queries.getRetryableFailedReferences(['get'])).toHaveLength(0);
expect(queries.getRetryableFailedReferences(['FastAPI'])).toHaveLength(0);
// The module ceiling still bounds the work.
expect(queries.getRetryableFailedReferences(['module:fastapi'], 500, 599)).toHaveLength(0);
});

it("leaves the edges a sync's retry just made out of the rebind's count, not out of the rebind", () => {
// Django: relinking every `from django.urls import path` must not stop a
// new definition of `path` from rebinding the calls it may now answer.
const queries = fixture();
queries.insertNode({ id: 'p', kind: 'variable', name: 'path', qualifiedName: 'path', filePath: 'conf.py',
language: 'python', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 });
const edge = (line: number) => ({ source: 'f', target: 'p', kind: 'calls' as const, line, column: 0,
metadata: { refName: 'path', resolvedBy: 'exact-match', confidence: 0.7 } });
queries.insertEdges([edge(1)]);
const beforeRetry = queries.getMaxEdgeId();
queries.insertEdges(Array.from({ length: 600 }, (_, i) => edge(i + 2)));

expect(queries.getResolutionEdgesByTargetName(['path'])).toHaveLength(0);
expect(queries.getResolutionEdgesByTargetName(['path'], 500, beforeRetry)).toHaveLength(601);
});
});
8 changes: 7 additions & 1 deletion src/codegraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,10 @@ export class CodeGraph {

// Resolve references if files were updated
const filesChanged = result.filesAdded > 0 || result.filesModified > 0;
// The last edge before the failed-ref retry below: the rebind's
// per-name ceiling counts only edges up to it (see
// resurrectStaleResolutionEdges).
let edgesBeforeRetry: number | undefined;
if (filesChanged) {
if (result.changedFilePaths) {
// Scope resolution to changed files (git fast path — bounded set)
Expand Down Expand Up @@ -1056,6 +1060,7 @@ export class CodeGraph {
// On a sync where no failed ref
// matches, this is one indexed lookup.
const tRetry = Date.now();
edgesBeforeRetry = this.queries.getMaxEdgeId();
const retryable = this.queries.getRetryableFailedReferences([...new Set([
...this.queries.getNodeNamesByFiles(result.changedFilePaths),
...result.changedFilePaths.flatMap(moduleReferenceKeys),
Expand Down Expand Up @@ -1130,7 +1135,8 @@ export class CodeGraph {
const tRebind = Date.now();
const rebound = this.orchestrator.resurrectStaleResolutionEdges(
result.definitionDelta,
result.changedFilePaths ?? []
result.changedFilePaths ?? [],
edgesBeforeRetry
);
if (process.env.CODEGRAPH_SYNTH_TIMINGS) {
console.error(
Expand Down
Loading
Loading