From 127c5120dd645ecbadb0d141c633da2f3403d62a Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Wed, 5 Oct 2022 15:08:00 +0200 Subject: [PATCH] Only fail parsing KotlinToolingVersion if parsing major or minor fails ^KT-54301 Verification Pending --- .../tooling/core/KotlinToolingVersion.kt | 23 ++-- .../tooling/core/KotlinToolingVersionTest.kt | 103 ++++++++++++------ 2 files changed, 86 insertions(+), 40 deletions(-) diff --git a/libraries/tools/kotlin-tooling-core/src/main/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersion.kt b/libraries/tools/kotlin-tooling-core/src/main/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersion.kt index 623eefc054d..4959eaa33d0 100644 --- a/libraries/tools/kotlin-tooling-core/src/main/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersion.kt +++ b/libraries/tools/kotlin-tooling-core/src/main/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersion.kt @@ -11,18 +11,22 @@ import java.io.Serializable import java.util.* fun KotlinToolingVersion(kotlinVersionString: String): KotlinToolingVersion { - fun throwInvalid(reason: String): Nothing = throw IllegalArgumentException("Invalid Kotlin version: $kotlinVersionString ($reason)") - val baseVersion = kotlinVersionString.split("-", limit = 2)[0] val classifier = kotlinVersionString.split("-", limit = 2).getOrNull(1) val baseVersionSplit = baseVersion.split(".") - if (!(baseVersionSplit.size == 2 || baseVersionSplit.size == 3)) throwInvalid("Expected 2 or 3 '.' separated parts") + + val majorVersion = baseVersionSplit[0].toIntOrNull() + val minorVersion = baseVersionSplit.getOrNull(1)?.toIntOrNull() + + if (majorVersion == null || minorVersion == null) { + throw IllegalArgumentException("Invalid Kotlin version: $kotlinVersionString (Failed parsing major/minor version)") + } return KotlinToolingVersion( - major = baseVersionSplit[0].toIntOrNull() ?: throwInvalid("Failed parsing major version"), - minor = baseVersionSplit[1].toIntOrNull() ?: throwInvalid("Failed parsing minor version"), - patch = baseVersionSplit.getOrNull(2)?.let { it.toIntOrNull() ?: throwInvalid("Failed parsing patch version") } ?: 0, + major = majorVersion, + minor = minorVersion, + patch = baseVersionSplit.getOrNull(2)?.toIntOrNull() ?: 0, classifier = classifier ) } @@ -31,6 +35,10 @@ fun KotlinToolingVersion(kotlinVersion: KotlinVersion, classifier: String? = nul return KotlinToolingVersion(kotlinVersion.major, kotlinVersion.minor, kotlinVersion.patch, classifier) } +@Deprecated( + "Use KotlinToolingVersion instead. Scheduled for removal with Kotlin 1.9", + replaceWith = ReplaceWith("KotlinToolingVersion(kotlinVersionString)") +) fun KotlinToolingVersionOrNull(kotlinVersionString: String): KotlinToolingVersion? { return try { KotlinToolingVersion(kotlinVersionString) @@ -59,8 +67,7 @@ class KotlinToolingVersion( classifier.matches(Regex("""beta(\d*)?(-release)?(-?\d+)?""")) -> Maturity.BETA classifier.matches(Regex("""alpha(\d*)?(-release)?(-?\d+)?""")) -> Maturity.ALPHA classifier.matches(Regex("""m\d+(-release)?(-\d+)?""")) -> Maturity.MILESTONE - classifier.matches(Regex("""([a-zA-Z]{3,})(-[a-zA-Z]\w*)*(-\d+)?""")) -> Maturity.DEV - else -> throw IllegalArgumentException("Can't infer maturity of KotlinVersion $this") + else -> Maturity.DEV } } diff --git a/libraries/tools/kotlin-tooling-core/src/test/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersionTest.kt b/libraries/tools/kotlin-tooling-core/src/test/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersionTest.kt index f863dcc1f60..297dac7c604 100644 --- a/libraries/tools/kotlin-tooling-core/src/test/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersionTest.kt +++ b/libraries/tools/kotlin-tooling-core/src/test/kotlin/org/jetbrains/kotlin/tooling/core/KotlinToolingVersionTest.kt @@ -167,37 +167,32 @@ class KotlinToolingVersionTest { assertMaturity(DEV, "1.8.0-one-a1-b2-2022") assertMaturity(DEV, "1.8.0-temporary-999") assertMaturity(DEV, "1.8.0-snapshot-42", "Looks strange but snapshot is rather special to allow more usages") - assertMaturity(null, "1.8.0-dev-", "Forbid dash at the end") - assertMaturity(null, "1.8.0-t-1000", "Forbid too short first classifier") - assertMaturity(null, "1.8.0-no-1", "First classifier should be strictly longer than 2 letters") - assertMaturity(null, "1.8.0-dev----999", "Forbid many dashes in a row") - assertMaturity(null, "1.8.0-999-0", "Forbid fully digit classifier") - assertMaturity(null, "1.8.0-some-5-0", "Forbid fully digit second classifier") + assertMaturity(DEV, "1.8.0-dev-") + assertMaturity(DEV, "1.8.0-t-1000") + assertMaturity(DEV, "1.8.0-no-1") + assertMaturity(DEV, "1.8.0-dev----999") + assertMaturity(DEV, "1.8.0-999-0") + assertMaturity(DEV, "1.8.0-some-5-0") } @Test fun maturityWithAdditionalReleaseSuffix() { assertMaturity(MILESTONE, "1.6.20-M1-release") - assertMaturity(null, "1.6.20-M1-release1") + assertMaturity(DEV, "1.6.20-M1-release1") assertMaturity(MILESTONE, "1.6.20-M1-release-22") - assertMaturity(null, "1.6.20-M1-release-", "Forbid handling dash") + assertMaturity(DEV, "1.6.20-M1-release-") assertMaturity(ALPHA, "1.6.20-alpha-release") assertMaturity(ALPHA, "1.6.20-alpha-release1") assertMaturity(ALPHA, "1.6.20-alpha-release39") - assertMaturity(null, "1.6.20-alpha-release-", "Forbid handling dash") + assertMaturity(DEV, "1.6.20-alpha-release-") assertMaturity(BETA, "1.6.20-beta2-release") assertMaturity(BETA, "1.6.20-beta2-release1") assertMaturity(BETA, "1.6.20-beta2-release-1") - assertMaturity(null, "1.6.20-beta2-release-", "Forbid handling dash") + assertMaturity(DEV, "1.6.20-beta2-release-") assertMaturity(RC, "1.6.20-rc1-release") assertMaturity(RC, "1.6.20-rc1-release1") assertMaturity(RC, "1.6.20-rc1-release-1") - assertMaturity(null, "1.6.20-rc1-release-", "Forbid handling dash") - } - - @Test - fun invalidMilestoneVersion() { - assertMaturity(null, "1.6.20-M") + assertMaturity(DEV, "1.6.20-rc1-release-") } @Test @@ -310,14 +305,69 @@ class KotlinToolingVersionTest { ) } + @Suppress("deprecation") @Test fun illegalVersionString() { assertFailsWith { KotlinToolingVersion("x") } - assertFailsWith { KotlinToolingVersion("1.6.20.1") } + assertFailsWith { KotlinToolingVersion("x.") } + assertFailsWith { KotlinToolingVersion("1.") } + assertFailsWith { KotlinToolingVersion("1.x") } + assertFailsWith { KotlinToolingVersion("x.1") } assertNull(KotlinToolingVersionOrNull("")) assertNull(KotlinToolingVersionOrNull("x")) - assertNull(KotlinToolingVersionOrNull("1.6.20.1")) - assertNull(KotlinToolingVersionOrNull("1.6.20-x")) + } + + @Test + fun testBehaviourInEdgeCases() { + KotlinToolingVersion("1.6.20.1").apply { + assertEquals(STABLE, maturity) + assertEquals(1, major) + assertEquals(6, minor) + assertEquals(20, patch) + assertNull(classifier) + assertNull(classifierNumber) + assertNull(buildNumber) + } + + KotlinToolingVersion("1.6-x").apply { + assertEquals(DEV, maturity) + assertEquals(1, major) + assertEquals(6, minor) + assertEquals(0, patch) + assertEquals("x", classifier) + assertNull(classifierNumber) + assertNull(buildNumber) + } + + KotlinToolingVersion("1.8.0-dev----999").apply { + assertEquals(DEV, maturity) + assertEquals(1, major) + assertEquals(8, minor) + assertEquals(0, patch) + assertEquals("dev----999", classifier) + assertNull(classifierNumber) + assertEquals(999, buildNumber) + } + + KotlinToolingVersion("1.6.20-dev-google-pr-510").apply { + assertEquals(DEV, maturity) + assertEquals(1, major) + assertEquals(6, minor) + assertEquals(20, patch) + assertEquals("dev-google-pr-510", classifier) + assertNull(classifierNumber) + assertEquals(510, buildNumber) + } + + KotlinToolingVersion("1.6.x-dev").apply { + assertEquals(DEV, maturity) + assertEquals(1, major) + assertEquals(6, minor) + assertEquals(0, patch) + assertEquals("dev", classifier) + assertNull(classifierNumber) + assertNull(buildNumber) + } } private fun assertBuildNumber(buildNumber: Int?, version: String, message: String? = null) { @@ -328,18 +378,7 @@ class KotlinToolingVersionTest { assertEquals(classifierNumber, KotlinToolingVersion(version).classifierNumber, message) } - private fun assertMaturity(maturity: KotlinToolingVersion.Maturity?, version: String, message: String? = null) { - if (maturity == null) { - val exception = assertFailsWith( - "Parsing maturity is expected to be failed for `$version`${message?.let { ". $it" } ?: ""}") { - KotlinToolingVersion(version) - } - assertTrue( - "maturity" in exception.message.orEmpty().toLowerCase(), - "Expected 'maturity' issue mentioned in error message. Found: ${exception.message}" - ) - } else { - assertEquals(maturity, KotlinToolingVersion(version).maturity, message) - } + private fun assertMaturity(maturity: KotlinToolingVersion.Maturity, version: String, message: String? = null) { + assertEquals(maturity, KotlinToolingVersion(version).maturity, message) } }