diff --git a/packages/engine/src/python-analysis.ts b/packages/engine/src/python-analysis.ts index 21ee559..ddde019 100644 --- a/packages/engine/src/python-analysis.ts +++ b/packages/engine/src/python-analysis.ts @@ -249,26 +249,50 @@ def alias_public_name(alias): return alias.asname return alias.name.split(".")[0] +def format_arg(arg, default=None): + res = arg.arg + if getattr(arg, "annotation", None) is not None: + res += ":" + unparse(arg.annotation) + if default is not None: + res += "=" + unparse(default) + return res + def function_signature(args): positional_defaults = [None] * (len(args.posonlyargs) + len(args.args) - len(args.defaults)) + list(args.defaults) pieces = [] for arg, default in zip(args.posonlyargs, positional_defaults[:len(args.posonlyargs)]): - pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else "")) + pieces.append(format_arg(arg, default)) if args.posonlyargs: pieces.append("/") offset = len(args.posonlyargs) for arg, default in zip(args.args, positional_defaults[offset:]): - pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else "")) + pieces.append(format_arg(arg, default)) if args.vararg is not None: - pieces.append("*" + args.vararg.arg) + pieces.append("*" + format_arg(args.vararg)) elif args.kwonlyargs: pieces.append("*") for arg, default in zip(args.kwonlyargs, args.kw_defaults): - pieces.append(arg.arg + (("=" + unparse(default)) if default is not None else "")) + pieces.append(format_arg(arg, default)) if args.kwarg is not None: - pieces.append("**" + args.kwarg.arg) + pieces.append("**" + format_arg(args.kwarg)) return ",".join(pieces) +def function_surface_part(node): + prefix = "py:async_function:" if isinstance(node, ast.AsyncFunctionDef) else "py:function:" + ret = (":" + unparse(node.returns)) if getattr(node, "returns", None) is not None else "" + return prefix + node.name + "(" + function_signature(node.args) + ")" + ret + +def class_surface_parts(node): + bases = ",".join([unparse(base) for base in node.bases]) + parts = ["py:class:" + node.name + "(" + bases + ")"] + for item in node.body: + if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)): + if (bool(item.name) and not item.name.startswith("_")) or item.name == "__init__": + method_prefix = "async_method:" if isinstance(item, ast.AsyncFunctionDef) else "method:" + ret = (":" + unparse(item.returns)) if getattr(item, "returns", None) is not None else "" + parts.append("py:class_member:" + node.name + "." + method_prefix + item.name + "(" + function_signature(item.args) + ")" + ret) + return parts + imports = [] import_keys = set() warnings = [] @@ -277,9 +301,11 @@ top_level_public = set() surface_parts = [] def surface_part_name(part): - for prefix in ("py:function:", "py:class:"): + for prefix in ("py:async_function:", "py:function:", "py:class:"): if part.startswith(prefix): return part[len(prefix):].split("(", 1)[0] + if part.startswith("py:class_member:"): + return part[len("py:class_member:"):].split(".", 1)[0] if part.startswith("py:variable:"): return part[len("py:variable:"):].split(":", 1)[0] if part.startswith("py:import:"): @@ -346,13 +372,11 @@ for node in tree.body: elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): if is_public(node.name): top_level_public.add(node.name) - surface_parts.append("py:function:" + node.name + "(" + function_signature(node.args) + ")") + surface_parts.append(function_surface_part(node)) elif isinstance(node, ast.ClassDef): if is_public(node.name): top_level_public.add(node.name) - bases = ",".join([unparse(base) for base in node.bases]) - top_level_public.add(node.name) - surface_parts.append("py:class:" + node.name + "(" + bases + ")") + surface_parts.extend(class_surface_parts(node)) elif isinstance(node, ast.Assign): names = [] for target in node.targets: diff --git a/tests/module-resolution.test.mjs b/tests/module-resolution.test.mjs index 60a8ea5..288b62c 100644 --- a/tests/module-resolution.test.mjs +++ b/tests/module-resolution.test.mjs @@ -3004,8 +3004,8 @@ test("module resolution extracts Python public symbols and surface hashes", () = ); assert.deepEqual([...extractPublicSymbols(inferredPath)].sort(), ["Box", "VERSION", "fetch", "public_helper", "run"]); const expectedParts = [ + "py:async_function:fetch(value,limit=1)", "py:class:Box(Base)", - "py:function:fetch(value,limit=1)", "py:function:run(value)", "py:import:public_helper", "py:variable:VERSION:str", diff --git a/tests/review-regressions.test.mjs b/tests/review-regressions.test.mjs index 9cfef09..2e35b44 100644 --- a/tests/review-regressions.test.mjs +++ b/tests/review-regressions.test.mjs @@ -561,3 +561,71 @@ test("bug #61 empty Changed-Cells is checked against actual ownership", (testCon git(rootDir, ["add", "."]); git(rootDir, ["commit", "-qm", message("none")]); assert.equal(checkCommitEvidence({ rootDir, manifest, commit: "HEAD" }).ok, true); }); + +test("bug #76 Python public surface hashes differentiate async, type annotations, and class methods", () => { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "cellfence-bug-76-")); + try { + const filePath = path.join(rootDir, "api.py"); + // Pair 1: sync vs async + writeFile(filePath, ["def api(value):", " return value"]); + const syncHash = publicSurfaceHash(filePath); + const syncParts = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(syncParts, ["py:function:api(value)"]); + + writeFile(filePath, ["async def api(value):", " return value"]); + const asyncHash = publicSurfaceHash(filePath); + const asyncParts = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(asyncParts, ["py:async_function:api(value)"]); + assert.notEqual(asyncHash, syncHash); + + // Pair 2: type annotations + writeFile(filePath, ["def api(value: str) -> str:", " return value"]); + const strHash = publicSurfaceHash(filePath); + const strParts = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(strParts, ["py:function:api(value:str):str"]); + + writeFile(filePath, ["def api(value: int) -> int:", " return value"]); + const intHash = publicSurfaceHash(filePath); + const intParts = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(intParts, ["py:function:api(value:int):int"]); + assert.notEqual(intHash, strHash); + + // Pair 3: class methods + writeFile(filePath, [ + "class Api:", + " def fetch(self, value):", + " return value", + ]); + const classHash1 = publicSurfaceHash(filePath); + const classParts1 = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(classParts1, [ + "py:class:Api()", + "py:class_member:Api.method:fetch(self,value)", + ]); + + writeFile(filePath, [ + "class Api:", + " def fetch(self, value, required):", + " return required", + ]); + const classHash2 = publicSurfaceHash(filePath); + const classParts2 = inspectPythonSource(filePath).surfaceParts; + assert.deepEqual(classParts2, [ + "py:class:Api()", + "py:class_member:Api.method:fetch(self,value,required)", + ]); + assert.notEqual(classHash2, classHash1); + + // Changes confined to function body should not change the hash + writeFile(filePath, [ + "class Api:", + " def fetch(self, value, required):", + " # Internal comment or changed body logic", + " temp = value + required", + " return temp", + ]); + assert.equal(publicSurfaceHash(filePath), classHash2); + } finally { + fs.rmSync(rootDir, { recursive: true, force: true }); + } +});