diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c288aa6e..0d6e83499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A file named the way Google names its tests, like `wire_format_unittest.cc` in protocolbuffers/protobuf, `logging_unittest.cc` in google/glog or a Python `foo_unittest.py`, now counts as a test, as `wire_format_test.cc` already did. Before, it was taken for production code: search and `codegraph_explore` ranked it alongside the code it tests, `codegraph affected` never listed it, and calls in production code could link to a class or type a unittest declares for itself, as google/breakpad's `stack_frame_entries_.size()` did to a unittest's `StackHelper`. Re-index projects that have such files after upgrading. - A file in a `unittests/` folder, where llvm/llvm-project and google/breakpad keep their unit tests, or in a folder named like a unittest, such as `dcheck_unittest/` in google/glog, now counts as a test whatever its own name says, like LLVM's `CountCopyAndMove.h` helper. Before, such a file was taken for production code unless its name said test, and none of them were kept apart from the program the way a `tests/` folder is: search and `codegraph_explore` ranked them alongside production code, `codegraph affected` missed them, and production code linked to a unit test's own declarations, as LLVM's `Ctx` and `OpInfo` variables did to ones its unit tests declare. A folder named just `unittest/`, like CPython's `Lib/unittest/`, is still production code. Re-index projects that have such folders after upgrading. - `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. - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading. - In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector>` or `Striped>`, 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. diff --git a/__tests__/sync-python-import-binding-retry.test.ts b/__tests__/sync-python-import-binding-retry.test.ts new file mode 100644 index 000000000..870d022d4 --- /dev/null +++ b/__tests__/sync-python-import-binding-retry.test.ts @@ -0,0 +1,289 @@ +/** + * A sync links a reference through a Python import alias once the module appears. + * + * `from .models import User as UserModel` and `from . import helpers as h` + * bind a name their module never declares: it has `User` and `helpers`. Every + * reference through that name — the import itself, a call, a base class, a + * class or function passed as a value — resolves only through the module. Sync + * retries a parked failed ref by its tail (#1240), and these were parked under + * the alias, which neither the module's file nor its symbols carry. So a module + * added after its importers were indexed, one restored after a delete, or one + * that gained the name in a later edit stayed unlinked from them until each + * 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. + */ +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 { moduleReferenceKeys } from '../src/db/reference-tail'; + +type Files = Record; + +/** The files that reach a module through a name the module declares under another. */ +const IMPORTERS: Files = { + 'app/__init__.py': '', + 'app/sub/__init__.py': '', + 'app/pkg/__init__.py': '', + // A package that re-exports a name from a module beside it. + 'app/store/__init__.py': 'from .user import Account\n', + 'app/main.py': [ + 'from .models import User as UserModel', + 'from . import helpers as h', + 'from app.models import Group as GroupModel', + 'from .pkg import sub as subpkg', + 'from app.pkg import other as other_mod', + 'from .svc import make_thing as mk', + 'from .store import Account as Acct', + '', + 'class Admin(UserModel):', + ' pass', + '', + 'def run(u):', + ' x = mk()', + ' if isinstance(u, UserModel):', + ' return GroupModel(), Acct(), x', + ' return mk', + '', + ].join('\n'), + 'app/sub/deep.py': [ + 'from .. import helpers as hh', + 'from ..models import User as U2', + 'from ..pkg import sub as sp2', + '', + 'def use():', + ' return U2()', + '', + ].join('\n'), + // A module at the root, imported by its absolute name. + 'main.py': 'from settings import Config as Cfg\n\n\ndef load():\n return Cfg()\n', +}; + +const MODULES: Files = { + 'app/models.py': 'class User:\n pass\n\n\nclass Group:\n pass\n', + 'app/helpers.py': 'def x():\n return 1\n', + // A subpackage: what `from .pkg import sub` imports is its `__init__.py`. + 'app/pkg/sub/__init__.py': 'VALUE = 1\n', + 'app/pkg/other.py': 'def f():\n return 2\n', + 'app/svc.py': 'def make_thing():\n return 3\n', + 'app/store/user.py': 'class Account:\n pass\n', + 'settings.py': 'class Config:\n pass\n', +}; + +/** What a full index links through the aliases: `: -> ::`. */ +const LINKS = [ + 'Acct: imports file app/main.py -> class app/store/user.py::Account', + 'Acct: instantiates function app/main.py -> class app/store/user.py::Account', + 'Cfg: imports file main.py -> class settings.py::Config', + 'Cfg: instantiates function main.py -> class settings.py::Config', + 'GroupModel: imports file app/main.py -> class app/models.py::Group', + 'GroupModel: instantiates function app/main.py -> class app/models.py::Group', + 'U2: imports file app/sub/deep.py -> class app/models.py::User', + 'U2: instantiates function app/sub/deep.py -> class app/models.py::User', + 'UserModel: extends class app/main.py -> class app/models.py::User', + 'UserModel: imports file app/main.py -> class app/models.py::User', + 'UserModel: references function app/main.py -> class app/models.py::User', + 'h: imports file app/main.py -> file app/helpers.py::helpers.py', + 'hh: imports file app/sub/deep.py -> file app/helpers.py::helpers.py', + 'mk: calls function app/main.py -> function app/svc.py::make_thing', + 'mk: imports file app/main.py -> function app/svc.py::make_thing', + 'mk: references function app/main.py -> function app/svc.py::make_thing', + 'other_mod: imports file app/main.py -> file app/pkg/other.py::other.py', + 'sp2: imports file app/sub/deep.py -> file app/pkg/sub/__init__.py::__init__.py', + 'subpkg: imports file app/main.py -> file app/pkg/sub/__init__.py::__init__.py', +]; + +/** An alias of a name its module doesn't have, which a full index links to a namesake. */ +const BY_NAME: Files = { + 'app/__init__.py': '', + 'app/lib.py': 'unrelated = 1\n', + 'app/form.py': 'from .lib import FormSchemaBase as FormSchema\n\n\nclass Form(FormSchema):\n pass\n', +}; +const BY_NAME_ADDED: Files = { 'app/schemas.py': 'class FormSchema:\n pass\n' }; + +let roots: string[] = []; +let graphs: CodeGraph[] = []; + +afterEach(() => { + for (const graph of graphs) graph.close(); + graphs = []; + for (const root of roots) fs.rmSync(root, { recursive: true, force: true }); + roots = []; +}); + +function tempRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-sync-pybinding-')); + roots.push(root); + return root; +} + +function write(root: string, files: Files): void { + for (const [file, text] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + } +} + +async function index(root: string): Promise { + const graph = await CodeGraph.init(root, { index: true }); + graphs.push(graph); + return graph; +} + +/** Every edge, by its ends' natural keys, with where it is written and how it resolved. */ +function edgesOf(graph: CodeGraph): string[] { + const keys = new Map( + graph.getFiles() + .flatMap((file) => graph.getNodesInFile(file.path)) + .map((n) => [n.id, `${n.kind} ${n.filePath}:${n.qualifiedName}:${n.startLine}`] as const) + ); + return graph.getOutgoingEdgesFrom([...keys.keys()]) + .map((e) => `${e.kind} ${keys.get(e.source)} -> ${keys.get(e.target) ?? e.target} @${e.line}:${e.column} ${JSON.stringify(e.metadata ?? {})}`) + .sort(); +} + +/** The {@link LINKS} the graph has. */ +function linksOf(graph: CodeGraph): string[] { + const names = new Set(LINKS.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)) { + const ref = edge.metadata?.refName; + if (typeof ref !== 'string' || !names.has(ref)) continue; + const source = graph.getNode(edge.source)!; + const target = graph.getNode(edge.target)!; + links.push(`${ref}: ${edge.kind} ${source.kind} ${source.filePath} -> ${target.kind} ${target.filePath}::${target.name}`); + } + return [...new Set(links)].sort(); +} + +/** The final tree, indexed from scratch in a folder of its own: what the sync has to match. */ +async function fullIndexOf(files: Files): Promise { + const root = tempRoot(); + write(root, files); + return edgesOf(await index(root)); +} + +describe('sync links references through a Python import alias to their module', () => { + it('when the module is added after its importers', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + const graph = await index(root); + expect(linksOf(graph)).toEqual([]); + + write(root, MODULES); + const result = await graph.sync(); + // The importers did not change: only the retry of their parked refs links them. + expect(result.filesAdded).toBe(Object.keys(MODULES).length); + expect(result.filesModified).toBe(0); + expect(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('when the module comes back after it was deleted', async () => { + const root = tempRoot(); + write(root, { ...IMPORTERS, ...MODULES }); + const graph = await index(root); + expect(linksOf(graph)).toEqual(LINKS); + + for (const file of Object.keys(MODULES)) fs.rmSync(path.join(root, file)); + expect((await graph.sync()).filesRemoved).toBe(Object.keys(MODULES).length); + expect(linksOf(graph)).toEqual([]); + + write(root, MODULES); + expect((await graph.sync()).filesAdded).toBe(Object.keys(MODULES).length); + expect(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('when an edit gives the module the name its importers import', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + write(root, Object.fromEntries(Object.keys(MODULES).map((file) => [file, 'PLACEHOLDER = 0\n']))); + const graph = await index(root); + + write(root, MODULES); + const result = await graph.sync(); + expect(result.filesModified).toBe(Object.keys(MODULES).length); + expect(result.filesAdded).toBe(0); + expect(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('and by its own name when its module never declares it', async () => { + // As vben's case for an ES module: the module has no such name, so a full + // index links the alias by its own name to the class that carries it. + const root = tempRoot(); + write(root, BY_NAME); + const graph = await index(root); + + write(root, BY_NAME_ADDED); + expect((await graph.sync()).filesAdded).toBe(1); + const formSchema = edgesOf(graph).filter((edge) => edge.includes('"refName":"FormSchema"')); + expect(formSchema).toHaveLength(2); + expect(formSchema.every((edge) => edge.includes('-> class app/schemas.py:'))).toBe(true); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...BY_NAME, ...BY_NAME_ADDED })); + }, 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. + 'app/other.py': [ + 'from .models import Group', + 'from collections import OrderedDict as OD', + 'from .models import User as UserModel', + '', + 'def build():', + ' return Group(), OD(), UserModel.create()', + '', + ].join('\n'), + }); + (await CodeGraph.init(root, { index: true })).close(); + + const db = DatabaseConnection.open(getDatabasePath(root)); + try { + const rows = db.getDb() + .prepare("SELECT reference_name AS name, reference_kind AS kind, file_path AS file, name_tail AS tail FROM unresolved_refs WHERE status = 'failed'") + .all() as Array<{ name: string; kind: string; file: string; tail: string }>; + const tailOf = (file: string, name: string, kind: string): string | undefined => + rows.find((r) => r.file === file && r.name === name && r.kind === kind)?.tail; + + // A module named relative to the importing file's package, or absolutely. + expect(tailOf('app/main.py', 'UserModel', 'imports')).toBe('module:models'); + expect(tailOf('app/main.py', 'UserModel', 'extends')).toBe('module:models'); + expect(tailOf('app/main.py', 'GroupModel', 'calls')).toBe('module:models'); + expect(tailOf('app/sub/deep.py', 'U2', 'calls')).toBe('module:models'); + expect(tailOf('app/main.py', 'mk', 'calls')).toBe('module:svc'); + // `from . import helpers` imports from the file's own package, `..` from the one above. + expect(tailOf('app/main.py', 'h', 'imports')).toBe('module:app'); + expect(tailOf('app/sub/deep.py', 'hh', 'imports')).toBe('module:app'); + expect(tailOf('app/main.py', 'subpkg', 'imports')).toBe('module:pkg'); + expect(tailOf('app/main.py', 'other_mod', 'imports')).toBe('module:pkg'); + expect(tailOf('app/main.py', 'Acct', 'calls')).toBe('module:store'); + expect(tailOf('main.py', 'Cfg', 'calls')).toBe('module:settings'); + // 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'); + expect(tailOf('app/other.py', 'OD', 'calls')).toBe('OD'); + expect(tailOf('app/other.py', 'UserModel.create', 'calls')).toBe('create'); + + // A submodule's keys hold its package's: a module file's folder, and + // the folder above a subpackage's `__init__.py`. + expect(moduleReferenceKeys('app/helpers.py')).toContain('module:app'); + expect(moduleReferenceKeys('app/pkg/sub/__init__.py')).toEqual(expect.arrayContaining(['module:sub', 'module:pkg'])); + expect(moduleReferenceKeys('pkg/__init__.py')).toEqual(['module:pkg', 'module:__init__.py', 'module:__init__']); + } finally { + db.close(); + } + }, 60_000); +}); diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index a0b954c59..2bdd6646a 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -127,5 +127,12 @@ export function importPathKeys(filePath: string): string[] { * name (`importBindingTail`), which is parked under the same keys. */ export function moduleReferenceKeys(filePath: string): string[] { - return importPathKeys(filePath).map((key) => MODULE_KEY + key); + const keys = importPathKeys(filePath); + // A Python package's `__init__.py` is a submodule of the package above it, + // as a module file beside it is through its folder: `app/pkg/sub/__init__.py` + // is what `from app.pkg import sub as s`, parked as 'module:pkg', imports. + const parts = filePath.split('/'); + const above = parts.length >= 3 && parts[parts.length - 1] === '__init__.py' ? parts[parts.length - 3]! : ''; + if (above && !keys.includes(above)) keys.push(above); + return keys.map((key) => MODULE_KEY + key); } diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 01cd934cd..ba9face41 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -3124,6 +3124,7 @@ export function isBoundToOutOfRepoImport( * keeps its tail. Undefined for every other reference. */ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { + if (ref.language === 'python') return pythonImportBindingTail(ref, context); if (!ESM_IMPORT_LANGUAGES.has(ref.language) || !IDENTIFIER.test(ref.referenceName)) return undefined; const binding = context.getImportMappings(ref.filePath, ref.language).find((m) => m.localName === ref.referenceName); if (!binding || (!binding.isDefault && !binding.isNamespace && binding.exportedName === binding.localName)) return undefined; @@ -3139,4 +3140,44 @@ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext return moduleTail(modulePath) || undefined; } +/** + * {@link importBindingTail} for Python: `from .models import User as + * UserModel` and `from . import helpers as h` bind a name their module has as + * `User` and `helpers`, `import app.tools as tl` one it has as `tools`. The + * module is the one the import names by its dotted path, counted from the + * importing file's own package when it starts with dots (`.` is that package, + * each further dot the one above): `.models` in `app/main.py` is `app/models`, + * parked as 'module:models', and `.` is `app`, 'module:app'. The name may + * import a submodule of it (`helpers` is `app/helpers.py`), whose keys hold + * the package's name too (moduleReferenceKeys). + * + * An unaliased import keeps its tail: the module, or the one it re-exports the + * name from, declares that name. So does an import of the standard library. An + * installed package's import can't be told from a module the project doesn't + * have yet (`from settings import Config as Cfg`), so it waits for its module + * too, and a sync still finds it by its own name. + */ +function pythonImportBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { + if (!IDENTIFIER.test(ref.referenceName)) return undefined; + const binding = context.getImportMappings(ref.filePath, 'python').find((m) => m.localName === ref.referenceName); + if (!binding) return undefined; + const declared = binding.isNamespace ? binding.source.slice(binding.source.lastIndexOf('.') + 1) : binding.exportedName; + if (declared === binding.localName || isExternalImport(binding.source, 'python', context)) return undefined; + const modulePath = pythonModulePath(binding.source, ref.filePath); + return modulePath ? moduleTail(modulePath) || undefined : undefined; +} + +/** + * A Python import's module as a project path: `app.models` → 'app/models'; + * from `app/sub/views.py`, `.models` → 'app/sub/models', `..` → 'app' and + * `..forms` → 'app/forms'. Null when its dots climb above the project. + */ +function pythonModulePath(source: string, fromFile: string): string | null { + const name = source.replace(/^\.+/, ''); + const dots = source.length - name.length; + if (dots === 0) return name.replace(/\./g, '/'); + const modulePath = path.posix.join(path.posix.dirname(fromFile), '../'.repeat(dots - 1), name.replace(/\./g, '/')); + return modulePath === '..' || modulePath.startsWith('../') ? null : modulePath; +} + const IDENTIFIER = /^[A-Za-z_$][\w$]*$/;