From 95d3a1db8860f8be97374ca9a29ba1a9191f9367 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov <zharinov@users.noreply.github.com> Date: Fri, 25 Aug 2023 10:41:07 +0300 Subject: [PATCH] refactor(poetry): Use transforms for dependencies parsing (#23965) --- .../poetry/__snapshots__/extract.spec.ts.snap | 19 +-- lib/modules/manager/poetry/extract.spec.ts | 2 - lib/modules/manager/poetry/extract.ts | 120 +++----------- lib/modules/manager/poetry/schema.ts | 151 +++++++++++++++++- lib/modules/manager/poetry/utils.spec.ts | 40 ----- lib/modules/manager/poetry/utils.ts | 14 -- 6 files changed, 168 insertions(+), 178 deletions(-) delete mode 100644 lib/modules/manager/poetry/utils.spec.ts delete mode 100644 lib/modules/manager/poetry/utils.ts diff --git a/lib/modules/manager/poetry/__snapshots__/extract.spec.ts.snap b/lib/modules/manager/poetry/__snapshots__/extract.spec.ts.snap index f6ece0f45b..985e28f4ff 100644 --- a/lib/modules/manager/poetry/__snapshots__/extract.spec.ts.snap +++ b/lib/modules/manager/poetry/__snapshots__/extract.spec.ts.snap @@ -377,30 +377,19 @@ exports[`modules/manager/poetry/extract extractPackageFile() extracts multiple d "datasource": "pypi", "depName": "dep3", "depType": "dependencies", - "managerData": { - "nestedVersion": true, - }, "skipReason": "path-dependency", }, { - "currentValue": "", "datasource": "pypi", "depName": "dep4", "depType": "dependencies", - "managerData": { - "nestedVersion": false, - }, "skipReason": "path-dependency", }, { - "currentValue": "", "datasource": "pypi", "depName": "dep5", "depType": "dependencies", - "managerData": { - "nestedVersion": false, - }, - "versioning": "pep440", + "skipReason": "unspecified-version", }, { "currentValue": "^0.8.3", @@ -504,7 +493,7 @@ exports[`modules/manager/poetry/extract extractPackageFile() extracts multiple d "nestedVersion": false, }, "packageName": "dev-dep2", - "skipReason": "unspecified-version", + "skipReason": "invalid-version", }, { "currentValue": "^0.8.3", @@ -554,13 +543,9 @@ exports[`modules/manager/poetry/extract extractPackageFile() handles multiple co { "deps": [ { - "currentValue": "", "datasource": "pypi", "depName": "foo", "depType": "dependencies", - "managerData": { - "nestedVersion": false, - }, "skipReason": "multiple-constraint-dep", }, ], diff --git a/lib/modules/manager/poetry/extract.spec.ts b/lib/modules/manager/poetry/extract.spec.ts index dc2738e8ed..6f581b6dee 100644 --- a/lib/modules/manager/poetry/extract.spec.ts +++ b/lib/modules/manager/poetry/extract.spec.ts @@ -226,7 +226,6 @@ describe('modules/manager/poetry/extract', () => { `; const res = (await extractPackageFile(content, filename))!.deps; expect(res[0].depName).toBe('flask'); - expect(res[0].currentValue).toBeEmptyString(); expect(res[0].skipReason).toBe('git-dependency'); expect(res).toHaveLength(2); }); @@ -264,7 +263,6 @@ describe('modules/manager/poetry/extract', () => { `; const res = (await extractPackageFile(content, filename))!.deps; expect(res[0].depName).toBe('flask'); - expect(res[0].currentValue).toBe(''); expect(res[0].skipReason).toBe('path-dependency'); expect(res).toHaveLength(2); }); diff --git a/lib/modules/manager/poetry/extract.ts b/lib/modules/manager/poetry/extract.ts index b86135590e..3fc28ba9b1 100644 --- a/lib/modules/manager/poetry/extract.ts +++ b/lib/modules/manager/poetry/extract.ts @@ -1,27 +1,21 @@ import is from '@sindresorhus/is'; import { logger } from '../../../logger'; -import type { SkipReason } from '../../../types'; +import { filterMap } from '../../../util/filter-map'; import { getSiblingFileName, localPathExists, readLocalFile, } from '../../../util/fs'; -import { parseGitUrl } from '../../../util/git/url'; -import { regEx } from '../../../util/regex'; import { Result } from '../../../util/result'; -import { GithubTagsDatasource } from '../../datasource/github-tags'; -import { PypiDatasource } from '../../datasource/pypi'; -import * as pep440Versioning from '../../versioning/pep440'; -import * as poetryVersioning from '../../versioning/poetry'; import type { PackageDependency, PackageFileContent } from '../types'; import { Lockfile, type PoetryDependencyRecord, type PoetryGroupRecord, type PoetrySchema, + PoetrySchemaToml, type PoetrySectionSchema, } from './schema'; -import { parsePoetry } from './utils'; function extractFromDependenciesSection( parsedFile: PoetrySchema, @@ -66,84 +60,20 @@ function extractFromSection( return []; } - const deps: PackageDependency[] = []; - - for (const depName of Object.keys(sectionContent)) { - if (depName === 'python' || depName === 'source') { - continue; + return filterMap(Object.values(sectionContent), (dep) => { + if (dep.depName === 'python' || dep.depName === 'source') { + return null; } - const pep503NormalizeRegex = regEx(/[-_.]+/g); - let packageName = depName.toLowerCase().replace(pep503NormalizeRegex, '-'); - let skipReason: SkipReason | null = null; - let currentValue = sectionContent[depName]; - let nestedVersion = false; - let datasource = PypiDatasource.id; - let lockedVersion: string | null = null; - if (packageName in poetryLockfile) { - lockedVersion = poetryLockfile[packageName]; - } - if (!is.string(currentValue)) { - if (is.array(currentValue)) { - currentValue = ''; - skipReason = 'multiple-constraint-dep'; - } else { - const version = currentValue.version; - const path = currentValue.path; - const git = currentValue.git; - if (version) { - currentValue = version; - nestedVersion = true; - if (!!path || git) { - skipReason = path ? 'path-dependency' : 'git-dependency'; - } - } else if (path) { - currentValue = ''; - skipReason = 'path-dependency'; - } else if (git) { - if (currentValue.tag) { - currentValue = currentValue.tag; - datasource = GithubTagsDatasource.id; - const githubPackageName = extractGithubPackageName(git); - if (githubPackageName) { - packageName = githubPackageName; - } else { - skipReason = 'git-dependency'; - } - } else { - currentValue = ''; - skipReason = 'git-dependency'; - } - } else { - currentValue = ''; - } - } - } - const dep: PackageDependency = { - depName, - depType, - currentValue, - managerData: { nestedVersion }, - datasource, - }; - if (lockedVersion) { - dep.lockedVersion = lockedVersion; - } - if (depName !== packageName) { - dep.packageName = packageName; - } - if (skipReason) { - dep.skipReason = skipReason; - } else if (pep440Versioning.isValid(currentValue)) { - dep.versioning = pep440Versioning.id; - } else if (poetryVersioning.isValid(currentValue)) { - dep.versioning = poetryVersioning.id; - } else { - dep.skipReason = 'unspecified-version'; + dep.depType = depType; + + const packageName = dep.packageName ?? dep.depName; + if (packageName && packageName in poetryLockfile) { + dep.lockedVersion = poetryLockfile[packageName]; } - deps.push(dep); - } - return deps; + + return dep; + }); } function extractRegistries(pyprojectfile: PoetrySchema): string[] | undefined { @@ -169,9 +99,12 @@ export async function extractPackageFile( packageFile: string ): Promise<PackageFileContent | null> { logger.trace(`poetry.extractPackageFile(${packageFile})`); - const pyprojectfile = parsePoetry(packageFile, content); - if (!pyprojectfile?.tool?.poetry) { - logger.debug({ packageFile }, `contains no poetry section`); + const { val: pyprojectfile, err } = Result.parse( + PoetrySchemaToml, + content + ).unwrap(); + if (err) { + logger.debug({ packageFile, err }, `Poetry: error parsing pyproject.toml`); return null; } @@ -209,9 +142,10 @@ export async function extractPackageFile( const extractedConstraints: Record<string, any> = {}; - if (is.nonEmptyString(pyprojectfile?.tool?.poetry?.dependencies?.python)) { - extractedConstraints.python = - pyprojectfile?.tool?.poetry?.dependencies?.python; + const pythonVersion = + pyprojectfile?.tool?.poetry?.dependencies?.python?.currentValue; + if (is.nonEmptyString(pythonVersion)) { + extractedConstraints.python = pythonVersion; } const res: PackageFileContent = { @@ -233,11 +167,3 @@ export async function extractPackageFile( } return res; } - -function extractGithubPackageName(url: string): string | null { - const parsedUrl = parseGitUrl(url); - if (parsedUrl.source !== 'github.com') { - return null; - } - return `${parsedUrl.owner}/${parsedUrl.name}`; -} diff --git a/lib/modules/manager/poetry/schema.ts b/lib/modules/manager/poetry/schema.ts index 13e40c8013..9e5e3db50c 100644 --- a/lib/modules/manager/poetry/schema.ts +++ b/lib/modules/manager/poetry/schema.ts @@ -1,17 +1,152 @@ import { z } from 'zod'; +import { parseGitUrl } from '../../../util/git/url'; +import { regEx } from '../../../util/regex'; import { LooseArray, LooseRecord, Toml } from '../../../util/schema-utils'; +import { GitRefsDatasource } from '../../datasource/git-refs'; +import { GithubTagsDatasource } from '../../datasource/github-tags'; +import { PypiDatasource } from '../../datasource/pypi'; +import * as pep440Versioning from '../../versioning/pep440'; +import * as poetryVersioning from '../../versioning/poetry'; +import type { PackageDependency } from '../types'; -const PoetryDependencySchema = z.object({ - path: z.string().optional(), - git: z.string().optional(), - tag: z.string().optional(), - version: z.string().optional(), -}); +const PoetryPathDependency = z + .object({ + path: z.string(), + version: z.string().optional().catch(undefined), + }) + .transform(({ version }): PackageDependency => { + const dep: PackageDependency = { + datasource: PypiDatasource.id, + skipReason: 'path-dependency', + }; + + if (version) { + dep.currentValue = version; + } + + return dep; + }); + +const PoetryGitDependency = z + .object({ + git: z.string(), + tag: z.string().optional().catch(undefined), + version: z.string().optional().catch(undefined), + }) + .transform(({ git, tag, version }): PackageDependency => { + if (!tag) { + const res: PackageDependency = { + datasource: GitRefsDatasource.id, + packageName: git, + skipReason: 'git-dependency', + }; + + if (version) { + res.currentValue = version; + } + + return res; + } + + const parsedUrl = parseGitUrl(git); + if (parsedUrl.source !== 'github.com') { + return { + datasource: GitRefsDatasource.id, + currentValue: tag, + packageName: git, + skipReason: 'git-dependency', + }; + } + + const { owner, name } = parsedUrl; + const repo = `${owner}/${name}`; + return { + datasource: GithubTagsDatasource.id, + currentValue: tag, + packageName: repo, + }; + }); + +const PoetryPypiDependency = z.union([ + z + .object({ version: z.string().optional() }) + .transform(({ version: currentValue }): PackageDependency => { + if (!currentValue) { + return { datasource: PypiDatasource.id }; + } + + return { + datasource: PypiDatasource.id, + managerData: { nestedVersion: true }, + currentValue, + }; + }), + z.string().transform( + (version): PackageDependency => ({ + datasource: PypiDatasource.id, + currentValue: version, + managerData: { nestedVersion: false }, + }) + ), +]); + +const PoetryDependencySchema = z.union([ + PoetryPathDependency, + PoetryGitDependency, + PoetryPypiDependency, +]); + +const PoetryArraySchema = z.array(z.unknown()).transform( + (): PackageDependency => ({ + datasource: PypiDatasource.id, + skipReason: 'multiple-constraint-dep', + }) +); + +const PoetryValue = z.union([PoetryDependencySchema, PoetryArraySchema]); +type PoetryValue = z.infer<typeof PoetryValue>; export const PoetryDependencyRecord = LooseRecord( z.string(), - z.union([PoetryDependencySchema, z.array(PoetryDependencySchema), z.string()]) -); + PoetryValue.transform((dep) => { + if (dep.skipReason) { + return dep; + } + + // istanbul ignore if: normaly should not happen + if (!dep.currentValue) { + dep.skipReason = 'unspecified-version'; + return dep; + } + + if (pep440Versioning.isValid(dep.currentValue)) { + dep.versioning = pep440Versioning.id; + return dep; + } + + if (poetryVersioning.isValid(dep.currentValue)) { + dep.versioning = poetryVersioning.id; + return dep; + } + + dep.skipReason = 'invalid-version'; + return dep; + }) +).transform((record) => { + for (const [depName, dep] of Object.entries(record)) { + dep.depName = depName; + if (!dep.packageName) { + const pep503NormalizeRegex = regEx(/[-_.]+/g); + const packageName = depName + .toLowerCase() + .replace(pep503NormalizeRegex, '-'); + if (depName !== packageName) { + dep.packageName = packageName; + } + } + } + return record; +}); export type PoetryDependencyRecord = z.infer<typeof PoetryDependencyRecord>; diff --git a/lib/modules/manager/poetry/utils.spec.ts b/lib/modules/manager/poetry/utils.spec.ts deleted file mode 100644 index d3ba582378..0000000000 --- a/lib/modules/manager/poetry/utils.spec.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { codeBlock } from 'common-tags'; -import { parsePoetry } from './utils'; - -describe('modules/manager/poetry/utils', () => { - const fileName = 'fileName'; - - describe('parsePoetry', () => { - it('load and parse successfully', () => { - const fileContent = codeBlock` - [tool.poetry.dependencies] - dep1 = "1.0.0" - [tool.poetry.group.dev.dependencies] - dep2 = "1.0.1" - `; - const actual = parsePoetry(fileName, fileContent); - expect(actual).toMatchObject({ - tool: { - poetry: { - dependencies: { dep1: '1.0.0' }, - group: { dev: { dependencies: { dep2: '1.0.1' } } }, - }, - }, - }); - }); - - it('invalid toml', () => { - const actual = parsePoetry(fileName, 'clearly_invalid'); - expect(actual).toBeNull(); - }); - - it('invalid schema', () => { - const fileContent = codeBlock` - [tool.poetry.dependencies]: - dep1 = 1 - `; - const actual = parsePoetry(fileName, fileContent); - expect(actual).toBeNull(); - }); - }); -}); diff --git a/lib/modules/manager/poetry/utils.ts b/lib/modules/manager/poetry/utils.ts deleted file mode 100644 index 3ec55cef79..0000000000 --- a/lib/modules/manager/poetry/utils.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { logger } from '../../../logger'; -import { type PoetrySchema, PoetrySchemaToml } from './schema'; - -export function parsePoetry( - fileName: string, - fileContent: string -): PoetrySchema | null { - const res = PoetrySchemaToml.safeParse(fileContent); - if (res.success) { - return res.data; - } - logger.debug({ err: res.error, fileName }, 'Error parsing poetry lockfile.'); - return null; -} -- GitLab