From 7fea9581a136fec61ee269ba13abb73f290fb2df Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 18:41:33 -0400 Subject: [PATCH 1/8] add custom exception type --- .../exception/ConfigurationException.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ConfigurationException.kt diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ConfigurationException.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ConfigurationException.kt new file mode 100644 index 0000000..c0f1744 --- /dev/null +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ConfigurationException.kt @@ -0,0 +1,18 @@ +package dev.aga.gradle.versioncatalogs.exception + +class ConfigurationException : RuntimeException { + constructor() : super() + + constructor(message: String) : super(message) + + constructor(message: String, cause: Throwable) : super(message, cause) + + constructor(cause: Throwable) : super(cause) + + constructor( + message: String, + cause: Throwable, + enableSuppression: Boolean, + writableStackTrace: Boolean, + ) : super(message, cause, enableSuppression, writableStackTrace) +} From 63dba4ca121182bb8ae848c8d48a4123facc277a Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 18:52:15 -0400 Subject: [PATCH 2/8] improve error messaging in FileCatalogParser --- .../service/FileCatalogParser.kt | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt index b45b30e..26a8fe9 100644 --- a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt @@ -1,5 +1,6 @@ package dev.aga.gradle.versioncatalogs.service +import dev.aga.gradle.versioncatalogs.exception.ConfigurationException import java.io.File import org.apache.maven.model.Dependency import org.tomlj.Toml @@ -19,14 +20,16 @@ internal class FileCatalogParser(private val file: File) : CatalogParser { ?.let { it as? TomlTable } ?.let { it[libraryName] } ?.let { it as? TomlTable } - ?: throw RuntimeException("${libraryName} not found in catalog file") + ?: throw ConfigurationException( + "${libraryName} not found in catalog file ${file.absolutePath}", + ) val versions = toml["versions"]?.let { it as? TomlTable } - return getGAV(library, versions) + return getGAV(libraryName, library, versions) } - private fun getGAV(library: TomlTable, versions: TomlTable?): Dependency { + private fun getGAV(libraryName: String, library: TomlTable, versions: TomlTable?): Dependency { val (group, name) = if (library["module"] is String) { val split = (library["module"] as String).split(":") @@ -34,14 +37,18 @@ internal class FileCatalogParser(private val file: File) : CatalogParser { } else { val group = library["group"]?.let { it as? String } - ?: throw RuntimeException("Group not found ") + ?: throw ConfigurationException( + "Group not found for library ${libraryName} in catalog file ${file.absolutePath}", + ) val name = library["name"]?.let { it as? String } - ?: throw RuntimeException("Name not found") + ?: throw ConfigurationException( + "Name not found for library ${libraryName} in catalog file ${file.absolutePath}", + ) group to name } - val version = getVersion(library, versions) + val version = getVersion(libraryName, library, versions) return Dependency().apply { groupId = group artifactId = name @@ -49,13 +56,18 @@ internal class FileCatalogParser(private val file: File) : CatalogParser { } } - private fun getVersion(library: TomlTable, versions: TomlTable?): String { + private fun getVersion(libraryName: String, library: TomlTable, versions: TomlTable?): String { library.getString("version.ref")?.let { return versions?.getString(it) - ?: throw RuntimeException("Version ref '${it}' not found") + ?: throw ConfigurationException( + "Version ref '${it}' for library ${libraryName} not found in catalog file ${file.absolutePath}", + ) } - return library.getString("version") ?: throw RuntimeException("Version not found") + return library.getString("version") + ?: throw ConfigurationException( + "Version not found for library ${libraryName} in catalog file ${file.absolutePath}", + ) } private fun parseCatalog(file: File) = Toml.parse(file.toPath()) From 5f81472399c80fb9924dd176aecc449bab16d953 Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:18:59 -0400 Subject: [PATCH 3/8] improve logic in FileCatalogParser and add tests --- .../service/FileCatalogParser.kt | 18 +++++---- .../service/FileCatalogParserTest.kt | 38 +++++++++++++++---- plugin/src/test/resources/libs.versions.toml | 2 + 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt index 26a8fe9..e4eef94 100644 --- a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParser.kt @@ -57,17 +57,21 @@ internal class FileCatalogParser(private val file: File) : CatalogParser { } private fun getVersion(libraryName: String, library: TomlTable, versions: TomlTable?): String { - library.getString("version.ref")?.let { - return versions?.getString(it) + if (library.isTable("version") && library.isString("version.ref")) { + val ref = library.getString("version.ref") + return versions?.getString(ref) ?: throw ConfigurationException( - "Version ref '${it}' for library ${libraryName} not found in catalog file ${file.absolutePath}", + "Version ref '${ref}' not found for library ${libraryName} in catalog file ${file.absolutePath}", ) } - return library.getString("version") - ?: throw ConfigurationException( - "Version not found for library ${libraryName} in catalog file ${file.absolutePath}", - ) + if (library.isString("version")) { + return library.getString("version")!! + } + + throw ConfigurationException( + "Version not found for library ${libraryName} in catalog file ${file.absolutePath}", + ) } private fun parseCatalog(file: File) = Toml.parse(file.toPath()) diff --git a/plugin/src/test/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParserTest.kt b/plugin/src/test/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParserTest.kt index eb5ae59..b3dc26f 100644 --- a/plugin/src/test/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParserTest.kt +++ b/plugin/src/test/kotlin/dev/aga/gradle/versioncatalogs/service/FileCatalogParserTest.kt @@ -1,5 +1,6 @@ package dev.aga.gradle.versioncatalogs.service +import dev.aga.gradle.versioncatalogs.exception.ConfigurationException import java.nio.file.Paths import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatExceptionOfType @@ -11,7 +12,12 @@ import org.junit.jupiter.params.provider.MethodSource internal class FileCatalogParserTest { @ParameterizedTest @MethodSource("testFindBomProvider") - fun testFindBom(libraryName: String, expected: Array, shouldThrow: Boolean = false) { + fun testFindBom( + libraryName: String, + expected: Array, + shouldThrow: Boolean, + errorContains: String, + ) { val file = buildPath("libs.versions.toml").toFile() val parser = FileCatalogParser(file) if (!shouldThrow) { @@ -20,9 +26,9 @@ internal class FileCatalogParserTest { .extracting("groupId", "artifactId", "version") .containsExactly(*expected) } else { - assertThatExceptionOfType(RuntimeException::class.java).isThrownBy { - parser.findLibrary(libraryName) - } + assertThatExceptionOfType(ConfigurationException::class.java) + .isThrownBy { parser.findLibrary(libraryName) } + .withMessageContaining(errorContains) } } @@ -33,10 +39,26 @@ internal class FileCatalogParserTest { @JvmStatic private fun testFindBomProvider(): List { return listOf( - arguments("groovy-core", arrayOf("org.codehaus.groovy", "groovy", "3.0.5"), false), - arguments("fake-lib", arrayOf("dev.aga.lib", "fake-lib", "1.0.2"), false), - arguments("another-lib", arrayOf("dev.aga.lib", "another-lib", "1.0.0"), false), - arguments("commons-lang3", arrayOf(""), true), + arguments( + "groovy-core", + arrayOf("org.codehaus.groovy", "groovy", "3.0.5"), + false, + "", + ), + arguments("fake-lib", arrayOf("dev.aga.lib", "fake-lib", "1.0.2"), false, ""), + arguments("another-lib", arrayOf("dev.aga.lib", "another-lib", "1.0.0"), false, ""), + arguments( + "commons-lang3", + arrayOf(""), + true, + "Version not found for library commons-lang3 in catalog file", + ), + arguments( + "missing-ref", + arrayOf(""), + true, + "Version ref 'bad-ref' not found for library missing-ref in catalog file", + ), ) } diff --git a/plugin/src/test/resources/libs.versions.toml b/plugin/src/test/resources/libs.versions.toml index ceec759..bd6374a 100644 --- a/plugin/src/test/resources/libs.versions.toml +++ b/plugin/src/test/resources/libs.versions.toml @@ -8,9 +8,11 @@ groovy-core = { module = "org.codehaus.groovy:groovy", version.ref = "groovy" } groovy-json = { module = "org.codehaus.groovy:groovy-json", version.ref = "groovy" } groovy-nio = { module = "org.codehaus.groovy:groovy-nio", version.ref = "groovy" } commons-lang3 = { group = "org.apache.commons", name = "commons-lang3", version = { strictly = "[3.8, 4.0[", prefer = "3.9" } } +missing-ref = { group = "org.apache.commons", name = "commons-lang3", version.ref = "bad-ref" } fake-lib = { group = "dev.aga.lib", name = "fake-lib", version = "1.0.2" } another-lib = { group = "dev.aga.lib", name = "another-lib", version.ref = "dev" } + [bundles] groovy = ["groovy-core", "groovy-json", "groovy-nio"] From c708585cef8463d3a3d80348baaa91365604199d Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:20:14 -0400 Subject: [PATCH 4/8] update baseline --- plugin/config/detekt-baseline.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugin/config/detekt-baseline.xml b/plugin/config/detekt-baseline.xml index 1dd9dbd..3f9f8d0 100644 --- a/plugin/config/detekt-baseline.xml +++ b/plugin/config/detekt-baseline.xml @@ -4,11 +4,6 @@ EmptyFunctionBlock:VersionCatalogGeneratorPlugin.kt$VersionCatalogGeneratorPlugin${} MagicNumber:LocalDependencyResolver.kt$LocalDependencyResolver$3 - TooGenericExceptionThrown:FileCatalogParser.kt$FileCatalogParser$throw RuntimeException("${libraryName} not found in catalog file") - TooGenericExceptionThrown:FileCatalogParser.kt$FileCatalogParser$throw RuntimeException("Group not found ") - TooGenericExceptionThrown:FileCatalogParser.kt$FileCatalogParser$throw RuntimeException("Name not found") - TooGenericExceptionThrown:FileCatalogParser.kt$FileCatalogParser$throw RuntimeException("Version not found") - TooGenericExceptionThrown:FileCatalogParser.kt$FileCatalogParser$throw RuntimeException("Version ref '${it}' not found") TooGenericExceptionThrown:GradleDependencyResolver.kt$GradleDependencyResolver$throw RuntimeException("Unable to resolve ${notation}") TooGenericExceptionThrown:LocalDependencyResolver.kt$LocalDependencyResolver$throw RuntimeException( "LocalDependencyResolver can only resolve File, Path, or Dependency notations", ) TooGenericExceptionThrown:LocalDependencyResolver.kt$LocalDependencyResolver$throw RuntimeException("Path ${filePath} does not exist") From b6a012e343a0d5820e6a1385d48dceb879c37b07 Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:28:03 -0400 Subject: [PATCH 5/8] add custom exception type for Resolution errors --- .../exception/ResolutionException.kt | 18 ++++++++++++++++++ .../service/GradleDependencyResolver.kt | 3 ++- 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ResolutionException.kt diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ResolutionException.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ResolutionException.kt new file mode 100644 index 0000000..ca6c234 --- /dev/null +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/exception/ResolutionException.kt @@ -0,0 +1,18 @@ +package dev.aga.gradle.versioncatalogs.exception + +class ResolutionException : RuntimeException { + constructor() : super() + + constructor(message: String) : super(message) + + constructor(message: String, cause: Throwable) : super(message, cause) + + constructor(cause: Throwable) : super(cause) + + constructor( + message: String, + cause: Throwable, + enableSuppression: Boolean, + writableStackTrace: Boolean, + ) : super(message, cause, enableSuppression, writableStackTrace) +} diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/GradleDependencyResolver.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/GradleDependencyResolver.kt index 7b5d24c..713b6f9 100644 --- a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/GradleDependencyResolver.kt +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/service/GradleDependencyResolver.kt @@ -1,5 +1,6 @@ package dev.aga.gradle.versioncatalogs.service +import dev.aga.gradle.versioncatalogs.exception.ResolutionException import java.util.concurrent.atomic.AtomicInteger import java.util.function.Supplier import org.apache.maven.model.Model @@ -38,7 +39,7 @@ class GradleDependencyResolver( val resolver = LocalDependencyResolver(path.parent) return resolver.resolve(path.fileName) } - throw RuntimeException("Unable to resolve ${notation}") + throw ResolutionException("Unable to resolve ${notation}") } private fun createConfiguration(): Configuration { From 45dfb27557ea189dbdb14f019ae83a9e419ab453 Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:28:12 -0400 Subject: [PATCH 6/8] do not count exception classes towards coverage --- codecov.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 codecov.yml diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..45a325c --- /dev/null +++ b/codecov.yml @@ -0,0 +1,2 @@ +ignore: + - "**/exception/**" From edef47050b33b04846b86cbd930e97e43d1ca5f3 Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:28:21 -0400 Subject: [PATCH 7/8] update baseline --- plugin/config/detekt-baseline.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin/config/detekt-baseline.xml b/plugin/config/detekt-baseline.xml index 3f9f8d0..12d1909 100644 --- a/plugin/config/detekt-baseline.xml +++ b/plugin/config/detekt-baseline.xml @@ -4,7 +4,6 @@ EmptyFunctionBlock:VersionCatalogGeneratorPlugin.kt$VersionCatalogGeneratorPlugin${} MagicNumber:LocalDependencyResolver.kt$LocalDependencyResolver$3 - TooGenericExceptionThrown:GradleDependencyResolver.kt$GradleDependencyResolver$throw RuntimeException("Unable to resolve ${notation}") TooGenericExceptionThrown:LocalDependencyResolver.kt$LocalDependencyResolver$throw RuntimeException( "LocalDependencyResolver can only resolve File, Path, or Dependency notations", ) TooGenericExceptionThrown:LocalDependencyResolver.kt$LocalDependencyResolver$throw RuntimeException("Path ${filePath} does not exist") TooGenericExceptionThrown:LocalDependencyResolver.kt$LocalDependencyResolver$throw RuntimeException("Unable to parse notation '${notation}") From 57b6da44eba02a12af380c427424f23aee258a0d Mon Sep 17 00:00:00 2001 From: Austin Arbor Date: Mon, 4 Sep 2023 19:45:44 -0400 Subject: [PATCH 8/8] warn when dependencyManagement section is null or empty instead of possible NPE --- .../kotlin/dev/aga/gradle/versioncatalogs/Generator.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/Generator.kt b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/Generator.kt index 00a4fd2..c4eafef 100644 --- a/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/Generator.kt +++ b/plugin/src/main/kotlin/dev/aga/gradle/versioncatalogs/Generator.kt @@ -203,7 +203,14 @@ object Generator { seenModules: MutableSet = mutableSetOf(), filter: (Dependency) -> Boolean, ): Map> { - return model.dependencyManagement.dependencies + val deps = model.dependencyManagement?.dependencies ?: listOf() + if (deps.isEmpty()) { + logger.warn( + "${model.groupId}:${model.artifactId}:${model.version} does not have any dependencies defined " + + "in dependencyManagement", + ) + } + return deps .asSequence() .onEach { it.groupId = mapGroup(model, it.groupId) } .filter(filter)