diff --git a/CHANGELOG.md b/CHANGELOG.md index ad3be1098..1b5dd83c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- A function or name passed as a value now resolves to what is in scope where it's written. A function nested inside another function is only reachable from inside it. In Python, a parameter or local of the same name is that local. A pytest fixture is a test's parameter only in its own module or under its `conftest.py`. Before, httpx's `self._build_auth(auth)` linked to an `auth` a test defines inside another function, and `auth_flow(self, request)` handing `request` on linked to the package's `request()` function. - In Python, an attribute passed as a value through an object nothing types, like netbox's `device=self.parent.device`, is no longer linked to the project's only method of that name when the object isn't named after the method's class. Before, netbox's model-field reads went to a GraphQL filter's `device` method, and healthchecks' `check.last_ping` went to a notification transport's. - In Scala, a kind-projector placeholder in a type, like `Align[Either[A, *]]`, is no longer linked to an operator method named `*`. A symbolic name in a type, like cats' `F ~> G`, now resolves only to a type. - C# type names now follow namespaces. A type is in reach from its own namespace or one nested inside it, or through a `using` of its namespace. Also counted: a project-wide `global using`, the project's `` entries, the SDK's implicit usings, or an alias that names it. Before, Newtonsoft's `async Task` tests linked `Task` to a test class of that name in another namespace, and AutoMapper's DTOs linked to another namespace's `BaseEntity`. diff --git a/__tests__/python-function-value-scope.test.ts b/__tests__/python-function-value-scope.test.ts new file mode 100644 index 000000000..d6152c78a --- /dev/null +++ b/__tests__/python-function-value-scope.test.ts @@ -0,0 +1,121 @@ +/** + * A name passed as a value means what is in scope where it is written. A + * function nested in another function is in scope only in there — httpx's + * `self._build_auth(auth)` passes its parameter, not the `auth` a test + * defines inside `test_custom_auth`. A Python name the function around it + * binds is that local's — `auth_flow(self, request)` handing `request` on is + * not the package's `request()`. And a pytest fixture is a test's parameter + * only in its own module or under its `conftest.py`. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-fnvalue-')); + const files: Record = { + 'httpx/_client.py': `class Client: + @property + def auth(self): + return self._auth + + def _build_auth(self, auth): + return auth + + def set_auth(self, auth): + self._auth = self._build_auth(auth) +`, + 'httpx/_api.py': `def request(method, url): + return method, url +`, + 'httpx/_auth.py': `class Auth: + def auth_flow(self, request): + yield request + + def sync_auth_flow(self, request): + flow = self.auth_flow(request) + return flow + + +class Recorder: + def request(self): + return None +`, + 'tests/test_auth.py': `def test_custom_auth(): + def auth(request): + return request + + return register(auth) + + +def test_basic_auth(): + auth = ("user", "pass") + return register(auth) +`, + 'docs/example/test_order.py': `import pytest + + +@pytest.fixture +def func(): + return 1 +`, + 'tests/test_coverage.py': `import pytest + + +def _run_both(func): + return func() + + +@pytest.fixture( + scope="module", +) +def recipe(): + return {} + + +def test_recipe(recipe): + return use(recipe) +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const targetsFrom = (file: string, qualifiedName?: string) => { + const ids = cg.getNodesInFile(file).filter((n) => !qualifiedName || n.qualifiedName === qualifiedName).map((n) => n.id); + return cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'references' || e.kind === 'calls') + .map((e) => cg.getNode(e.target)!).map((t) => `${t.filePath}:${t.qualifiedName}`); +}; + +describe('Python names passed as values', () => { + it('never reach a function nested in another function', () => { + expect(targetsFrom('httpx/_client.py')).not.toContain('tests/test_auth.py:test_custom_auth::auth'); + expect(targetsFrom('tests/test_auth.py', 'test_basic_auth')).not.toContain('tests/test_auth.py:test_custom_auth::auth'); + }); + + it('reach a nested function from inside its container', () => { + expect(targetsFrom('tests/test_auth.py', 'test_custom_auth')).toContain('tests/test_auth.py:test_custom_auth::auth'); + }); + + it('are the local a parameter binds', () => { + expect(targetsFrom('httpx/_auth.py')).not.toContain('httpx/_api.py:request'); + }); + + it('reach a fixture only from its own module', () => { + expect(targetsFrom('tests/test_coverage.py', '_run_both')).not.toContain('docs/example/test_order.py:func'); + expect(targetsFrom('tests/test_coverage.py', 'test_recipe')).toContain('tests/test_coverage.py:recipe'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 888ac17dd..e4f45f48a 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -523,7 +523,7 @@ export function matchFunctionRef( }; } - let candidates = context + const named = context .getNodesByName(ref.referenceName) .filter( (n) => @@ -533,7 +533,19 @@ export function matchFunctionRef( sameLanguageFamily(n.language, ref.language) && n.id !== ref.fromNodeId // a function registering itself is not a dependency edge ); + // A function declared inside another is in scope only in there: httpx's + // `self._build_auth(auth)` passes its own parameter, not the `auth` a test + // defines inside `test_custom_auth`. Those still count against a lone + // cross-file guess below — a name several functions use for themselves is + // as likely a local's. + let candidates = named.filter((n) => isLexicallyReachable(n, ref, context)); if (candidates.length === 0) return null; + // A Python name the function around it binds — a parameter, an assignment — + // is that local's value: httpx's `auth_flow(self, request)` handing `request` + // on is not the package's `request()` function. A pytest fixture is what a + // test's parameter of its name receives. + if (ref.language === 'python' && !candidates.some((n) => isFixtureInReach(n, ref.filePath, context)) && + isPythonLocallyBound(ref.referenceName, ref, context)) return null; // Swift implicit-self: a bare identifier can name a METHOD only of the // ENCLOSING type (`Button(action: handleTap)` written inside that type) — @@ -593,8 +605,11 @@ export function matchFunctionRef( } // Cross-file (imported names the import resolver didn't already claim): - // only an unambiguous match resolves. - if (candidates.length === 1) { + // only an unambiguous match resolves — or, in Python, the one in reach of + // a name the file imports (netbox's `sender=CustomField` beside a test's + // own nested `CustomField`). + if (candidates.length === 1 && (named.length === 1 || + (ref.language === 'python' && pythonFromImports(ref.filePath, context).has(ref.referenceName)))) { return { original: ref, targetNodeId: candidates[0]!.id, @@ -1557,7 +1572,7 @@ function fitsPythonCallShape(n: Node, shape: PythonCallShape, ref: UnresolvedRef if (isPythonNameImportedFromOutside(ref.referenceName, ref, context)) return false; // `view = UserView.as_view()` … `view(request)`: the file's own value — // unless it is a pytest fixture, which a test takes as a parameter of that name. - return isPytestFixture(n) || !isPythonLocallyBound(ref.referenceName, ref, context); + return isFixtureInReach(n, ref.filePath, context) || !isPythonLocallyBound(ref.referenceName, ref, context); } // A member of what the chain names: a method of a class of that name, or a // function / class in a module of that name (`helpers.slugify()`). @@ -1573,9 +1588,41 @@ function fitsPythonCallShape(n: Node, shape: PythonCallShape, ref: UnresolvedRef return stem === shape.owner || (stem === '__init__' && parts[parts.length - 2] === shape.owner); } -/** A pytest fixture: `@pytest.fixture` / `@fixture`, or anything a `conftest.py` defines. */ -function isPytestFixture(n: Node): boolean { - return /(?:^|\/)conftest\.py$/.test(n.filePath) || (n.decorators ?? []).some((d) => /(?:^|\.)fixture\b/.test(d)); +/** + * A pytest fixture: `@pytest.fixture` / `@fixture`, or anything a `conftest.py` + * defines. Python decorators are not kept on the node, so they are read from + * the lines above its `def` (a decorator's arguments may span lines). + */ +function isPytestFixture(n: Node, context: ResolutionContext): boolean { + if (/(?:^|\/)conftest\.py$/.test(n.filePath) || (n.decorators ?? []).some((d) => /(?:^|\.)fixture\b/.test(d))) return true; + return isDecoratedFixture(n, context); +} + +/** + * A fixture a test at `filePath` can take by name: one its own module defines, + * or one a `conftest.py` of its directory or a parent does. A test module's + * fixture is that module's alone — pytest's `_run_both(func)` is handing on its + * parameter, not a doc example's `func` fixture. + */ +function isFixtureInReach(n: Node, filePath: string, context: ResolutionContext): boolean { + if (n.filePath === filePath) return isPytestFixture(n, context); + const conftest = /^(.*?)(?:^|\/)conftest\.py$/.exec(n.filePath); + return conftest !== null && (conftest[1] === '' || filePath.startsWith(`${conftest[1]}/`)); +} + +function isDecoratedFixture(n: Node, context: ResolutionContext): boolean { + if (n.language !== 'python' || n.kind !== 'function') return false; + const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? []; + // Upward through the decorator lines: each one starts with `@`, or sits inside one's parentheses. + let open = 0; + for (let i = n.startLine - 2; i >= 0 && i >= n.startLine - 16; i--) { + const text = lines[i]?.trim() ?? ''; + open += (text.match(/\)/g)?.length ?? 0) - (text.match(/\(/g)?.length ?? 0); + if (open > 0) continue; + if (!text.startsWith('@')) return false; + if (/^@(?:\w+\.)*fixture\b/.test(text)) return true; + } + return false; } const PY_LOCAL_BINDS = new WeakMap>();