mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-24 19:30:10 +08:00
fix(python): the module of a from-import no longer links a same-named variable (#2603)
The module of a Python `from X import ...` names a module or package, never a symbol, so `from app import email` no longer links the importing file to a project variable named app, like a FastAPI or Flask app = FastAPI(). A from-import module that resolves to anything but a file or module links nothing. On five Python repos this removes 12 wrong import links and nothing else. Fixes issue 2584. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
e48565067e
commit
4f222a3d65
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- In Python, `from app import email` no longer links the importing file to a project variable named `app`, such as a FastAPI or Flask `app = FastAPI()`. (#2584)
|
||||
- In Python, a call through a module-level instance, like `_svc.delete(obj)` after `_svc = SceneService()`, now links to the method its class has or inherits, even when other classes have a method of that name. (#2582)
|
||||
- In Python, a call on a module imported from outside your project, like `subprocess.run(…)` after `import subprocess`, no longer links to a project method that happens to share its name. Thanks @Egrn for the report. (#2599)
|
||||
- `codegraph_explore`, the CLI `codegraph explore` and the prompt hook no longer stall for a minute or more on a large project after a full index; the step that finds the project's busiest file no longer slows to a crawl when a few files are huge, like a generated Android `R.java` (#2571). Thanks @yangfengfan2011.
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
/**
|
||||
* The module of a Python `from X import y` names a module or package, never a
|
||||
* symbol: `from app import email` must not link the importing file to a
|
||||
* project variable that happens to be named `app` (#2584). The imported names
|
||||
* keep their edges.
|
||||
*/
|
||||
|
||||
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';
|
||||
|
||||
let tempDir: string;
|
||||
let cg: CodeGraph | null = null;
|
||||
|
||||
async function importEdges(files: Record<string, string>, fromFile: string): Promise<string[]> {
|
||||
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-2584-'));
|
||||
for (const [rel, source] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(tempDir, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(tempDir, rel), source);
|
||||
}
|
||||
cg = await CodeGraph.init(tempDir, { index: true });
|
||||
cg.resolveReferences();
|
||||
const file = cg.getNodesByKind('file').find((n) => n.filePath === fromFile)!;
|
||||
expect(file).toBeDefined();
|
||||
return cg
|
||||
.getOutgoingEdges(file.id)
|
||||
.filter((e) => e.kind === 'imports')
|
||||
.map((e) => cg!.getNode(e.target))
|
||||
.filter((n): n is NonNullable<typeof n> => !!n)
|
||||
.map((n) => `${n.kind}:${n.name}@${n.filePath}`)
|
||||
.sort();
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cg?.close();
|
||||
cg = null;
|
||||
fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const PKG = {
|
||||
'app/__init__.py': '',
|
||||
'app/email.py': 'def send():\n return 1\n',
|
||||
};
|
||||
|
||||
describe('the module of a Python from-import is never a symbol (#2584)', () => {
|
||||
it.each([
|
||||
['in the package', { 'app/main.py': 'app = object()\n' }],
|
||||
['at the project root', { 'server.py': 'app = object()\n' }],
|
||||
])('does not link a variable named like the package, %s', async (_label, extra) => {
|
||||
const out = await importEdges(
|
||||
{ ...PKG, ...extra, 'tests/test_email.py': 'from app import email\n\n\ndef test_send():\n email.send()\n' },
|
||||
'tests/test_email.py'
|
||||
);
|
||||
expect(out.filter((e) => e.startsWith('variable:'))).toEqual([]);
|
||||
expect(out).toContain('file:email.py@app/email.py');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['split after the module', 'from app \\\n import email\n'],
|
||||
['split after from', 'from \\\n app import email\n'],
|
||||
['under if TYPE_CHECKING', 'from typing import TYPE_CHECKING\n\nif TYPE_CHECKING:\n from app import email\n'],
|
||||
])('does not link a variable for a from-import %s', async (_label, code) => {
|
||||
const out = await importEdges({ ...PKG, 'app/main.py': 'app = object()\n', 'tests/test_email.py': code }, 'tests/test_email.py');
|
||||
expect(out.filter((e) => e.startsWith('variable:'))).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps an imported name that is a variable', async () => {
|
||||
const out = await importEdges(
|
||||
{ ...PKG, 'app/main.py': 'app = object()\n', 'run.py': 'from app.main import app\n' },
|
||||
'run.py'
|
||||
);
|
||||
expect(out).toContain('variable:app@app/main.py');
|
||||
});
|
||||
});
|
||||
@@ -21,7 +21,7 @@ import {
|
||||
isImportableKind,
|
||||
CPP_DEFINE_SIGNATURE,
|
||||
} from './types';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchByFilePath, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, isRustForeignImplCallable, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, GO_TYPE_KINDS, matchGoAssertedCall, isGoAssertedLocal, isGoLocallyBound } from './name-matcher';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchByFilePath, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, isRustForeignImplCallable, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, GO_TYPE_KINDS, matchGoAssertedCall, isGoAssertedLocal, isGoLocallyBound, isPythonFromImportModule } from './name-matcher';
|
||||
import { isVisibleCppMacro, clearCppMacroVisibility } from './cpp-macro-visibility';
|
||||
import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor';
|
||||
import { isCppSupertypeRef, matchCppSupertype, clearCppSupertypeMemos } from './cpp-supertypes';
|
||||
@@ -1255,6 +1255,10 @@ export class ReferenceResolver {
|
||||
// names itself: a self-edge that says nothing (#2327).
|
||||
if (resolved && ref.language === 'dart' && ref.referenceKind === 'references' &&
|
||||
resolved.targetNodeId === ref.fromNodeId) return null;
|
||||
if (resolved && isPythonFromImportModule(ref, this.context)) {
|
||||
const module = this.nodeById(resolved.targetNodeId);
|
||||
if (module && module.kind !== 'file' && module.kind !== 'module') return null;
|
||||
}
|
||||
if (!resolved || ref.referenceKind !== 'calls') return resolved;
|
||||
|
||||
const target = this.nodeById(resolved.targetNodeId);
|
||||
|
||||
@@ -16637,6 +16637,26 @@ function goEmbeddedTypes(typeNode: Node, context: ResolutionContext): Array<{ na
|
||||
return embedded;
|
||||
}
|
||||
|
||||
/**
|
||||
* The module of a Python `from X import …`, which the extractor refs at the
|
||||
* statement's start. It names a module or package, never a symbol: `from app
|
||||
* import email` is not a use of a variable named `app` (#2584).
|
||||
*/
|
||||
export function isPythonFromImportModule(ref: UnresolvedRef, context: ResolutionContext): boolean {
|
||||
if (ref.language !== 'python' || ref.referenceKind !== 'imports') return false;
|
||||
const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split(/\r?\n/);
|
||||
const line = lines?.[ref.line - 1];
|
||||
if (line === undefined || !line.startsWith('from', ref.column)) return false;
|
||||
// `from app \` continues the statement on the next line.
|
||||
let text = line.slice(ref.column + 4);
|
||||
for (let next = ref.line; /\\\s*$/.test(text) && next < ref.line + 3; next++) {
|
||||
text = text.replace(/\\\s*$/, ' ') + (lines![next] ?? '');
|
||||
}
|
||||
const name = text.trimStart();
|
||||
return name.length < text.length && name.startsWith(ref.referenceName) &&
|
||||
/^\s+import\b/.test(name.slice(ref.referenceName.length));
|
||||
}
|
||||
|
||||
/**
|
||||
* A Go call made through a type assertion — `srv.(KVServer).Range(ctx, in)`,
|
||||
* `c.Reader.(*pipe).Close()`, `v.(storage.Store).Fetch(k)` — calls a method of
|
||||
|
||||
Reference in New Issue
Block a user