From 87d26472a46801b3caacc2cb792ba19730fed1ac Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Tue, 7 Sep 2021 19:51:39 +0200 Subject: [PATCH] fix(manager/gradle): ignore catalog deps without version (#11621) * fix(manager/gradle): support catalog modules without version * fix: more ignorable deps * chore: fix type Co-authored-by: Rhys Arkins --- .../shallow/__fixtures__/2/libs.versions.toml | 3 + lib/manager/gradle/shallow/extract.spec.ts | 57 ++++- lib/manager/gradle/shallow/extract.ts | 12 +- lib/manager/gradle/shallow/extract/catalog.ts | 201 ++++++++++++++---- lib/manager/gradle/shallow/types.ts | 10 +- lib/manager/gradle/types.ts | 2 +- 6 files changed, 217 insertions(+), 68 deletions(-) diff --git a/lib/manager/gradle/shallow/__fixtures__/2/libs.versions.toml b/lib/manager/gradle/shallow/__fixtures__/2/libs.versions.toml index 8308a0c81674fa..85bad73700c60a 100644 --- a/lib/manager/gradle/shallow/__fixtures__/2/libs.versions.toml +++ b/lib/manager/gradle/shallow/__fixtures__/2/libs.versions.toml @@ -7,6 +7,9 @@ okHttp = "com.squareup.okhttp3:okhttp:4.9.0" okio = { module = "com.squareup.okio:okio", version = "2.8.0" } picasso = { group = "com.squareup.picasso", name = "picasso", version = "2.5.1" } retrofit2-retrofit = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" } +google-firebase-analytics = { module = "com.google.firebase:firebase-analytics" } +google-firebase-crashlytics = { group = "com.google.firebase", name = "firebase-crashlytics" } +google-firebase-messaging = "com.google.firebase:firebase-messaging" [plugins] kotlinJvm = { id = "org.jetbrains.kotlin.jvm", version = "1.5.21" } diff --git a/lib/manager/gradle/shallow/extract.spec.ts b/lib/manager/gradle/shallow/extract.spec.ts index e070e7371feef3..33678e57c896c9 100644 --- a/lib/manager/gradle/shallow/extract.spec.ts +++ b/lib/manager/gradle/shallow/extract.spec.ts @@ -1,5 +1,6 @@ import { extractAllPackageFiles } from '..'; import { fs, loadFixture } from '../../../../test/util'; +import type { ExtractConfig } from '../../types'; jest.mock('../../../util/fs'); @@ -24,7 +25,7 @@ describe('manager/gradle/shallow/extract', () => { 'build.gradle': '', }); - const res = await extractAllPackageFiles({} as never, [ + const res = await extractAllPackageFiles({} as ExtractConfig, [ 'build.gradle', 'gradle.properties', ]); @@ -39,7 +40,7 @@ describe('manager/gradle/shallow/extract', () => { 'settings.gradle': null, }); - const res = await extractAllPackageFiles({} as never, [ + const res = await extractAllPackageFiles({} as ExtractConfig, [ 'build.gradle', 'gradle.properties', 'settings.gradle', @@ -75,7 +76,7 @@ describe('manager/gradle/shallow/extract', () => { 'settings.gradle': null, }); - const res = await extractAllPackageFiles({} as never, [ + const res = await extractAllPackageFiles({} as ExtractConfig, [ 'build.gradle', 'gradle.properties', 'settings.gradle', @@ -114,7 +115,7 @@ describe('manager/gradle/shallow/extract', () => { 'settings.gradle': null, }); - const res = await extractAllPackageFiles({} as never, [ + const res = await extractAllPackageFiles({} as ExtractConfig, [ 'build.gradle', 'gradle.properties', 'settings.gradle', @@ -156,7 +157,10 @@ describe('manager/gradle/shallow/extract', () => { mockFs(fsMock); - const res = await extractAllPackageFiles({} as never, Object.keys(fsMock)); + const res = await extractAllPackageFiles( + {} as ExtractConfig, + Object.keys(fsMock) + ); expect(res).toMatchObject([ { packageFile: 'gradle.properties', deps: [] }, @@ -187,7 +191,10 @@ describe('manager/gradle/shallow/extract', () => { mockFs(fsMock); - const res = await extractAllPackageFiles({} as never, Object.keys(fsMock)); + const res = await extractAllPackageFiles( + {} as ExtractConfig, + Object.keys(fsMock) + ); expect(res).toMatchObject([ { @@ -218,7 +225,10 @@ describe('manager/gradle/shallow/extract', () => { 'gradle/libs.versions.toml': tomlFile, }; mockFs(fsMock); - const res = await extractAllPackageFiles({} as never, Object.keys(fsMock)); + const res = await extractAllPackageFiles( + {} as ExtractConfig, + Object.keys(fsMock) + ); expect(res).toMatchObject([ { packageFile: 'gradle/libs.versions.toml', @@ -302,7 +312,10 @@ describe('manager/gradle/shallow/extract', () => { 'gradle/libs.versions.toml': tomlFile, }; mockFs(fsMock); - const res = await extractAllPackageFiles({} as never, Object.keys(fsMock)); + const res = await extractAllPackageFiles( + {} as ExtractConfig, + Object.keys(fsMock) + ); expect(res).toMatchObject([ { packageFile: 'gradle/libs.versions.toml', @@ -343,6 +356,27 @@ describe('manager/gradle/shallow/extract', () => { packageFile: 'gradle/libs.versions.toml', }, }, + { + depName: 'google-firebase-analytics', + managerData: { + packageFile: 'gradle/libs.versions.toml', + }, + skipReason: 'no-version', + }, + { + depName: 'google-firebase-crashlytics', + managerData: { + packageFile: 'gradle/libs.versions.toml', + }, + skipReason: 'no-version', + }, + { + depName: 'google-firebase-messaging', + managerData: { + packageFile: 'gradle/libs.versions.toml', + }, + skipReason: 'no-version', + }, { depName: 'org.jetbrains.kotlin.jvm', depType: 'plugin', @@ -351,7 +385,7 @@ describe('manager/gradle/shallow/extract', () => { lookupName: 'org.jetbrains.kotlin.jvm:org.jetbrains.kotlin.jvm.gradle.plugin', managerData: { - fileReplacePosition: 415, + fileReplacePosition: 661, packageFile: 'gradle/libs.versions.toml', }, registryUrls: [ @@ -386,7 +420,10 @@ describe('manager/gradle/shallow/extract', () => { 'gradle/libs.versions.toml': tomlFile, }; mockFs(fsMock); - const res = await extractAllPackageFiles({} as never, Object.keys(fsMock)); + const res = await extractAllPackageFiles( + {} as ExtractConfig, + Object.keys(fsMock) + ); expect(res).toBeNull(); }); }); diff --git a/lib/manager/gradle/shallow/extract.ts b/lib/manager/gradle/shallow/extract.ts index cd9a3161f800a5..a89b590bd1cc23 100644 --- a/lib/manager/gradle/shallow/extract.ts +++ b/lib/manager/gradle/shallow/extract.ts @@ -67,12 +67,8 @@ export async function extractAllPackageFiles( updateVars(vars); extractedDeps.push(...deps); } else if (isTOMLFile(packageFile)) { - try { - const updatesFromCatalog = parseCatalog(packageFile, content); - extractedDeps.push(...updatesFromCatalog); - } catch (error) { - logger.warn({ error }, 'TOML parsing error'); - } + const updatesFromCatalog = parseCatalog(packageFile, content); + extractedDeps.push(...updatesFromCatalog); } else if (isGradleFile(packageFile)) { const vars = getVars(registry, dir); const { @@ -89,9 +85,9 @@ export async function extractAllPackageFiles( updateVars(gradleVars); extractedDeps.push(...deps); } - } catch (e) { + } catch (err) { logger.warn( - { config, packageFile }, + { err, config, packageFile }, `Failed to process Gradle file: ${packageFile}` ); } diff --git a/lib/manager/gradle/shallow/extract/catalog.ts b/lib/manager/gradle/shallow/extract/catalog.ts index 0812932b95731c..89200c8f829acf 100644 --- a/lib/manager/gradle/shallow/extract/catalog.ts +++ b/lib/manager/gradle/shallow/extract/catalog.ts @@ -1,7 +1,15 @@ import { parse } from '@iarna/toml'; -import { PackageDependency } from '../../../types'; -import { GradleManagerData } from '../../types'; -import type { GradleCatalog, GradleCatalogPluginDescriptor } from '../types'; +import deepmerge from 'deepmerge'; +import { SkipReason } from '../../../../types'; +import { hasKey } from '../../../../util/object'; +import type { PackageDependency } from '../../../types'; +import type { GradleManagerData } from '../../types'; +import type { + GradleCatalog, + GradleCatalogArtifactDescriptor, + GradleCatalogModuleDescriptor, + VersionPointer, +} from '../types'; function findIndexAfter( content: string, @@ -12,6 +20,123 @@ function findIndexAfter( return slicePoint + content.slice(slicePoint).indexOf(find); } +function isArtifactDescriptor( + obj: GradleCatalogArtifactDescriptor | GradleCatalogModuleDescriptor +): obj is GradleCatalogArtifactDescriptor { + return hasKey('group', obj); +} + +interface VersionExtract { + currentValue?: string; + fileReplacePosition?: number; +} + +function extractVersion({ + version, + versions, + depStartIndex, + depSubContent, + depName, + versionStartIndex, + versionSubContent, +}: { + version: string | VersionPointer; + versions: Record; + depStartIndex: number; + depSubContent: string; + depName: string; + versionStartIndex: number; + versionSubContent: string; +}): VersionExtract { + if (!version) { + return {}; + } + const currentValue = + typeof version === 'string' ? version : versions[version.ref]; + + const fileReplacePosition = + typeof version === 'string' + ? depStartIndex + findIndexAfter(depSubContent, depName, currentValue) + : versionStartIndex + + findIndexAfter(versionSubContent, version.ref, currentValue); + return { currentValue, fileReplacePosition }; +} + +function extractDependency({ + descriptor, + versions, + depStartIndex, + depSubContent, + depName, + versionStartIndex, + versionSubContent, +}: { + descriptor: + | string + | GradleCatalogModuleDescriptor + | GradleCatalogArtifactDescriptor; + versions: Record; + depStartIndex: number; + depSubContent: string; + depName: string; + versionStartIndex: number; + versionSubContent: string; +}): PackageDependency { + if (typeof descriptor === 'string') { + const [groupName, name, currentValue] = descriptor.split(':'); + if (!currentValue) { + return { + depName, + skipReason: SkipReason.NoVersion, + }; + } + return { + depName: `${groupName}:${name}`, + groupName, + currentValue, + managerData: { + fileReplacePosition: + depStartIndex + findIndexAfter(depSubContent, depName, currentValue), + }, + }; + } + + const { currentValue, fileReplacePosition } = extractVersion({ + version: descriptor.version, + versions, + depStartIndex, + depSubContent, + depName, + versionStartIndex, + versionSubContent, + }); + + if (!currentValue) { + return { + depName, + skipReason: SkipReason.NoVersion, + }; + } + + if (isArtifactDescriptor(descriptor)) { + const { group: groupName, name } = descriptor; + return { + depName: `${groupName}:${name}`, + groupName, + currentValue, + managerData: { fileReplacePosition }, + }; + } + const [groupName, name] = descriptor.module.split(':'); + const dependency = { + depName: `${groupName}:${name}`, + groupName, + currentValue, + managerData: { fileReplacePosition }, + }; + return dependency; +} + export function parseCatalog( packageFile: string, content: string @@ -26,58 +151,46 @@ export function parseCatalog( const extractedDeps: PackageDependency[] = []; for (const libraryName of Object.keys(libs)) { const libDescriptor = libs[libraryName]; - const group: string = - typeof libDescriptor === 'string' - ? libDescriptor.split(':')[0] - : libDescriptor.group || libDescriptor.module?.split(':')[0]; - const name: string = - typeof libDescriptor === 'string' - ? libDescriptor.split(':')[1] - : libDescriptor.name || libDescriptor.module?.split(':')[1]; - const version = libDescriptor.version || libDescriptor.split(':')[2]; - const currentVersion = - typeof version === 'string' ? version : versions[version.ref]; - const fileReplacePosition = - typeof version === 'string' - ? libStartIndex + - findIndexAfter(libSubContent, libraryName, currentVersion) - : versionStartIndex + - findIndexAfter(versionSubContent, version.ref, currentVersion); - const dependency = { - depName: `${group}:${name}`, - groupName: group, - currentValue: currentVersion, - managerData: { fileReplacePosition, packageFile }, - }; + const dependency = extractDependency({ + descriptor: libDescriptor, + versions, + depStartIndex: libStartIndex, + depSubContent: libSubContent, + depName: libraryName, + versionStartIndex, + versionSubContent, + }); extractedDeps.push(dependency); } + const plugins = tomlContent.plugins || {}; const pluginsStartIndex = content.indexOf('[plugins]'); const pluginsSubContent = content.slice(pluginsStartIndex); for (const pluginName of Object.keys(plugins)) { - const pluginDescriptor = plugins[ - pluginName - ] as GradleCatalogPluginDescriptor; - const pluginId = pluginDescriptor.id; - const version = pluginDescriptor.version; - const currentVersion: string = - typeof version === 'string' ? version : versions[version.ref]; - const fileReplacePosition = - typeof version === 'string' - ? pluginsStartIndex + - findIndexAfter(pluginsSubContent, pluginId, currentVersion) - : versionStartIndex + - findIndexAfter(versionSubContent, version.ref, currentVersion); + const pluginDescriptor = plugins[pluginName]; + const depName = pluginDescriptor.id; + const { currentValue, fileReplacePosition } = extractVersion({ + version: pluginDescriptor.version, + versions, + depStartIndex: pluginsStartIndex, + depSubContent: pluginsSubContent, + depName, + versionStartIndex, + versionSubContent, + }); + const dependency = { depType: 'plugin', - depName: pluginId, - lookupName: `${pluginId}:${pluginId}.gradle.plugin`, + depName, + lookupName: `${depName}:${depName}.gradle.plugin`, registryUrls: ['https://plugins.gradle.org/m2/'], - currentValue: currentVersion, + currentValue, commitMessageTopic: `plugin ${pluginName}`, - managerData: { fileReplacePosition, packageFile }, + managerData: { fileReplacePosition }, }; extractedDeps.push(dependency); } - return extractedDeps; + return extractedDeps.map((dep) => + deepmerge(dep, { managerData: { packageFile } }) + ); } diff --git a/lib/manager/gradle/shallow/types.ts b/lib/manager/gradle/shallow/types.ts index a1867f968ceaf0..384f14a08d28a2 100644 --- a/lib/manager/gradle/shallow/types.ts +++ b/lib/manager/gradle/shallow/types.ts @@ -63,23 +63,23 @@ export interface ParseGradleResult { } export interface GradleCatalog { - versions?: Map; - libraries?: Map< + versions?: Record; + libraries?: Record< string, GradleCatalogModuleDescriptor | GradleCatalogArtifactDescriptor | string >; - plugins?: Map; + plugins?: Record; } export interface GradleCatalogModuleDescriptor { module: string; - version: string | VersionPointer; + version?: string | VersionPointer; } export interface GradleCatalogArtifactDescriptor { name: string; group: string; - version: string | VersionPointer; + version?: string | VersionPointer; } export interface GradleCatalogPluginDescriptor { diff --git a/lib/manager/gradle/types.ts b/lib/manager/gradle/types.ts index db9264e9240d49..915ce0da45f9d0 100644 --- a/lib/manager/gradle/types.ts +++ b/lib/manager/gradle/types.ts @@ -1,4 +1,4 @@ export interface GradleManagerData { - fileReplacePosition: number; + fileReplacePosition?: number; packageFile?: string; }