diff --git a/packages/validation-python/src/node-shims.d.ts b/packages/validation-python/src/node-shims.d.ts index 3821fbd..a0d053b 100644 --- a/packages/validation-python/src/node-shims.d.ts +++ b/packages/validation-python/src/node-shims.d.ts @@ -29,6 +29,11 @@ declare module "node:fs" { export function writeFileSync(path: string, data: string): void; } +declare module "node:fs/promises" { + export function copyFile(source: string, destination: string): Promise; + export function mkdir(path: string, options?: { recursive?: boolean }): Promise; +} + declare module "node:os" { export function tmpdir(): string; } @@ -36,6 +41,9 @@ declare module "node:os" { declare module "node:path" { export function dirname(path: string): string; export function join(...paths: string[]): string; + export function relative(from: string, to: string): string; + export function resolve(...paths: string[]): string; + export const sep: string; } type BufferEncoding = "utf8"; diff --git a/packages/validation-python/src/type-check.ts b/packages/validation-python/src/type-check.ts index 609b4d3..7cd1bbf 100644 --- a/packages/validation-python/src/type-check.ts +++ b/packages/validation-python/src/type-check.ts @@ -1,10 +1,33 @@ import type { ValidationCheckDefinition } from "@the-open-engine/opcore-validation"; +import type { ValidationDiagnostic } from "@the-open-engine/opcore-contracts"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdir, copyFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join, relative, resolve, sep } from "node:path"; import { PYTHON_TYPES_CHECK_ID } from "./check-ids.js"; import { pythonCheckAdapter, pythonCheckOwner, supportedPythonValidationScopes } from "./check-constants.js"; -import { materializePythonSources } from "./source-files.js"; -import { probePythonToolchain, type PythonValidationToolchainOptions } from "./toolchain.js"; +import { diagnostic, sortDiagnostics } from "./diagnostics.js"; +import { runTool } from "./process.js"; +import { materializePythonSources, type PythonMaterializedSourceSet } from "./source-files.js"; +import { type PythonValidationToolchainOptions } from "./toolchain.js"; +import { resolvePythonTool, type PythonToolResolution } from "./toolchain-resolver.js"; -export interface PythonTypeCheckOptions extends PythonValidationToolchainOptions {} +export interface PythonTypeCheckOptions extends PythonValidationToolchainOptions { + timeoutMs?: number; +} + +interface MaterializedPythonTypeWorkspace { + root: string; + cleanup: () => void; +} + +const pythonToolConfigFiles = [ + "mypy.ini", + "setup.cfg", + "tox.ini", + "pyproject.toml", + "pyrightconfig.json" +] as const; export function createTypeCheck(options: PythonTypeCheckOptions = {}): ValidationCheckDefinition { return { @@ -17,9 +40,8 @@ export function createTypeCheck(options: PythonTypeCheckOptions = {}): Validatio const sourceSet = await materializePythonSources(context); if (sourceSet.files.length === 0) return { diagnostics: [] }; const repoRoot = context.request.repo.repoRoot ?? process.cwd(); - const toolchain = probePythonToolchain({ ...options, repoRoot }); - const hasTypeChecker = toolchain.some((tool) => (tool.tool === "mypy" || tool.tool === "pyright") && tool.available); - if (!hasTypeChecker) { + const checker = selectTypeChecker({ ...options, repoRoot }); + if (checker === undefined) { return { status: "unsupported_request", diagnostics: [ @@ -32,17 +54,149 @@ export function createTypeCheck(options: PythonTypeCheckOptions = {}): Validatio ] }; } - return { - status: "skipped", - diagnostics: [ - { - category: "types", - severity: "info", - code: "PYTHON_TYPES_DEFERRED", - message: "Python type validation tool execution is deferred to a follow-up adapter hardening pass." - } - ] - }; + + const workspace = await materializePythonTypeWorkspace(repoRoot, sourceSet); + try { + const result = runTool(checker.command, checkerArgs(checker, sourceSet), { + cwd: workspace.root, + env: options.env, + timeoutMs: options.timeoutMs ?? 30000 + }); + if (result.failureMessage !== undefined || result.exitCode === null) { + return unsupportedToolFailure(checker, result.failureMessage ?? `${checker.tool} invocation failed`); + } + return { + diagnostics: sortDiagnostics(parseTypeCheckerDiagnostics(checker, result.stdout, result.stderr, workspace.root)) + }; + } finally { + workspace.cleanup(); + } } }; } + +function selectTypeChecker(options: Required> & PythonValidationToolchainOptions): PythonToolResolution | undefined { + const resolverOptions = { repoRoot: options.repoRoot, env: options.env, pythonCommand: options.pythonCommand }; + const mypy = resolvePythonTool("mypy", "mypy", ["--version"], resolverOptions); + const pyright = resolvePythonTool("pyright", "pyright", ["--version"], resolverOptions); + if (!mypy.available && !pyright.available) return undefined; + if (pyright.available && !mypy.available) return pyright; + if (mypy.available && !pyright.available) return mypy; + if (pyright.configFile?.endsWith("pyrightconfig.json") && !mypy.configFile?.endsWith("mypy.ini")) return pyright; + return mypy; +} + +function checkerArgs(checker: PythonToolResolution, sourceSet: PythonMaterializedSourceSet): readonly string[] { + if (checker.tool === "pyright") return [...sourceSet.rootPaths]; + return [...sourceSet.rootPaths]; +} + +async function materializePythonTypeWorkspace( + repoRoot: string, + sourceSet: PythonMaterializedSourceSet +): Promise { + const tempRoot = mkdtempSync(join(tmpdir(), "opcore-python-types-")); + const root = join(tempRoot, "repo"); + try { + await mkdir(root, { recursive: true }); + await copyPythonConfigFiles(repoRoot, root); + for (const source of sourceSet.files) { + const absolutePath = resolveRepoPath(root, source.path); + await mkdir(dirname(absolutePath), { recursive: true }); + writeFileSync(absolutePath, source.content); + } + return { root, cleanup: () => rmSync(tempRoot, { recursive: true, force: true }) }; + } catch (error) { + rmSync(tempRoot, { recursive: true, force: true }); + throw error; + } +} + +async function copyPythonConfigFiles(repoRoot: string, root: string): Promise { + for (const file of pythonToolConfigFiles) { + const source = join(repoRoot, file); + if (!existsSync(source)) continue; + await copyFile(source, join(root, file)); + } +} + +function resolveRepoPath(root: string, path: string): string { + const absolutePath = resolve(root, path); + const relativePath = relative(root, absolutePath); + if (relativePath === "" || relativePath.startsWith("..") || relativePath.split(sep).includes("..")) { + throw new Error(`Repo-relative path escapes materialized Python workspace: ${path}`); + } + return absolutePath; +} + +function unsupportedToolFailure(checker: PythonToolResolution, message: string) { + return { + status: "unsupported_request" as const, + diagnostics: [ + diagnostic({ + category: "types", + severity: "info", + code: "PYTHON_TYPES_TOOL_FAILED", + message: `${checker.tool} could not run: ${message}` + }) + ] + }; +} + +function parseTypeCheckerDiagnostics( + checker: PythonToolResolution, + stdout: string, + stderr: string, + workspaceRoot: string +): readonly ValidationDiagnostic[] { + const text = [stdout, stderr].filter((part) => part.trim().length > 0).join("\n"); + const diagnostics: ValidationDiagnostic[] = []; + for (const line of text.split(/\r?\n/u)) { + const parsed = checker.tool === "pyright" ? parsePyrightLine(line, workspaceRoot) : parseMypyLine(line, workspaceRoot); + if (parsed !== undefined) diagnostics.push(parsed); + } + return diagnostics; +} + +function parseMypyLine(line: string, workspaceRoot: string): ValidationDiagnostic | undefined { + const match = /^(?.+?):(?\d+)(?::(?\d+))?:\s+(?error|warning|note):\s+(?.+?)(?:\s+\[(?[^\]]+)\])?$/u.exec(line.trim()); + if (match?.groups === undefined) return undefined; + const severity = match.groups.severity === "error" ? "error" : "warning"; + const code = match.groups.code !== undefined ? `MYPY_${normalizeDiagnosticCode(match.groups.code)}` : "MYPY_TYPE_ERROR"; + return diagnostic({ + category: "types", + severity, + path: repoRelativeDiagnosticPath(match.groups.path, workspaceRoot), + code, + message: withLocation(match.groups.message, match.groups.line, match.groups.column) + }); +} + +function parsePyrightLine(line: string, workspaceRoot: string): ValidationDiagnostic | undefined { + const match = /^\s*(?.+?):(?\d+):(?\d+)\s+-\s+(?error|warning|information):\s+(?.+?)(?:\s+\((?[^)]+)\))?$/u.exec(line.trim()); + if (match?.groups === undefined) return undefined; + const severity = match.groups.severity === "error" ? "error" : match.groups.severity === "warning" ? "warning" : "info"; + const code = match.groups.code !== undefined ? `PYRIGHT_${normalizeDiagnosticCode(match.groups.code)}` : "PYRIGHT_TYPE_ERROR"; + return diagnostic({ + category: "types", + severity, + path: repoRelativeDiagnosticPath(match.groups.path, workspaceRoot), + code, + message: withLocation(match.groups.message, match.groups.line, match.groups.column) + }); +} + +function repoRelativeDiagnosticPath(path: string, workspaceRoot: string): string { + const absolute = resolve(workspaceRoot, path); + const relativePath = relative(workspaceRoot, absolute).replaceAll("\\", "/"); + return relativePath.length > 0 && !relativePath.startsWith("..") ? relativePath : path.replaceAll("\\", "/"); +} + +function withLocation(message: string, line: string | undefined, column: string | undefined): string { + if (line === undefined) return message; + return column === undefined ? `${message} (line ${line})` : `${message} (line ${line}, column ${column})`; +} + +function normalizeDiagnosticCode(code: string): string { + return code.replace(/([a-z])([A-Z])/g, "$1_$2").replace(/[^A-Za-z0-9]+/g, "_").replace(/^_+|_+$/g, "").toUpperCase(); +} diff --git a/tests/validation-python.test.mjs b/tests/validation-python.test.mjs index 4e4f53b..a0fa9f0 100644 --- a/tests/validation-python.test.mjs +++ b/tests/validation-python.test.mjs @@ -71,6 +71,124 @@ describe("validation-python adapter", () => { assert.deepEqual(result.diagnostics.map((diagnostic) => diagnostic.code), ["PYTHON_TYPES_UNSUPPORTED"]); }); + it("executes repo-local mypy and maps type diagnostics", async () => { + const repoRoot = mkdtempSync(join(tmpdir(), "opcore-python-types-mypy-")); + try { + writeToolShim( + repoRoot, + "mypy", + [ + "#!/bin/sh", + "if [ \"$1\" = \"--version\" ]; then echo 'mypy 1.8.0'; exit 0; fi", + "while IFS= read -r line; do", + " case \"$line\" in", + " *\"'wrong'\"*) echo \"$1:1: error: Incompatible types in assignment (expression has type \\\"str\\\", variable has type \\\"int\\\") [assignment]\"; exit 1 ;;", + " esac", + "done < \"$1\"", + "exit 0", + "" + ].join("\n") + ); + + const result = await runner({ + files: { + "pkg/app.py": "value: int = 'wrong'\n" + }, + checks: createPythonValidationChecks({ env: { PATH: "" }, repoRoot }) + }).runValidation( + request({ + repo: { repoRoot }, + checks: [PYTHON_TYPES_CHECK_ID] + }) + ); + + assert.equal(result.status, "policy_failure"); + assert.equal(result.diagnostics[0].path, "pkg/app.py"); + assert.equal(result.diagnostics[0].category, "types"); + assert.equal(result.diagnostics[0].code, "MYPY_ASSIGNMENT"); + assert.match(result.diagnostics[0].message, /Incompatible types in assignment/); + } finally { + rmSync(repoRoot, { recursive: true, force: true }); + } + }); + + it("checks overlay after-state content instead of the original Python file", async () => { + const repoRoot = mkdtempSync(join(tmpdir(), "opcore-python-types-overlay-")); + try { + writeToolShim( + repoRoot, + "mypy", + [ + "#!/bin/sh", + "if [ \"$1\" = \"--version\" ]; then echo 'mypy 1.8.0'; exit 0; fi", + "while IFS= read -r line; do", + " case \"$line\" in", + " *\"'wrong'\"*) echo \"$1:1: error: overlay type failure [assignment]\"; exit 1 ;;", + " esac", + "done < \"$1\"", + "exit 0", + "" + ].join("\n") + ); + + const result = await runner({ + files: { + "pkg/app.py": "value: int = 1\n" + }, + checks: createPythonValidationChecks({ env: { PATH: "" }, repoRoot }) + }).runValidation( + request({ + repo: { repoRoot }, + checks: [PYTHON_TYPES_CHECK_ID], + overlays: [{ path: "pkg/app.py", action: "write", content: "value: int = 'wrong'\n" }] + }) + ); + + assert.equal(result.status, "policy_failure"); + assert.deepEqual(result.diagnostics.map((diagnostic) => diagnostic.code), ["MYPY_ASSIGNMENT"]); + assert.match(result.diagnostics[0].message, /overlay type failure/); + } finally { + rmSync(repoRoot, { recursive: true, force: true }); + } + }); + + it("prefers pyright when pyright project config is present", async () => { + const repoRoot = mkdtempSync(join(tmpdir(), "opcore-python-types-pyright-")); + try { + writeFileSync(join(repoRoot, "pyrightconfig.json"), "{}\n"); + writeToolShim(repoRoot, "mypy", "#!/bin/sh\nif [ \"$1\" = \"--version\" ]; then echo 'mypy 1.8.0'; exit 0; fi\necho 'mypy should not run'; exit 1\n"); + writeToolShim( + repoRoot, + "pyright", + [ + "#!/bin/sh", + "if [ \"$1\" = \"--version\" ]; then echo 'pyright 1.1.0'; exit 0; fi", + "echo \" $1:1:14 - error: Type \\\"Literal['wrong']\\\" is not assignable to declared type \\\"int\\\" (reportAssignmentType)\"", + "exit 1", + "" + ].join("\n") + ); + + const result = await runner({ + files: { + "pkg/app.py": "value: int = 'wrong'\n" + }, + checks: createPythonValidationChecks({ env: { PATH: "" }, repoRoot }) + }).runValidation( + request({ + repo: { repoRoot }, + checks: [PYTHON_TYPES_CHECK_ID] + }) + ); + + assert.equal(result.status, "policy_failure"); + assert.equal(result.diagnostics[0].code, "PYRIGHT_REPORT_ASSIGNMENT_TYPE"); + assert.equal(result.diagnostics[0].path, "pkg/app.py"); + } finally { + rmSync(repoRoot, { recursive: true, force: true }); + } + }); + it("reports syntax diagnostics from overlay after-state content", async () => { const result = await runner({ files: { @@ -482,6 +600,14 @@ function walkFiles(root) { return paths.sort(); } +function writeToolShim(repoRoot, name, content) { + const bin = join(repoRoot, ".venv", "bin"); + mkdirSync(bin, { recursive: true }); + const shimPath = join(bin, name); + writeFileSync(shimPath, content); + chmodSync(shimPath, 0o755); +} + function graphClient(overrides = {}) { return { status: (validationRequest) => availableStatus(validationRequest.graph.mode, validationRequest.repo),